Re: [PATCH] compat: fix possible out-of-bound accesses in compat_get_bitmap() and compat_put_bitmap()

2015-06-06 Thread Linus Torvalds
On Fri, Jun 5, 2015 at 11:24 AM, Al Viro wrote: > > Yes, but... if we decrement that sucker at all, why do we need to play with > i at all? We need exactly nr_compat_longs get_user(), so why not make _that_ > the condition in the single-level loop? Yeah, I think that code could be clarified

Re: [PATCH] compat: fix possible out-of-bound accesses in compat_get_bitmap() and compat_put_bitmap()

2015-06-06 Thread Linus Torvalds
On Fri, Jun 5, 2015 at 11:24 AM, Al Viro v...@zeniv.linux.org.uk wrote: Yes, but... if we decrement that sucker at all, why do we need to play with i at all? We need exactly nr_compat_longs get_user(), so why not make _that_ the condition in the single-level loop? Yeah, I think that code

Re: [PATCH] compat: fix possible out-of-bound accesses in compat_get_bitmap() and compat_put_bitmap()

2015-06-05 Thread Al Viro
On Fri, Jun 05, 2015 at 09:49:46AM -0700, Linus Torvalds wrote: > On Thu, Jun 4, 2015 at 5:36 PM, Al Viro wrote: > > > > So basically that thing will trigger only on the last pass through > > the outer loop. The only way for it to trigger a wraparound would > > be to have

Re: [PATCH] compat: fix possible out-of-bound accesses in compat_get_bitmap() and compat_put_bitmap()

2015-06-05 Thread Linus Torvalds
On Thu, Jun 4, 2015 at 5:36 PM, Al Viro wrote: > > So basically that thing will trigger only on the last pass through > the outer loop. The only way for it to trigger a wraparound would > be to have sizeof(long)/sizeof(compat_long_t) greater than LONG_MAX, > which is, not too likely. No. You

Re: [PATCH] compat: fix possible out-of-bound accesses in compat_get_bitmap() and compat_put_bitmap()

2015-06-05 Thread Al Viro
On Fri, Jun 05, 2015 at 09:49:46AM -0700, Linus Torvalds wrote: On Thu, Jun 4, 2015 at 5:36 PM, Al Viro v...@zeniv.linux.org.uk wrote: So basically that thing will trigger only on the last pass through the outer loop. The only way for it to trigger a wraparound would be to have

Re: [PATCH] compat: fix possible out-of-bound accesses in compat_get_bitmap() and compat_put_bitmap()

2015-06-05 Thread Linus Torvalds
On Thu, Jun 4, 2015 at 5:36 PM, Al Viro v...@zeniv.linux.org.uk wrote: So basically that thing will trigger only on the last pass through the outer loop. The only way for it to trigger a wraparound would be to have sizeof(long)/sizeof(compat_long_t) greater than LONG_MAX, which is, not too

Re: [PATCH] compat: fix possible out-of-bound accesses in compat_get_bitmap() and compat_put_bitmap()

2015-06-04 Thread Al Viro
On Fri, Jun 05, 2015 at 12:07:43AM +0200, Helge Deller wrote: > In the functions compat_get_bitmap() and compat_put_bitmap() the variable > nr_compat_longs stores how many compat_ulong_t words should be copied in a > loop. > > The copy loop itself is this: > if (nr_compat_longs-- > 0) { >

Re: [PATCH] compat: fix possible out-of-bound accesses in compat_get_bitmap() and compat_put_bitmap()

2015-06-04 Thread Helge Deller
Hi Linus, * Linus Torvalds : > On Thu, Jun 4, 2015 at 6:45 AM, Helge Deller wrote: > > > > Do you want me to send it again cleaned up, or will you just take yours? > > I'd prefer to get a re-send, I've already nuked the patch from me tree. Sure. The new patch is attached below. If you think

Re: [PATCH] compat: fix possible out-of-bound accesses in compat_get_bitmap() and compat_put_bitmap()

2015-06-04 Thread Linus Torvalds
On Thu, Jun 4, 2015 at 6:45 AM, Helge Deller wrote: > > Do you want me to send it again cleaned up, or will you just take yours? I'd prefer to get a re-send, I've already nuked the patch from me tree. These days, 99% of the patches I write are throw-away stuff just for this kind of "how about

Re: [PATCH] compat: fix possible out-of-bound accesses in compat_get_bitmap() and compat_put_bitmap()

2015-06-04 Thread Helge Deller
Hi Linus, On 02.06.2015 03:49, Linus Torvalds wrote: On Mon, Jun 1, 2015 at 11:44 AM, Helge Deller wrote: Since nr_compat_longs gets unconditionally decremented in each loop, it's type needs to be signed instead of unsigned to avoid possibly accessing userspace memory behind the bitmap which

Re: [PATCH] compat: fix possible out-of-bound accesses in compat_get_bitmap() and compat_put_bitmap()

2015-06-04 Thread Helge Deller
Hi Linus, * Linus Torvalds torva...@linux-foundation.org: On Thu, Jun 4, 2015 at 6:45 AM, Helge Deller del...@gmx.de wrote: Do you want me to send it again cleaned up, or will you just take yours? I'd prefer to get a re-send, I've already nuked the patch from me tree. Sure. The new patch

Re: [PATCH] compat: fix possible out-of-bound accesses in compat_get_bitmap() and compat_put_bitmap()

2015-06-04 Thread Al Viro
On Fri, Jun 05, 2015 at 12:07:43AM +0200, Helge Deller wrote: In the functions compat_get_bitmap() and compat_put_bitmap() the variable nr_compat_longs stores how many compat_ulong_t words should be copied in a loop. The copy loop itself is this: if (nr_compat_longs-- 0) { if

Re: [PATCH] compat: fix possible out-of-bound accesses in compat_get_bitmap() and compat_put_bitmap()

2015-06-04 Thread Linus Torvalds
On Thu, Jun 4, 2015 at 6:45 AM, Helge Deller del...@gmx.de wrote: Do you want me to send it again cleaned up, or will you just take yours? I'd prefer to get a re-send, I've already nuked the patch from me tree. These days, 99% of the patches I write are throw-away stuff just for this kind of

Re: [PATCH] compat: fix possible out-of-bound accesses in compat_get_bitmap() and compat_put_bitmap()

2015-06-04 Thread Helge Deller
Hi Linus, On 02.06.2015 03:49, Linus Torvalds wrote: On Mon, Jun 1, 2015 at 11:44 AM, Helge Deller del...@gmx.de wrote: Since nr_compat_longs gets unconditionally decremented in each loop, it's type needs to be signed instead of unsigned to avoid possibly accessing userspace memory behind the

Re: [PATCH] compat: fix possible out-of-bound accesses in compat_get_bitmap() and compat_put_bitmap()

2015-06-01 Thread Linus Torvalds
On Mon, Jun 1, 2015 at 11:44 AM, Helge Deller wrote: > > Since nr_compat_longs gets unconditionally decremented in each loop, it's type > needs to be signed instead of unsigned to avoid possibly accessing userspace > memory behind the bitmap which shouldn't be accessed. I'd actually prefer to

[PATCH] compat: fix possible out-of-bound accesses in compat_get_bitmap() and compat_put_bitmap()

2015-06-01 Thread Helge Deller
In functions compat_get_bitmap() and compat_put_bitmap() the variable nr_compat_longs stores how many compat_ulong_t words should be copied in a loop. The copy-loop itself is this: if (nr_compat_longs-- > 0) { if (__get_user(um, umask)) return -EFAULT; } else { um = 0; } Since

[PATCH] compat: fix possible out-of-bound accesses in compat_get_bitmap() and compat_put_bitmap()

2015-06-01 Thread Helge Deller
In functions compat_get_bitmap() and compat_put_bitmap() the variable nr_compat_longs stores how many compat_ulong_t words should be copied in a loop. The copy-loop itself is this: if (nr_compat_longs-- 0) { if (__get_user(um, umask)) return -EFAULT; } else { um = 0; } Since

Re: [PATCH] compat: fix possible out-of-bound accesses in compat_get_bitmap() and compat_put_bitmap()

2015-06-01 Thread Linus Torvalds
On Mon, Jun 1, 2015 at 11:44 AM, Helge Deller del...@gmx.de wrote: Since nr_compat_longs gets unconditionally decremented in each loop, it's type needs to be signed instead of unsigned to avoid possibly accessing userspace memory behind the bitmap which shouldn't be accessed. I'd actually