linux-kernel - Re: Use of zero-length arrays in bcachefs structures inner fields

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: <vu7w6if47tv3kwnbbbsdchu3wpsbkqlvlkvewtvjx5hkq57fya@rgl6bp33eizt>
Date: Fri, 24 May 2024 11:28:54 -0400
From: Kent Overstreet <kent.overstreet@...ux.dev>
To: Mathieu Desnoyers <mathieu.desnoyers@...icios.com>
Cc: Brian Foster <bfoster@...hat.com>, Kees Cook <keescook@...omium.org>, 
	linux-kernel <linux-kernel@...r.kernel.org>, linux-bcachefs@...r.kernel.org
Subject: Re: Use of zero-length arrays in bcachefs structures inner fields

On Thu, May 23, 2024 at 01:53:42PM -0400, Mathieu Desnoyers wrote:
> Hi Kent,
> 
> Looking around in the bcachefs code for possible causes of this KMSAN
> bug report:
> 
> https://lore.kernel.org/lkml/000000000000fd5e7006191f78dc@google.com/
> 
> I notice the following pattern in the bcachefs structures: zero-length
> arrays members are inserted in structures (not always at the end),
> seemingly to achieve a result similar to what could be done with a
> union:
> 
> fs/bcachefs/bcachefs_format.h:
> 
> struct bkey_packed {
>         __u64           _data[0];
> 
>         /* Size of combined key and value, in u64s */
>         __u8            u64s;
> [...]
> };
> 
> likewise:
> 
> struct bkey_i {
>         __u64                   _data[0];
> 
>         struct bkey     k;
>         struct bch_val  v;
> };
> 
> (and there are many more examples of this pattern in bcachefs)
> 
> AFAIK, the C11 standard states that array declarator constant expression
> 
> Effectively, we can verify that this code triggers an undefined behavior
> with:
> 
> #include <stdio.h>
> 
> struct z {
>         int x[0];
>         int y;
>         int z;
> } __attribute__((packed));
> 
> int main(void)
> {
>         struct z a;
> 
>         a.y = 1;
>         printf("%d\n", a.x[0]);
> }
> delimited by [ ] shall have a value greater than zero.

Yet another example of the C people going absolutely nutty with
everything being undefined. Look, this isn't ok, we need to get work
done, and I've already wasted entirely too much time on ZLA vs. flex
array member nonsense.

There's a bunch of legit uses for zero length arrays, and your example,
where we're not even _assigning_ to x, is just batshit. Someone needs to
get his head examined.

> So I wonder if the issue reported by KMSAN could be caused by this
> pattern ?

Possibly; the KMSAN errors I've been looking at do look suspicious. But
it sounds like we need a real fix that involves defining proper
semantics, not compiler folks giving up and saying 'aiee!'.

IOW, clang/KMSAN are broken if they simply choke on a zero length array
being present.