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]

Date: Fri, 24 May 2024 11:35:21 -0400
From: Mathieu Desnoyers <mathieu.desnoyers@...icios.com>
To: Kent Overstreet <kent.overstreet@...ux.dev>
Cc: Brian Foster <bfoster@...hat.com>, Kees Cook <keescook@...omium.org>,
 linux-kernel <linux-kernel@...r.kernel.org>, linux-bcachefs@...r.kernel.org,
 Alexander Potapenko <glider@...gle.com>, Marco Elver <elver@...gle.com>,
 Dmitry Vyukov <dvyukov@...gle.com>, kasan-dev@...glegroups.com,
 Nathan Chancellor <nathan@...nel.org>,
 Nick Desaulniers <ndesaulniers@...gle.com>, Bill Wendling
 <morbo@...gle.com>, Justin Stitt <justinstitt@...gle.com>,
 llvm@...ts.linux.dev
Subject: Re: Use of zero-length arrays in bcachefs structures inner fields

[ Adding clang/llvm and KMSAN maintainers/reviewers in CC. ]

On 2024-05-24 11:28, Kent Overstreet wrote:
> 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.

-- 
Mathieu Desnoyers
EfficiOS Inc.
https://www.efficios.com