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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <CAHP4M8Ww0-VqCBKX=iLd=zy1AcDoNdzTOqJuaqRxCGZsMhoX9w@mail.gmail.com>
Date:   Sun, 7 Nov 2021 00:50:17 +0530
From:   Ajay Garg <ajaygargnsit@...il.com>
To:     Andy Shevchenko <andy.shevchenko@...il.com>,
        Pavel Skripkin <paskripkin@...il.com>,
        Greg KH <gregkh@...uxfoundation.org>, jirislaby@...nel.org,
        kernel@...il.dk, David Laight <David.Laight@...lab.com>
Cc:     "linux-serial@...r.kernel.org" <linux-serial@...r.kernel.org>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>
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