|
Message-ID: <878tp3j12z.fsf@concordia.ellerman.id.au> Date: Sat, 18 Feb 2017 20:32:52 +1100 From: Michael Ellerman <mpe@...erman.id.au> To: Kees Cook <keescook@...omium.org>, Geert Uytterhoeven <geert@...ux-m68k.org> Cc: "linux-kernel\@vger.kernel.org" <linux-kernel@...r.kernel.org>, Hoeun Ryu <hoeun.ryu@...il.com>, "kernel-hardening\@lists.openwall.com" <kernel-hardening@...ts.openwall.com> Subject: Re: Re: [PATCH] usercopy: Add tests for all get_user() sizes Kees Cook <keescook@...omium.org> writes: > On Wed, Feb 15, 2017 at 12:50 AM, Geert Uytterhoeven > <geert@...ux-m68k.org> wrote: >> On Tue, Feb 14, 2017 at 9:40 PM, Kees Cook <keescook@...omium.org> wrote: >>> The existing test was only exercising native unsigned long size >>> get_user(). For completeness, we should check all sizes. >>> >>> Signed-off-by: Kees Cook <keescook@...omium.org> >>> --- >>> lib/test_user_copy.c | 45 ++++++++++++++++++++++++++++++++++----------- >>> 1 file changed, 34 insertions(+), 11 deletions(-) >>> >>> diff --git a/lib/test_user_copy.c b/lib/test_user_copy.c >>> index ac3a60ba9331..49569125b7c5 100644 >>> --- a/lib/test_user_copy.c >>> +++ b/lib/test_user_copy.c >>> @@ -40,8 +40,11 @@ static int __init test_user_copy_init(void) >>> char __user *usermem; >>> char *bad_usermem; >>> unsigned long user_addr; >>> - unsigned long value = 0x5A; >>> char *zerokmem; >>> + u8 val_u8; >>> + u16 val_u16; >>> + u32 val_u32; >>> + u64 val_u64; >>> >>> kmem = kmalloc(PAGE_SIZE * 2, GFP_KERNEL); >>> if (!kmem) >>> @@ -72,10 +75,20 @@ static int __init test_user_copy_init(void) >>> "legitimate copy_from_user failed"); >>> ret |= test(copy_to_user(usermem, kmem, PAGE_SIZE), >>> "legitimate copy_to_user failed"); >>> - ret |= test(get_user(value, (unsigned long __user *)usermem), >>> - "legitimate get_user failed"); >>> - ret |= test(put_user(value, (unsigned long __user *)usermem), >>> - "legitimate put_user failed"); >>> + >>> +#define test_legit(size) \ >>> + do { \ >>> + ret |= test(get_user(val_##size, (size __user *)usermem), \ >>> + "legitimate get_user (" #size ") failed"); \ >>> + ret |= test(put_user(val_##size, (size __user *)usermem), \ >>> + "legitimate put_user (" #size ") failed"); \ >>> + } while (0) >>> + >>> + test_legit(u8); >>> + test_legit(u16); >>> + test_legit(u32); >>> + test_legit(u64); >>> +#undef test_legit >> >> ERROR: "__get_user_bad" [lib/test_user_copy.ko] undefined! >> >> http://kisskb.ellerman.id.au/kisskb/buildresult/12936728/ >> >> So 64-bit get_user() support is mandatory now? > > That's not my intention. :) In my sampling of architectures, I missed > a couple 32-bit archs that don't support 64-bit getuser(). I'm not > sure how to correctly write these tests, though, since it seems rather > ad-hoc. e.g. m68k has 64-bit getuser() commented out due to an old gcc > bug... > > Should I just universally skip 64-bit getuser on 32-bit archs? I think you should just make it opt-in for 32-bit arches. cheers
Powered by blists - more mailing lists
Confused about mailing lists and their use? Read about mailing lists on Wikipedia and check out these guidelines on proper formatting of your messages.