linux-kernel - Re: inet: frags: Turn fqdir->dead into an int for old Alphas

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] [day] [month] [year] [list]

Date:   Sat, 8 Jun 2019 11:50:19 -0700
From:   "Paul E. McKenney" <paulmck@...ux.ibm.com>
To:     Linus Torvalds <torvalds@...ux-foundation.org>
Cc:     Eric Dumazet <eric.dumazet@...il.com>,
        Herbert Xu <herbert@...dor.apana.org.au>,
        Alan Stern <stern@...land.harvard.edu>,
        Boqun Feng <boqun.feng@...il.com>,
        Frederic Weisbecker <fweisbec@...il.com>,
        Fengguang Wu <fengguang.wu@...el.com>, LKP <lkp@...org>,
        LKML <linux-kernel@...r.kernel.org>,
        Netdev <netdev@...r.kernel.org>,
        "David S. Miller" <davem@...emloft.net>,
        Andrea Parri <andrea.parri@...rulasolutions.com>,
        Luc Maranget <luc.maranget@...ia.fr>,
        Jade Alglave <j.alglave@....ac.uk>
Subject: Re: inet: frags: Turn fqdir->dead into an int for old Alphas

On Sat, Jun 08, 2019 at 10:50:51AM -0700, Linus Torvalds wrote:
> On Sat, Jun 8, 2019 at 10:42 AM Linus Torvalds
> <torvalds@...ux-foundation.org> wrote:
> >
> > There are no atomic rmw sequences that have reasonable performance for
> > the bitfield updates themselves.
> 
> Note that this is purely about the writing side. Reads of bitfield
> values can be (and generally _should_ be) atomic, and hopefully C11
> means that you wouldn't see intermediate values.
> 
> But I'm not convinced about that either: one natural way to update a
> bitfield is to first do the masking, and then do the insertion of new
> bits, so a bitfield assignment very easily exposes non-real values to
> a concurrent read on another CPU.

Agreed on the "not convinced" part (though perhaps most implementations
would handle concurrent reads and writes involving different fields of
the same bitfield).  And the C standard does not guarantee this, because
data races are defined in terms of memory locations.  So as far as the
C standard is concerned, if there are two concurrent accesses to fields
within a bitfield that are not separated by ":0", there is a data race
and so the compiler can do whatever it wants.

But do we really care about this case?

> What I think C11 is supposed to protect is from compilers doing
> horribly bad things, and accessing bitfields with bigger types than
> the field itself, ie when you have
> 
>    struct {
>        char c;
>        int field1:5;
>    };
> 
> then a write to "field1" had better not touch "char c" as part of the
> rmw operation, because that would indeed introduce a data-race with a
> completely independent field that might have completely independent
> locking rules.
> 
> But
> 
>    struct {
>         int c:8;
>         int field1:5;
>    };
> 
> would not sanely have the same guarantees, even if the layout in
> memory might be identical. Once you have bitfields next to each other,
> and use a base type that means they can be combined together, they
> can't be sanely modified without locking.
>
> (And I don't know if C11 took up the "base type of the bitfield"
> thing. Maybe you still need to use the ":0" thing to force alignment,
> and maybe the C standards people still haven't made the underlying
> type be meaningful other than for sign handling).

The C standard draft (n2310) gives similar examples:

	EXAMPLE A structure declared as

		struct {
			char a;
			int b:5, c:11,:0, d:8;
			struct { int ee:8; } e;
		}

	contains four separate memory locations: The member a, and
	bit-fields d and e.ee are each separate memory locations,
	and can be modified concurrently without interfering with each
	other. The bit-fields b and c together constitute the fourth
	memory location. The bit-fields b and c cannot be concurrently
	modified, but b and a, for example, can be.

So yes, ":0" still forces alignment to the next storage unit.  And it
can be used to allow concurrent accesses to fields within a bitfield,
but only when those two fields are separated by ":0".

On the underlying type, according to J.3.9 of the current C working draft,
the following are implementation-specified behavior:

-	Whether a "plain" int bit-field is treated as a signed int
	bit-field or as an unsigned int bit-field (6.7.2, 6.7.2.1).

-	Whether atomic types are permitted for bit-fields (6.7.2.1).

This last is strange because you are not allowed to take the address of
a bit field, and the various operations on atomic types take addresses.
Search me!

							Thanx, Paul