netdev - Re: [PATCH 2/9] lib/cmdline: Fix an invalid format specifier in an assertion msg

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

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <CAHk-=wiODww51Kz-TTWn0ka5T8oMtt0AfbO9t0U3iJqfLZO+8w@mail.gmail.com>
Date: Thu, 22 Feb 2024 09:56:40 -0800
From: Linus Torvalds <torvalds@...ux-foundation.org>
To: Daniel Latypov <dlatypov@...gle.com>
Cc: David Gow <davidgow@...gle.com>, Justin Stitt <justinstitt@...gle.com>, 
	Shuah Khan <skhan@...uxfoundation.org>, Guenter Roeck <linux@...ck-us.net>, 
	Rae Moar <rmoar@...gle.com>, Matthew Auld <matthew.auld@...el.com>, 
	Arunpravin Paneer Selvam <arunpravin.paneerselvam@....com>, 
	Christian König <christian.koenig@....com>, 
	Kees Cook <keescook@...omium.org>, Maíra Canal <mcanal@...lia.com>, 
	Rodrigo Vivi <rodrigo.vivi@...el.com>, Matthew Brost <matthew.brost@...el.com>, 
	Willem de Bruijn <willemb@...gle.com>, Florian Westphal <fw@...len.de>, Cassio Neri <cassio.neri@...il.com>, 
	Javier Martinez Canillas <javierm@...hat.com>, Arthur Grillo <arthur.grillo@....br>, 
	Brendan Higgins <brendan.higgins@...ux.dev>, Stephen Boyd <sboyd@...nel.org>, 
	David Airlie <airlied@...il.com>, Maxime Ripard <mripard@...nel.org>, 
	"David S . Miller" <davem@...emloft.net>, dri-devel@...ts.freedesktop.org, 
	linux-kernel@...r.kernel.org, intel-xe@...ts.freedesktop.org, 
	linux-rtc@...r.kernel.org, linux-kselftest@...r.kernel.org, 
	kunit-dev@...glegroups.com, linux-hardening@...r.kernel.org, 
	netdev@...r.kernel.org
Subject: Re: [PATCH 2/9] lib/cmdline: Fix an invalid format specifier in an
 assertion msg

On Thu, 22 Feb 2024 at 09:36, Daniel Latypov <dlatypov@...gle.com> wrote:
>
> Copying the line for context, it's about `p-r` where
>   p = memchr_inv(&r[1], 0, sizeof(r) - sizeof(r[0]));
> `p-r` should never be negative unless something has gone horribly
> horribly wrong.

Sure it would - if 'p' is NULL.

Of course, then a negative value wouldn't be helpful either, and in
this case that's what the EXPECT_PTR_EQ checking is testing in the
first place, so it's a non-issue.

IOW, in practice clearly the sign should simply not matter here.

I do think that the default case for pointer differences should be
that they are signed, because they *can* be.

Just because of that "default case", unless there's some actual reason
to use '%tu', I think '%td' should be seen as the normal case to use.

That said, just as a quick aside: be careful with pointer differences
in the kernel.

For this particular case, when we're talking about just 'char *', it's
not a big deal, but we've had code where people didn't think about
what it means to do a pointer difference in C, and how it can be often
unnecessarily expensive due to the implied "divide by the size of the
pointed object".

Sometimes it's actually worth writing the code in ways that avoids
pointer differences entirely (which might involve passing around
indexes instead of pointers).

                 Linus