lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
Open Source and information security mailing list archives
| ||
|
Date: Wed, 19 Feb 2020 14:56:32 +0100 From: Rasmus Villemoes <linux@...musvillemoes.dk> To: Petr Mladek <pmladek@...e.com> Cc: Ilya Dryomov <idryomov@...il.com>, Steven Rostedt <rostedt@...dmis.org>, Sergey Senozhatsky <sergey.senozhatsky@...il.com>, Andy Shevchenko <andriy.shevchenko@...ux.intel.com>, Jonathan Corbet <corbet@....net>, Kees Cook <keescook@...omium.org>, "Tobin C . Harding" <me@...in.cc>, Linus Torvalds <torvalds@...ux-foundation.org>, linux-doc@...r.kernel.org, LKML <linux-kernel@...r.kernel.org> Subject: Re: [PATCH] vsprintf: sanely handle NULL passed to %pe On 19/02/2020 14.48, Petr Mladek wrote: > On Wed 2020-02-19 12:53:22, Rasmus Villemoes wrote: >> --- a/lib/vsprintf.c >> +++ b/lib/vsprintf.c >> @@ -619,7 +619,7 @@ static char *err_ptr(char *buf, char *end, void *ptr, >> struct printf_spec spec) >> { >> int err = PTR_ERR(ptr); >> - const char *sym = errname(err); >> + const char *sym = err ? errname(err) : "NULL"; > > I like this more than adding "NULL" errname. OK. >> if (sym) >> return string_nocheck(buf, end, sym, spec); >> >> instead of the change(s) in errname.c? And then the test case for >> '"%pe", NULL' should also be moved outside CONFIG_SYMBOLIC_ERRNAME. > > The test should go into null_pointer() instead of errptr(). Eh, no, the behaviour of %pe is tested by errptr(). I'll keep it that way. But I should add a #else section that tests how %pe behaves without CONFIG_SYMBOLIC_ERRNAME - though that's orthogonal to this patch. > Could you send updated patch, please? ;-) I'll wait a day or two for more comments. It doesn't seem very urgent. >> BTW., your original patch for %p lacks corresponding update of >> test_vsprintf.c. Please add appropriate test cases. > > Good point. The existing test_hashed() is rather weak > and it did not catch this change. > > It would be nice to make test_hash() more powerful. > Anyway, the minimal udpate would be: > > diff --git a/lib/test_printf.c b/lib/test_printf.c > index 2d9f520d2f27..1726a678bccd 100644 > --- a/lib/test_printf.c > +++ b/lib/test_printf.c > @@ -333,7 +333,7 @@ test_hashed(const char *fmt, const void *p) > static void __init > null_pointer(void) > { > - test_hashed("%p", NULL); > + test(ZEROS "00000000", "%p", NULL); No, it most certainly also needs to check a few "%p", ERR_PTR(-4) cases (where one of course has to use explicit integers and not E* constants). Rasmus
Powered by blists - more mailing lists