Re: [PATCH] tty: vt: keyboard: do not copy an extra-byte in copy_to_user

[Ajay Garg]

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@xxxxxxxxxxxxxxxx/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@xxxxxxxxxxxxxxxx/T/#m1c4aaa4347b02fd4c11ce611ff5029fcb71c37a1

David has provided a simpler fix (usage of strscpy), as in :
https://lore.kernel.org/linux-serial/868025b485b94480ad17d0ec971b3ee9@xxxxxxxxxxxxxxxx/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