* [PATCH 0/5] vt: get rid of worst cases of __put_user/__get_user
@ 2017-06-03 7:32 Adam Borowski
2017-06-03 7:35 ` [PATCH 1/5] vt: use copy_from/to_user instead of __get/put_user for scrnmap ioctls Adam Borowski
2017-06-03 15:42 ` [PATCH 0/5] vt: get rid of worst cases of __put_user/__get_user Greg Kroah-Hartman
0 siblings, 2 replies; 12+ messages in thread
From: Adam Borowski @ 2017-06-03 7:32 UTC (permalink / raw)
To: Greg Kroah-Hartman, Jiri Slaby, linux-kernel
Hi!
In a recent discussion, Linus and Al Viro said quite a bit of expletives
about __put_user() and __get_user(), that it's a bad interface that's
almost always the wrong thing to use:
https://marc.info/?l=linux-kernel&m=149463725626316&w=2
https://marc.info/?l=linux-kernel&m=149465866929092&w=2
Here's a few patches applying the lessons from that discussion to vt.
None of the uses is performance-critical, but at least we get a nice bit
of code simplification. And, it's a start of manual review + conversion
that Al Viro wants.
Adam Borowski (5):
vt: use copy_from/to_user instead of __get/put_user for scrnmap ioctls
vt: fix unchecked __put_user() in tioclinux ioctls
vt: use copy_to_user instead of __put_user in GIO_UNIMAP ioctl
vt: use memdup_user in PIO_UNIMAP ioctl
vt: drop access_ok() calls in unimap ioctls
drivers/tty/vt/consolemap.c | 56 ++++++++++++++++----------------------------------------
drivers/tty/vt/vt.c | 6 +++---
drivers/tty/vt/vt_ioctl.c | 8 --------
3 files changed, 19 insertions(+), 51 deletions(-)
--
⢀⣴⠾⠻⢶⣦⠀ A tit a day keeps the vet away.
⣾⠁⢰⠒⠀⣿⡁
⢿⡄⠘⠷⠚⠋⠀ (Rejoice as my small-animal-murder-machine got unbroken after
⠈⠳⣄⠀⠀⠀⠀ nearly two years of no catch!)
^ permalink raw reply [flat|nested] 12+ messages in thread* [PATCH 1/5] vt: use copy_from/to_user instead of __get/put_user for scrnmap ioctls 2017-06-03 7:32 [PATCH 0/5] vt: get rid of worst cases of __put_user/__get_user Adam Borowski @ 2017-06-03 7:35 ` Adam Borowski 2017-06-03 7:35 ` [PATCH 2/5] vt: fix unchecked __put_user() in tioclinux ioctls Adam Borowski ` (3 more replies) 2017-06-03 15:42 ` [PATCH 0/5] vt: get rid of worst cases of __put_user/__get_user Greg Kroah-Hartman 1 sibling, 4 replies; 12+ messages in thread From: Adam Borowski @ 2017-06-03 7:35 UTC (permalink / raw) To: Greg Kroah-Hartman, Jiri Slaby, linux-kernel; +Cc: Adam Borowski Linus wants to get rid of these functions, and these uses are especially egregious: they copy a big linear array element by element. Signed-off-by: Adam Borowski <kilobyte@angband.pl> --- drivers/tty/vt/consolemap.c | 31 +++++++------------------------ 1 file changed, 7 insertions(+), 24 deletions(-) diff --git a/drivers/tty/vt/consolemap.c b/drivers/tty/vt/consolemap.c index 1f6e17fc3fb0..1361f2a8b832 100644 --- a/drivers/tty/vt/consolemap.c +++ b/drivers/tty/vt/consolemap.c @@ -322,15 +322,13 @@ int con_set_trans_old(unsigned char __user * arg) { int i; unsigned short inbuf[E_TABSZ]; + unsigned char ubuf[E_TABSZ]; - if (!access_ok(VERIFY_READ, arg, E_TABSZ)) + if (copy_from_user(ubuf, arg, E_TABSZ)) return -EFAULT; - for (i = 0; i < E_TABSZ ; i++) { - unsigned char uc; - __get_user(uc, arg+i); - inbuf[i] = UNI_DIRECT_BASE | uc; - } + for (i = 0; i < E_TABSZ ; i++) + inbuf[i] = UNI_DIRECT_BASE | ubuf[i]; console_lock(); memcpy(translations[USER_MAP], inbuf, sizeof(inbuf)); @@ -345,9 +343,6 @@ int con_get_trans_old(unsigned char __user * arg) unsigned short *p = translations[USER_MAP]; unsigned char outbuf[E_TABSZ]; - if (!access_ok(VERIFY_WRITE, arg, E_TABSZ)) - return -EFAULT; - console_lock(); for (i = 0; i < E_TABSZ ; i++) { @@ -356,22 +351,16 @@ int con_get_trans_old(unsigned char __user * arg) } console_unlock(); - for (i = 0; i < E_TABSZ ; i++) - __put_user(outbuf[i], arg+i); - return 0; + return copy_to_user(arg, outbuf, sizeof(outbuf)) ? -EFAULT : 0; } int con_set_trans_new(ushort __user * arg) { - int i; unsigned short inbuf[E_TABSZ]; - if (!access_ok(VERIFY_READ, arg, E_TABSZ*sizeof(unsigned short))) + if (copy_from_user(inbuf, arg, sizeof(inbuf))) return -EFAULT; - for (i = 0; i < E_TABSZ ; i++) - __get_user(inbuf[i], arg+i); - console_lock(); memcpy(translations[USER_MAP], inbuf, sizeof(inbuf)); update_user_maps(); @@ -381,19 +370,13 @@ int con_set_trans_new(ushort __user * arg) int con_get_trans_new(ushort __user * arg) { - int i; unsigned short outbuf[E_TABSZ]; - if (!access_ok(VERIFY_WRITE, arg, E_TABSZ*sizeof(unsigned short))) - return -EFAULT; - console_lock(); memcpy(outbuf, translations[USER_MAP], sizeof(outbuf)); console_unlock(); - for (i = 0; i < E_TABSZ ; i++) - __put_user(outbuf[i], arg+i); - return 0; + return copy_to_user(arg, outbuf, sizeof(outbuf)) ? -EFAULT : 0; } /* -- 2.11.0 ^ permalink raw reply related [flat|nested] 12+ messages in thread
* [PATCH 2/5] vt: fix unchecked __put_user() in tioclinux ioctls 2017-06-03 7:35 ` [PATCH 1/5] vt: use copy_from/to_user instead of __get/put_user for scrnmap ioctls Adam Borowski @ 2017-06-03 7:35 ` Adam Borowski 2017-06-03 7:35 ` [PATCH 3/5] vt: use copy_to_user instead of __put_user in GIO_UNIMAP ioctl Adam Borowski ` (2 subsequent siblings) 3 siblings, 0 replies; 12+ messages in thread From: Adam Borowski @ 2017-06-03 7:35 UTC (permalink / raw) To: Greg Kroah-Hartman, Jiri Slaby, linux-kernel Cc: Adam Borowski, stable, #, v3.7- Only read access is checked before this call. Actually, at the moment this is not an issue, as every in-tree arch does the same manual checks for VERIFY_READ vs VERIFY_WRITE, relying on the MMU to tell them apart, but this wasn't the case in the past and may happen again on some odd arch in the future. If anyone cares about 3.7 and earlier, this is a security hole (untested) on real 80386 CPUs. Signed-off-by: Adam Borowski <kilobyte@angband.pl> CC: stable@vger.kernel.org # v3.7- --- drivers/tty/vt/vt.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/drivers/tty/vt/vt.c b/drivers/tty/vt/vt.c index 9309c7da660a..2ebaba16f785 100644 --- a/drivers/tty/vt/vt.c +++ b/drivers/tty/vt/vt.c @@ -2709,13 +2709,13 @@ int tioclinux(struct tty_struct *tty, unsigned long arg) * related to the kernel should not use this. */ data = vt_get_shift_state(); - ret = __put_user(data, p); + ret = put_user(data, p); break; case TIOCL_GETMOUSEREPORTING: console_lock(); /* May be overkill */ data = mouse_reporting(); console_unlock(); - ret = __put_user(data, p); + ret = put_user(data, p); break; case TIOCL_SETVESABLANK: console_lock(); @@ -2724,7 +2724,7 @@ int tioclinux(struct tty_struct *tty, unsigned long arg) break; case TIOCL_GETKMSGREDIRECT: data = vt_get_kmsg_redirect(); - ret = __put_user(data, p); + ret = put_user(data, p); break; case TIOCL_SETKMSGREDIRECT: if (!capable(CAP_SYS_ADMIN)) { -- 2.11.0 ^ permalink raw reply related [flat|nested] 12+ messages in thread
* [PATCH 3/5] vt: use copy_to_user instead of __put_user in GIO_UNIMAP ioctl 2017-06-03 7:35 ` [PATCH 1/5] vt: use copy_from/to_user instead of __get/put_user for scrnmap ioctls Adam Borowski 2017-06-03 7:35 ` [PATCH 2/5] vt: fix unchecked __put_user() in tioclinux ioctls Adam Borowski @ 2017-06-03 7:35 ` Adam Borowski 2017-06-05 16:34 ` Alan Cox 2017-06-03 7:35 ` [PATCH 4/5] vt: use memdup_user in PIO_UNIMAP ioctl Adam Borowski 2017-06-03 7:35 ` [PATCH 5/5] vt: drop access_ok() calls in unimap ioctls Adam Borowski 3 siblings, 1 reply; 12+ messages in thread From: Adam Borowski @ 2017-06-03 7:35 UTC (permalink / raw) To: Greg Kroah-Hartman, Jiri Slaby, linux-kernel; +Cc: Adam Borowski A nice big linear transfer, no need to flip stac/PAN/etc every half-entry. Also, yay __put_user() after checking only read. Signed-off-by: Adam Borowski <kilobyte@angband.pl> --- drivers/tty/vt/consolemap.c | 14 ++++++-------- 1 file changed, 6 insertions(+), 8 deletions(-) diff --git a/drivers/tty/vt/consolemap.c b/drivers/tty/vt/consolemap.c index 1361f2a8b832..c6a692f63a9b 100644 --- a/drivers/tty/vt/consolemap.c +++ b/drivers/tty/vt/consolemap.c @@ -740,11 +740,11 @@ EXPORT_SYMBOL(con_copy_unimap); */ int con_get_unimap(struct vc_data *vc, ushort ct, ushort __user *uct, struct unipair __user *list) { - int i, j, k; + int i, j, k, ret = 0; ushort ect; u16 **p1, *p2; struct uni_pagedir *p; - struct unipair *unilist, *plist; + struct unipair *unilist; unilist = kmalloc_array(ct, sizeof(struct unipair), GFP_KERNEL); if (!unilist) @@ -775,13 +775,11 @@ int con_get_unimap(struct vc_data *vc, ushort ct, ushort __user *uct, struct uni } } console_unlock(); - for (i = min(ect, ct), plist = unilist; i; i--, list++, plist++) { - __put_user(plist->unicode, &list->unicode); - __put_user(plist->fontpos, &list->fontpos); - } - __put_user(ect, uct); + if (copy_to_user(list, unilist, min(ect, ct) * sizeof(struct unipair))) + ret = -EFAULT; + put_user(ect, uct); kfree(unilist); - return ((ect <= ct) ? 0 : -ENOMEM); + return ret ? ret : (ect <= ct) ? 0 : -ENOMEM; } /* -- 2.11.0 ^ permalink raw reply related [flat|nested] 12+ messages in thread
* Re: [PATCH 3/5] vt: use copy_to_user instead of __put_user in GIO_UNIMAP ioctl 2017-06-03 7:35 ` [PATCH 3/5] vt: use copy_to_user instead of __put_user in GIO_UNIMAP ioctl Adam Borowski @ 2017-06-05 16:34 ` Alan Cox 2017-06-05 23:05 ` Al Viro 0 siblings, 1 reply; 12+ messages in thread From: Alan Cox @ 2017-06-05 16:34 UTC (permalink / raw) To: Adam Borowski; +Cc: Greg Kroah-Hartman, Jiri Slaby, linux-kernel > @@ -775,13 +775,11 @@ int con_get_unimap(struct vc_data *vc, ushort ct, ushort __user *uct, struct uni > } > } > console_unlock(); > - for (i = min(ect, ct), plist = unilist; i; i--, list++, plist++) { > - __put_user(plist->unicode, &list->unicode); > - __put_user(plist->fontpos, &list->fontpos); > - } > - __put_user(ect, uct); > + if (copy_to_user(list, unilist, min(ect, ct) * sizeof(struct unipair))) > + ret = -EFAULT; > + put_user(ect, uct); > kfree(unilist); > - return ((ect <= ct) ? 0 : -ENOMEM); > + return ret ? ret : (ect <= ct) ? 0 : -ENOMEM; > } The rest looks good but that line needs taking out and shooting. Alan ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH 3/5] vt: use copy_to_user instead of __put_user in GIO_UNIMAP ioctl 2017-06-05 16:34 ` Alan Cox @ 2017-06-05 23:05 ` Al Viro 0 siblings, 0 replies; 12+ messages in thread From: Al Viro @ 2017-06-05 23:05 UTC (permalink / raw) To: Alan Cox; +Cc: Adam Borowski, Greg Kroah-Hartman, Jiri Slaby, linux-kernel On Mon, Jun 05, 2017 at 05:34:16PM +0100, Alan Cox wrote: > > @@ -775,13 +775,11 @@ int con_get_unimap(struct vc_data *vc, ushort ct, ushort __user *uct, struct uni > > } > > } > > console_unlock(); > > - for (i = min(ect, ct), plist = unilist; i; i--, list++, plist++) { > > - __put_user(plist->unicode, &list->unicode); > > - __put_user(plist->fontpos, &list->fontpos); > > - } > > - __put_user(ect, uct); > > + if (copy_to_user(list, unilist, min(ect, ct) * sizeof(struct unipair))) > > + ret = -EFAULT; > > + put_user(ect, uct); > > kfree(unilist); > > - return ((ect <= ct) ? 0 : -ENOMEM); > > + return ret ? ret : (ect <= ct) ? 0 : -ENOMEM; > > } > > > The rest looks good but that line needs taking out and shooting. The rest of that function is also Not Nice(tm). Creative indentation, unchecked put_user() result... How about this on top of Adam's commit? One thing I'm not sure about is this -ENOMEM return on overflow; there's no way for caller to tell it from allocation failure, so what's the point copying the list out in that case? I've kept the behaviour as-is, but... diff --git a/drivers/tty/vt/consolemap.c b/drivers/tty/vt/consolemap.c index c6a692f63a9b..73ef478c7e68 100644 --- a/drivers/tty/vt/consolemap.c +++ b/drivers/tty/vt/consolemap.c @@ -731,6 +731,36 @@ int con_copy_unimap(struct vc_data *dst_vc, struct vc_data *src_vc) } EXPORT_SYMBOL(con_copy_unimap); +static int fill_unilist(struct unipair *list, struct uni_pagedir *dir, u16 ct) +{ + int i, j, k, n; + + if (!dir) + return 0; + + for (n = i = 0; i < 32; i++) { + u16 **p1 = dir->uni_pgdir[i]; + if (!p1) + continue; + for (j = 0; j < 32; j++) { + u16 *p2 = p1[j]; + if (!p2) + continue; + for (k = 0; k < 64; k++) { + u16 pos = p2[k]; + if (pos >= MAX_GLYPH) + continue; + if (unlikely(n == ct)) + return -1; + list[n].unicode = (i<<11) + (j<<6) + k; + list[n].fontpos = pos; + n++; + } + } + } + return n; +} + /** * con_get_unimap - get the unicode map * @vc: the console to read from @@ -740,46 +770,29 @@ EXPORT_SYMBOL(con_copy_unimap); */ int con_get_unimap(struct vc_data *vc, ushort ct, ushort __user *uct, struct unipair __user *list) { - int i, j, k, ret = 0; - ushort ect; - u16 **p1, *p2; - struct uni_pagedir *p; struct unipair *unilist; + ushort ect; + int n, ret = 0; unilist = kmalloc_array(ct, sizeof(struct unipair), GFP_KERNEL); if (!unilist) return -ENOMEM; console_lock(); - - ect = 0; - if (*vc->vc_uni_pagedir_loc) { - p = *vc->vc_uni_pagedir_loc; - for (i = 0; i < 32; i++) { - p1 = p->uni_pgdir[i]; - if (p1) - for (j = 0; j < 32; j++) { - p2 = *(p1++); - if (p2) - for (k = 0; k < 64; k++, p2++) { - if (*p2 >= MAX_GLYPH) - continue; - if (ect < ct) { - unilist[ect].unicode = - (i<<11)+(j<<6)+k; - unilist[ect].fontpos = *p2; - } - ect++; - } - } - } - } + n = fill_unilist(unilist, *vc->vc_uni_pagedir_loc, ct); console_unlock(); - if (copy_to_user(list, unilist, min(ect, ct) * sizeof(struct unipair))) + if (n < 0) { + ect = ct; + ret = -ENOMEM; + } else { + ect = n; + ret = 0; + } + if (copy_to_user(list, unilist, ect * sizeof(struct unipair)) || + put_user(ect, uct)) ret = -EFAULT; - put_user(ect, uct); kfree(unilist); - return ret ? ret : (ect <= ct) ? 0 : -ENOMEM; + return ret; } /* ^ permalink raw reply related [flat|nested] 12+ messages in thread
* [PATCH 4/5] vt: use memdup_user in PIO_UNIMAP ioctl 2017-06-03 7:35 ` [PATCH 1/5] vt: use copy_from/to_user instead of __get/put_user for scrnmap ioctls Adam Borowski 2017-06-03 7:35 ` [PATCH 2/5] vt: fix unchecked __put_user() in tioclinux ioctls Adam Borowski 2017-06-03 7:35 ` [PATCH 3/5] vt: use copy_to_user instead of __put_user in GIO_UNIMAP ioctl Adam Borowski @ 2017-06-03 7:35 ` Adam Borowski 2017-06-03 7:35 ` [PATCH 5/5] vt: drop access_ok() calls in unimap ioctls Adam Borowski 3 siblings, 0 replies; 12+ messages in thread From: Adam Borowski @ 2017-06-03 7:35 UTC (permalink / raw) To: Greg Kroah-Hartman, Jiri Slaby, linux-kernel; +Cc: Adam Borowski Again, a nice linear transfer that simplifies the code. Signed-off-by: Adam Borowski <kilobyte@angband.pl> --- drivers/tty/vt/consolemap.c | 11 +++-------- 1 file changed, 3 insertions(+), 8 deletions(-) diff --git a/drivers/tty/vt/consolemap.c b/drivers/tty/vt/consolemap.c index c6a692f63a9b..a5f88cf0f61d 100644 --- a/drivers/tty/vt/consolemap.c +++ b/drivers/tty/vt/consolemap.c @@ -540,14 +540,9 @@ int con_set_unimap(struct vc_data *vc, ushort ct, struct unipair __user *list) if (!ct) return 0; - unilist = kmalloc_array(ct, sizeof(struct unipair), GFP_KERNEL); - if (!unilist) - return -ENOMEM; - - for (i = ct, plist = unilist; i; i--, plist++, list++) { - __get_user(plist->unicode, &list->unicode); - __get_user(plist->fontpos, &list->fontpos); - } + unilist = memdup_user(list, ct * sizeof(struct unipair)); + if (IS_ERR(unilist)) + return PTR_ERR(unilist); console_lock(); -- 2.11.0 ^ permalink raw reply related [flat|nested] 12+ messages in thread
* [PATCH 5/5] vt: drop access_ok() calls in unimap ioctls 2017-06-03 7:35 ` [PATCH 1/5] vt: use copy_from/to_user instead of __get/put_user for scrnmap ioctls Adam Borowski ` (2 preceding siblings ...) 2017-06-03 7:35 ` [PATCH 4/5] vt: use memdup_user in PIO_UNIMAP ioctl Adam Borowski @ 2017-06-03 7:35 ` Adam Borowski 3 siblings, 0 replies; 12+ messages in thread From: Adam Borowski @ 2017-06-03 7:35 UTC (permalink / raw) To: Greg Kroah-Hartman, Jiri Slaby, linux-kernel; +Cc: Adam Borowski Done by copy_{from,to}_user(). Signed-off-by: Adam Borowski <kilobyte@angband.pl> --- drivers/tty/vt/vt_ioctl.c | 8 -------- 1 file changed, 8 deletions(-) diff --git a/drivers/tty/vt/vt_ioctl.c b/drivers/tty/vt/vt_ioctl.c index 0cbfe1ff6f6c..96d389cb506c 100644 --- a/drivers/tty/vt/vt_ioctl.c +++ b/drivers/tty/vt/vt_ioctl.c @@ -266,10 +266,6 @@ do_unimap_ioctl(int cmd, struct unimapdesc __user *user_ud, int perm, struct vc_ if (copy_from_user(&tmp, user_ud, sizeof tmp)) return -EFAULT; - if (tmp.entries) - if (!access_ok(VERIFY_WRITE, tmp.entries, - tmp.entry_ct*sizeof(struct unipair))) - return -EFAULT; switch (cmd) { case PIO_UNIMAP: if (!perm) @@ -1170,10 +1166,6 @@ compat_unimap_ioctl(unsigned int cmd, struct compat_unimapdesc __user *user_ud, if (copy_from_user(&tmp, user_ud, sizeof tmp)) return -EFAULT; tmp_entries = compat_ptr(tmp.entries); - if (tmp_entries) - if (!access_ok(VERIFY_WRITE, tmp_entries, - tmp.entry_ct*sizeof(struct unipair))) - return -EFAULT; switch (cmd) { case PIO_UNIMAP: if (!perm) -- 2.11.0 ^ permalink raw reply related [flat|nested] 12+ messages in thread
* Re: [PATCH 0/5] vt: get rid of worst cases of __put_user/__get_user 2017-06-03 7:32 [PATCH 0/5] vt: get rid of worst cases of __put_user/__get_user Adam Borowski 2017-06-03 7:35 ` [PATCH 1/5] vt: use copy_from/to_user instead of __get/put_user for scrnmap ioctls Adam Borowski @ 2017-06-03 15:42 ` Greg Kroah-Hartman 2017-06-05 6:13 ` Al Viro 1 sibling, 1 reply; 12+ messages in thread From: Greg Kroah-Hartman @ 2017-06-03 15:42 UTC (permalink / raw) To: Adam Borowski; +Cc: Jiri Slaby, linux-kernel On Sat, Jun 03, 2017 at 09:32:55AM +0200, Adam Borowski wrote: > Hi! > In a recent discussion, Linus and Al Viro said quite a bit of expletives > about __put_user() and __get_user(), that it's a bad interface that's > almost always the wrong thing to use: > https://marc.info/?l=linux-kernel&m=149463725626316&w=2 > https://marc.info/?l=linux-kernel&m=149465866929092&w=2 > > Here's a few patches applying the lessons from that discussion to vt. > None of the uses is performance-critical, but at least we get a nice bit > of code simplification. And, it's a start of manual review + conversion > that Al Viro wants. Ah, nice work, at first glance these all look good to me. I'll queue them up on Monday. greg k-h ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH 0/5] vt: get rid of worst cases of __put_user/__get_user 2017-06-03 15:42 ` [PATCH 0/5] vt: get rid of worst cases of __put_user/__get_user Greg Kroah-Hartman @ 2017-06-05 6:13 ` Al Viro 2017-06-05 6:41 ` Greg Kroah-Hartman 2017-06-09 9:18 ` Greg Kroah-Hartman 0 siblings, 2 replies; 12+ messages in thread From: Al Viro @ 2017-06-05 6:13 UTC (permalink / raw) To: Greg Kroah-Hartman; +Cc: Adam Borowski, Jiri Slaby, linux-kernel On Sun, Jun 04, 2017 at 12:42:52AM +0900, Greg Kroah-Hartman wrote: > On Sat, Jun 03, 2017 at 09:32:55AM +0200, Adam Borowski wrote: > > Hi! > > In a recent discussion, Linus and Al Viro said quite a bit of expletives > > about __put_user() and __get_user(), that it's a bad interface that's > > almost always the wrong thing to use: > > https://marc.info/?l=linux-kernel&m=149463725626316&w=2 > > https://marc.info/?l=linux-kernel&m=149465866929092&w=2 > > > > Here's a few patches applying the lessons from that discussion to vt. > > None of the uses is performance-critical, but at least we get a nice bit > > of code simplification. And, it's a start of manual review + conversion > > that Al Viro wants. > > Ah, nice work, at first glance these all look good to me. I'll queue > them up on Monday. Could you put that into a separate no-rebase branch? Or I could do that in vfs.git, for that matter... ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH 0/5] vt: get rid of worst cases of __put_user/__get_user 2017-06-05 6:13 ` Al Viro @ 2017-06-05 6:41 ` Greg Kroah-Hartman 2017-06-09 9:18 ` Greg Kroah-Hartman 1 sibling, 0 replies; 12+ messages in thread From: Greg Kroah-Hartman @ 2017-06-05 6:41 UTC (permalink / raw) To: Al Viro; +Cc: Adam Borowski, Jiri Slaby, linux-kernel On Mon, Jun 05, 2017 at 07:13:50AM +0100, Al Viro wrote: > On Sun, Jun 04, 2017 at 12:42:52AM +0900, Greg Kroah-Hartman wrote: > > On Sat, Jun 03, 2017 at 09:32:55AM +0200, Adam Borowski wrote: > > > Hi! > > > In a recent discussion, Linus and Al Viro said quite a bit of expletives > > > about __put_user() and __get_user(), that it's a bad interface that's > > > almost always the wrong thing to use: > > > https://marc.info/?l=linux-kernel&m=149463725626316&w=2 > > > https://marc.info/?l=linux-kernel&m=149465866929092&w=2 > > > > > > Here's a few patches applying the lessons from that discussion to vt. > > > None of the uses is performance-critical, but at least we get a nice bit > > > of code simplification. And, it's a start of manual review + conversion > > > that Al Viro wants. > > > > Ah, nice work, at first glance these all look good to me. I'll queue > > them up on Monday. > > Could you put that into a separate no-rebase branch? Or I could do that > in vfs.git, for that matter... If you just want to take these in vfs.git feel free to, I don't mind at all: Acked-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org> that's probably the easiest. thanks, greg k-h ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH 0/5] vt: get rid of worst cases of __put_user/__get_user 2017-06-05 6:13 ` Al Viro 2017-06-05 6:41 ` Greg Kroah-Hartman @ 2017-06-09 9:18 ` Greg Kroah-Hartman 1 sibling, 0 replies; 12+ messages in thread From: Greg Kroah-Hartman @ 2017-06-09 9:18 UTC (permalink / raw) To: Al Viro; +Cc: Adam Borowski, Jiri Slaby, linux-kernel On Mon, Jun 05, 2017 at 07:13:50AM +0100, Al Viro wrote: > On Sun, Jun 04, 2017 at 12:42:52AM +0900, Greg Kroah-Hartman wrote: > > On Sat, Jun 03, 2017 at 09:32:55AM +0200, Adam Borowski wrote: > > > Hi! > > > In a recent discussion, Linus and Al Viro said quite a bit of expletives > > > about __put_user() and __get_user(), that it's a bad interface that's > > > almost always the wrong thing to use: > > > https://marc.info/?l=linux-kernel&m=149463725626316&w=2 > > > https://marc.info/?l=linux-kernel&m=149465866929092&w=2 > > > > > > Here's a few patches applying the lessons from that discussion to vt. > > > None of the uses is performance-critical, but at least we get a nice bit > > > of code simplification. And, it's a start of manual review + conversion > > > that Al Viro wants. > > > > Ah, nice work, at first glance these all look good to me. I'll queue > > them up on Monday. > > Could you put that into a separate no-rebase branch? Or I could do that > in vfs.git, for that matter... Yes, here's a tag/branch for you to pull from that will not go away until 4.13-rc1. The following changes since commit 3c2993b8c6143d8a5793746a54eba8f86f95240f: Linux 4.12-rc4 (2017-06-04 16:47:43 -0700) are available in the git repository at: git://git.kernel.org/pub/scm/linux/kernel/git/gregkh/tty.git/ tags/vt_copy_cleanup_tag for you to fetch changes up to f8564c93e0907651e21d586920e629227bb0d024: vt: drop access_ok() calls in unimap ioctls (2017-06-09 11:07:36 +0200) ---------------------------------------------------------------- vt: copy/from_to cleanup for vt code for Al to pull from. Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org> ---------------------------------------------------------------- Adam Borowski (5): vt: use copy_from/to_user instead of __get/put_user for scrnmap ioctls vt: fix unchecked __put_user() in tioclinux ioctls vt: use copy_to_user instead of __put_user in GIO_UNIMAP ioctl vt: use memdup_user in PIO_UNIMAP ioctl vt: drop access_ok() calls in unimap ioctls drivers/tty/vt/consolemap.c | 56 +++++++++++++-------------------------------- drivers/tty/vt/vt.c | 6 ++--- drivers/tty/vt/vt_ioctl.c | 8 ------- 3 files changed, 19 insertions(+), 51 deletions(-) ^ permalink raw reply [flat|nested] 12+ messages in thread
end of thread, other threads:[~2017-06-09 9:19 UTC | newest] Thread overview: 12+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2017-06-03 7:32 [PATCH 0/5] vt: get rid of worst cases of __put_user/__get_user Adam Borowski 2017-06-03 7:35 ` [PATCH 1/5] vt: use copy_from/to_user instead of __get/put_user for scrnmap ioctls Adam Borowski 2017-06-03 7:35 ` [PATCH 2/5] vt: fix unchecked __put_user() in tioclinux ioctls Adam Borowski 2017-06-03 7:35 ` [PATCH 3/5] vt: use copy_to_user instead of __put_user in GIO_UNIMAP ioctl Adam Borowski 2017-06-05 16:34 ` Alan Cox 2017-06-05 23:05 ` Al Viro 2017-06-03 7:35 ` [PATCH 4/5] vt: use memdup_user in PIO_UNIMAP ioctl Adam Borowski 2017-06-03 7:35 ` [PATCH 5/5] vt: drop access_ok() calls in unimap ioctls Adam Borowski 2017-06-03 15:42 ` [PATCH 0/5] vt: get rid of worst cases of __put_user/__get_user Greg Kroah-Hartman 2017-06-05 6:13 ` Al Viro 2017-06-05 6:41 ` Greg Kroah-Hartman 2017-06-09 9:18 ` Greg Kroah-Hartman
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.