Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
From:   David Laight <David.Laight@ACULAB.COM>
To:     'Paul Moore' <paul@paul-moore.com>,
        =?iso-8859-1?Q?Christian_G=F6ttsche?= <cgzones@googlemail.com>,
        "selinux@vger.kernel.org" <selinux@vger.kernel.org>
CC:     Stephen Smalley <stephen.smalley.work@gmail.com>,
        Eric Paris <eparis@parisplace.org>,
        "linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>
Subject: RE: [PATCH v2 2/9] selinux: use u32 as bit type in ebitmap code
Thread-Topic: [PATCH v2 2/9] selinux: use u32 as bit type in ebitmap code
Thread-Index: AQHZxno7WAML9teEJUiVyNIDpo4Zsa/aOz6A
Date:   Fri, 4 Aug 2023 15:11:49 +0000
Message-ID: <52329f0195a549d1b7abb3417cb2c225@AcuMS.aculab.com>
References: <20230728155501.39632-1-cgzones@googlemail.com>
 <c8f7b16afb26b2357fdc2b590a8cdcba.paul@paul-moore.com>
In-Reply-To: <c8f7b16afb26b2357fdc2b590a8cdcba.paul@paul-moore.com>
Accept-Language: en-GB, en-US
MIME-Version: 1.0
Content-Language: en-US
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8BIT
Precedence: bulk

From: Paul Moore
> Sent: 04 August 2023 03:20
> 
> On Jul 28, 2023 =?UTF-8?q?Christian=20G=C3=B6ttsche?= <cgzones@googlemail.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)