Messages in this thread | | | From | Ajay Garg <> | Date | Sun, 7 Nov 2021 00:50:17 +0530 | Subject | Re: [PATCH] tty: vt: keyboard: do not copy an extra-byte in copy_to_user |
| |
Thanks Pavel, Andy, David for the help.
Andy,
There is no compilation/runtime blocker. There were warnings reported by smatch.
My intention is to make the method "vt_do_kdgkb_ioctl" bullet-proof in itself, without depending upon external clients.
Pavel has explained that currently things are fine, as per : https://lore.kernel.org/linux-serial/868025b485b94480ad17d0ec971b3ee9@AcuMS.aculab.com/T/#m740fffb7c6ee52fdc98b9ef0b4e32a060b6a3be3
but it seems that there is a big flaw - we are dependent on the length of "func_table[kb_func]" being ok. If func_table[kb_func] goes awry, the method will cause overflow.
Since func_table[kb_func]" is not managed by the method, so the method must not depend on func_table[kb_func]" length-correctness. Instead, "vt_do_kdgkb_ioctl" must ensure no overflow, without depending how external entities (func_table[kb_func] behave.
The issue with strlcpy, along with a potential "fix", has been explained in : https://lore.kernel.org/linux-serial/868025b485b94480ad17d0ec971b3ee9@AcuMS.aculab.com/T/#m1c4aaa4347b02fd4c11ce611ff5029fcb71c37a1
David has provided a simpler fix (usage of strscpy), as in : https://lore.kernel.org/linux-serial/868025b485b94480ad17d0ec971b3ee9@AcuMS.aculab.com/T/#m63dab1137e593f2030920a53272f71866b442f40
So, we could go with one of the above changes (mine/David's), or nothing at all (since there is no blocker).
I vote for David's strscpy "fix", as it is simple, and does away with the dependency on the length of "func_table[kb_func]".
Would like to know what the maintainers think. If there is a consensus that the method "vt_do_kdgkb_ioctl" be made bullet-proof in itself, please let me know, I will float the next version of patch.
Thanks again Pavel, David, Andy.
Thanks and Regards, Ajay
| |