[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <20190403172415.GD17500@fuggles.cambridge.arm.com>
Date: Wed, 3 Apr 2019 18:24:15 +0100
From: Will Deacon <will.deacon@....com>
To: Christian Brauner <christian@...uner.io>
Cc: linux-kernel@...r.kernel.org, Kees Cook <keescook@...omium.org>,
Andrew Morton <akpm@...ux-foundation.org>, croce@...hat.com
Subject: Re: [PATCH] kernel/sysctl.c: Fix out-of-bounds access when setting
file-max
Hi Christian,
On Wed, Apr 03, 2019 at 05:40:45PM +0200, Christian Brauner wrote:
> On Wed, Apr 03, 2019 at 04:34:09PM +0100, Will Deacon wrote:
> > Commit 32a5ad9c2285 ("sysctl: handle overflow for file-max") hooked
> > up min/max values for the file-max sysctl parameter via the .extra1
> > and .extra2 fields in the corresponding struct ctl_table entry.
> >
> > Unfortunately, the minimum value points at the global 'zero' variable,
> > which is an int. This results in a KASAN splat when accessed as a long
> > by proc_doulongvec_minmax on 64-bit architectures:
> >
> > | BUG: KASAN: global-out-of-bounds in __do_proc_doulongvec_minmax+0x5d8/0x6a0
> > | Read of size 8 at addr ffff2000133d1c20 by task systemd/1
> > |
> > | CPU: 0 PID: 1 Comm: systemd Not tainted 5.1.0-rc3-00012-g40b114779944 #2
> > | Hardware name: linux,dummy-virt (DT)
> > | Call trace:
> > | dump_backtrace+0x0/0x228
> > | show_stack+0x14/0x20
> > | dump_stack+0xe8/0x124
> > | print_address_description+0x60/0x258
> > | kasan_report+0x140/0x1a0
> > | __asan_report_load8_noabort+0x18/0x20
> > | __do_proc_doulongvec_minmax+0x5d8/0x6a0
> > | proc_doulongvec_minmax+0x4c/0x78
> > | proc_sys_call_handler.isra.19+0x144/0x1d8
> > | proc_sys_write+0x34/0x58
> > | __vfs_write+0x54/0xe8
> > | vfs_write+0x124/0x3c0
> > | ksys_write+0xbc/0x168
> > | __arm64_sys_write+0x68/0x98
> > | el0_svc_common+0x100/0x258
> > | el0_svc_handler+0x48/0xc0
> > | el0_svc+0x8/0xc
> > |
> > | The buggy address belongs to the variable:
> > | zero+0x0/0x40
> > |
> > | Memory state around the buggy address:
> > | ffff2000133d1b00: 00 00 00 00 00 00 00 00 fa fa fa fa 04 fa fa fa
> > | ffff2000133d1b80: fa fa fa fa 04 fa fa fa fa fa fa fa 04 fa fa fa
> > | >ffff2000133d1c00: fa fa fa fa 04 fa fa fa fa fa fa fa 00 00 00 00
> > | ^
> > | ffff2000133d1c80: fa fa fa fa 00 fa fa fa fa fa fa fa 00 00 00 00
> > | ffff2000133d1d00: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
> >
> > Fix the splat by introducing a unsigned long 'zero_ul' and using that
> > instead.
> >
> > Fixes: 32a5ad9c2285 ("sysctl: handle overflow for file-max")
> > Cc: Christian Brauner <christian@...uner.io>
> > Cc: Kees Cook <keescook@...omium.org>
> > Cc: Andrew Morton <akpm@...ux-foundation.org>
> > Signed-off-by: Will Deacon <will.deacon@....com>
>
> Hey Will, thanks! For the record, there's another patch by Matteo (Cced)
> for the same thing:
> https://lore.kernel.org/lkml/20190328130306.25384-1-mcroce@redhat.com/
Oops, sorry, I didn't spot that. I just ran into this while I was using
KASAN to investigate a different issue and quickly sent out a fix.
> He's proposing a slightly different version for the fix. I don't care
> very much but I like that you've used the explicit unsigned long so:
>
> Acked-by: Christian Brauner <christian@...uner.io>
>
> Thanks to both of you for fixing this!
I'm also not bothered about which patch gets in, as long as the problem is
fixed. I assume somebody will pick up one of them...
Will
Powered by blists - more mailing lists