Re: [PATCH v2] pid: Replace struct pid 1-element array with flex-array

[Christian Brauner]

On Fri, Jun 30, 2023 at 11:04:22AM -0700, Kees Cook wrote:
> For pid namespaces, struct pid uses a dynamically sized array member,
> "numbers". This was implemented using the ancient 1-element fake flexible
> array, which has been deprecated for decades. Replace it with a C99
> flexible array, refactor the array size calculations to use struct_size(),
> and address elements via indexes. Note that the static initializer (which
> defines a single element) works as-is, and requires no special handling.
> 
> Without this, CONFIG_UBSAN_BOUNDS (and potentially CONFIG_FORTIFY_SOURCE)
> will trigger bounds checks when entering a pid namespace:
> https://lore.kernel.org/lkml/20230517-bushaltestelle-super-e223978c1ba6@brauner
> 
> For example: unshare --fork --pid --mount-proc readlink /proc/self
> 
> Cc: Christian Brauner <brauner@xxxxxxxxxx>
> Cc: Jan Kara <jack@xxxxxxx>
> Cc: Andreas Gruenbacher <agruenba@xxxxxxxxxx>
> Cc: Daniel Verkamp <dverkamp@xxxxxxxxxxxx>
> Cc: "Paul E. McKenney" <paulmck@xxxxxxxxxx>
> Cc: Jeff Xu <jeffxu@xxxxxxxxxx>
> Cc: Andrew Morton <akpm@xxxxxxxxxxxxxxxxxxxx>
> Cc: Boqun Feng <boqun.feng@xxxxxxxxx>
> Cc: Luis Chamberlain <mcgrof@xxxxxxxxxx>
> Cc: Frederic Weisbecker <frederic@xxxxxxxxxx>
> Reported-by: syzbot+ac3b41786a2d0565b6d5@xxxxxxxxxxxxxxxxxxxxxxxxx
> Closes: https://lore.kernel.org/lkml/000000000000c6de2a05fbdecbbb@xxxxxxxxxx/
> Acked-by: Jeff Xu <jeffxu@xxxxxxxxxx>
> Signed-off-by: Kees Cook <keescook@xxxxxxxxxxxx>
> ---
> v2:
>  - drop pointer math to array index conversions (torvalds)
>  - use struct_size_t now that it exists (torvalds)
>  - updated commit log with reproducer example

Thanks for that. Linus already merged the other version I gave him with
his requested changes but without your struct_size_t() update. It didn't
exist when you originally sent that patch afaiu and I just realized that
it existed right now. In any case, I'm just going to send a trivial
follow-up patch switching the two places to struct_size_t().

Thanks, Kees!