Messages in this thread | | | Subject | Re: [PATCH 17/20] vt: Manual replacement of the deprecated strlcpy() with return values | From | Jiri Slaby <> | Date | Fri, 26 Feb 2021 10:49:02 +0100 |
| |
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@gmail.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
| |