Message ID | 20210222193922.11502-2-tim.gardner@canonical.com |
---|---|
State | New |
Headers | show |
Series | CVE-2020-25656 | expand |
On Mon, Feb 22, 2021 at 12:39:21PM -0700, Tim Gardner wrote: > From: Jiri Slaby <jslaby@suse.cz> > > Use 'strlen' of the string, add one for NUL terminator and simply do > 'copy_to_user' instead of the explicit 'for' loop. This makes the > KDGKBSENT case more compact. > > The only thing we need to take care about is NULL 'func_table[i]'. Use > an empty string in that case. > > The original check for overflow could never trigger as the func_buf > strings are always shorter or equal to 'struct kbsentry's. > > Cc: <stable@vger.kernel.org> > Signed-off-by: Jiri Slaby <jslaby@suse.cz> > Link: https://lore.kernel.org/r/20201019085517.10176-1-jslaby@suse.cz > Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org> > (cherry picked from commit 6ca03f90527e499dd5e32d6522909e2ad390896b) As this is part of a fix for CVE-2020-25656, there needs to be a CVE line, which we usually put around here: CVE-2020-25656 So, this commit appears as part of the changelog entry for the CVE. Aside from that, Acked-by: Thadeu Lima de Souza Cascardo <cascardo@canonical.com> > Signed-off-by: Tim Gardner <tim.gardner@canonical.com> > --- > drivers/tty/vt/keyboard.c | 28 +++++++++------------------- > 1 file changed, 9 insertions(+), 19 deletions(-) > > diff --git a/drivers/tty/vt/keyboard.c b/drivers/tty/vt/keyboard.c > index 568b2171f335..e64e3a7aa2f1 100644 > --- a/drivers/tty/vt/keyboard.c > +++ b/drivers/tty/vt/keyboard.c > @@ -1994,9 +1994,7 @@ int vt_do_kdsk_ioctl(int cmd, struct kbentry __user *user_kbe, int perm, > int vt_do_kdgkb_ioctl(int cmd, struct kbsentry __user *user_kdgkb, int perm) > { > struct kbsentry *kbs; > - char *p; > u_char *q; > - u_char __user *up; > int sz, fnw_sz; > int delta; > char *first_free, *fj, *fnw; > @@ -2022,23 +2020,15 @@ int vt_do_kdgkb_ioctl(int cmd, struct kbsentry __user *user_kdgkb, int perm) > i = kbs->kb_func; > > switch (cmd) { > - case KDGKBSENT: > - sz = sizeof(kbs->kb_string) - 1; /* sz should have been > - a struct member */ > - up = user_kdgkb->kb_string; > - p = func_table[i]; > - if(p) > - for ( ; *p && sz; p++, sz--) > - if (put_user(*p, up++)) { > - ret = -EFAULT; > - goto reterr; > - } > - if (put_user('\0', up)) { > - ret = -EFAULT; > - goto reterr; > - } > - kfree(kbs); > - return ((p && *p) ? -EOVERFLOW : 0); > + case KDGKBSENT: { > + /* size should have been a struct member */ > + unsigned char *from = func_table[i] ? : ""; > + > + ret = copy_to_user(user_kdgkb->kb_string, from, > + strlen(from) + 1) ? -EFAULT : 0; > + > + goto reterr; > + } > case KDSKBSENT: > if (!perm) { > ret = -EPERM; > -- > 2.17.1 > > > -- > kernel-team mailing list > kernel-team@lists.ubuntu.com > https://lists.ubuntu.com/mailman/listinfo/kernel-team
diff --git a/drivers/tty/vt/keyboard.c b/drivers/tty/vt/keyboard.c index 568b2171f335..e64e3a7aa2f1 100644 --- a/drivers/tty/vt/keyboard.c +++ b/drivers/tty/vt/keyboard.c @@ -1994,9 +1994,7 @@ int vt_do_kdsk_ioctl(int cmd, struct kbentry __user *user_kbe, int perm, int vt_do_kdgkb_ioctl(int cmd, struct kbsentry __user *user_kdgkb, int perm) { struct kbsentry *kbs; - char *p; u_char *q; - u_char __user *up; int sz, fnw_sz; int delta; char *first_free, *fj, *fnw; @@ -2022,23 +2020,15 @@ int vt_do_kdgkb_ioctl(int cmd, struct kbsentry __user *user_kdgkb, int perm) i = kbs->kb_func; switch (cmd) { - case KDGKBSENT: - sz = sizeof(kbs->kb_string) - 1; /* sz should have been - a struct member */ - up = user_kdgkb->kb_string; - p = func_table[i]; - if(p) - for ( ; *p && sz; p++, sz--) - if (put_user(*p, up++)) { - ret = -EFAULT; - goto reterr; - } - if (put_user('\0', up)) { - ret = -EFAULT; - goto reterr; - } - kfree(kbs); - return ((p && *p) ? -EOVERFLOW : 0); + case KDGKBSENT: { + /* size should have been a struct member */ + unsigned char *from = func_table[i] ? : ""; + + ret = copy_to_user(user_kdgkb->kb_string, from, + strlen(from) + 1) ? -EFAULT : 0; + + goto reterr; + } case KDSKBSENT: if (!perm) { ret = -EPERM;