[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <a9f26339-c366-40c4-1cd6-c7ae1838e2b6@kernel.org>
Date: Fri, 26 Feb 2021 10:49:02 +0100
From: Jiri Slaby <jirislaby@...nel.org>
To: Romain Perier <romain.perier@...il.com>, Kees Cook
<keescook@...omium.org>, kernel-hardening@...ts.openwall.com,
Greg Kroah-Hartman <gregkh@...uxfoundation.org>
Cc: linux-kernel@...r.kernel.org
Subject: Re: [PATCH 17/20] vt: Manual replacement of the deprecated strlcpy()
with return values
On 22. 02. 21, 16:12, Romain Perier wrote:
> The strlcpy() reads the entire source buffer first, it is dangerous if
> the source buffer lenght is unbounded or possibility non NULL-terminated.
"length" and it's NUL, not NULL in this case.
> It can lead to linear read overflows, crashes, etc...
>
> As recommended in the deprecated interfaces [1], it should be replaced
> by strscpy.
>
> This commit replaces all calls to strlcpy that handle the return values
s/that/which/ ?
"handles"
"value"
> by the corresponding strscpy calls with new handling of the return
> values (as it is quite different between the two functions).
Sorry, I have hard times understand the whole sentence. Could you
rephrase a bit?
> [1] https://www.kernel.org/doc/html/latest/process/deprecated.html#strlcpy
>
> Signed-off-by: Romain Perier <romain.perier@...il.com>
> ---
> drivers/tty/vt/keyboard.c | 5 ++++-
> 1 file changed, 4 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/tty/vt/keyboard.c b/drivers/tty/vt/keyboard.c
> index 77638629c562..5e20c6c307e0 100644
> --- a/drivers/tty/vt/keyboard.c
> +++ b/drivers/tty/vt/keyboard.c
> @@ -2067,9 +2067,12 @@ int vt_do_kdgkb_ioctl(int cmd, struct kbsentry __user *user_kdgkb, int perm)
> return -ENOMEM;
>
> spin_lock_irqsave(&func_buf_lock, flags);
> - len = strlcpy(kbs, func_table[kb_func] ? : "", len);
> + len = strscpy(kbs, func_table[kb_func] ? : "", len);
func_table[kb_func] is NUL-terminated and kbs is of length len anyway,
so this is only cosmetical.
> spin_unlock_irqrestore(&func_buf_lock, flags);
>
> + if (len == -E2BIG)
> + return -E2BIG;
> +
This can never happen, right?
> ret = copy_to_user(user_kdgkb->kb_string, kbs, len + 1) ?
> -EFAULT : 0;
>
>
thanks,
--
js
Powered by blists - more mailing lists
Confused about mailing lists and their use?
Read about mailing lists on Wikipedia
and check out these
guidelines on proper formatting of your messages.
