linux-kernel - RE: [PATCH v2 2/9] selinux: use u32 as bit type in ebitmap code

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: <52329f0195a549d1b7abb3417cb2c225@AcuMS.aculab.com>
Date:   Fri, 4 Aug 2023 15:11:49 +0000
From:   David Laight <David.Laight@...LAB.COM>
To:     'Paul Moore' <paul@...l-moore.com>,
        Christian Göttsche <cgzones@...glemail.com>,
        "selinux@...r.kernel.org" <selinux@...r.kernel.org>
CC:     Stephen Smalley <stephen.smalley.work@...il.com>,
        Eric Paris <eparis@...isplace.org>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>
Subject: RE: [PATCH v2 2/9] selinux: use u32 as bit type in ebitmap code

From: Paul Moore
> Sent: 04 August 2023 03:20
> 
> On Jul 28, 2023 =?UTF-8?q?Christian=20G=C3=B6ttsche?= <cgzones@...glemail.com> wrote:
....
> > +	last_startbit = (u32)-1;
> 
> I can't say I'm as current on all of the C standards and compilier
> oddities as some other in the Linux kernel space, but my
> understanding is that on assignment the right value is always
> implicitly type cast to the type of the left variable, is that not
> true?  Assuming it is true, I think this explicit cast isn't
> necessary and could actually be harmful if we need to change the
> ebitmap types in the future.

The only question is where any required sign extend happens.
If you do:
	u64 val = -1;
then the signed int is first sign extended to 64 bit and then
converted to unsigned (which just copies the bit pattern on any
sane system that Linux might run on).
Whereas:
	u64 val = (u32)-1;
Converts an (assumed) 32bit -1 to unsigned and then zero extends it.

What you should really be using is a named constant that is
(for the current implementation) (~0u) and doesn't ever need
any casts and is always unsigned.

If you are actually worried about 'int' being other than 32bits
then there will be a lot more places that need fixing.

But you could use ((u32)~(u32)0) if you really want to allow
for 'u32' being both smaller and larger than 'int' and for
non 2's compliment (eg 1's compliment and sign overpunch)
systems.
(Good luck on finding a working C compiler for either of those.)

	David

-
Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, UK
Registration No: 1397386 (Wales)