[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <20230701-umspielen-wohin-de72fcaa7bbc@brauner>
Date: Sat, 1 Jul 2023 08:24:15 +0200
From: Christian Brauner <brauner@...nel.org>
To: Kees Cook <keescook@...omium.org>
Cc: Jan Kara <jack@...e.cz>, Andreas Gruenbacher <agruenba@...hat.com>,
Daniel Verkamp <dverkamp@...omium.org>,
"Paul E. McKenney" <paulmck@...nel.org>,
Jeff Xu <jeffxu@...gle.com>,
Andrew Morton <akpm@...ux-foundation.org>,
Boqun Feng <boqun.feng@...il.com>,
Luis Chamberlain <mcgrof@...nel.org>,
Frederic Weisbecker <frederic@...nel.org>,
syzbot+ac3b41786a2d0565b6d5@...kaller.appspotmail.com,
linux-kernel@...r.kernel.org, linux-hardening@...r.kernel.org
Subject: Re: [PATCH v2] pid: Replace struct pid 1-element array with
flex-array
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@...nel.org>
> Cc: Jan Kara <jack@...e.cz>
> Cc: Andreas Gruenbacher <agruenba@...hat.com>
> Cc: Daniel Verkamp <dverkamp@...omium.org>
> Cc: "Paul E. McKenney" <paulmck@...nel.org>
> Cc: Jeff Xu <jeffxu@...gle.com>
> Cc: Andrew Morton <akpm@...ux-foundation.org>
> Cc: Boqun Feng <boqun.feng@...il.com>
> Cc: Luis Chamberlain <mcgrof@...nel.org>
> Cc: Frederic Weisbecker <frederic@...nel.org>
> Reported-by: syzbot+ac3b41786a2d0565b6d5@...kaller.appspotmail.com
> Closes: https://lore.kernel.org/lkml/000000000000c6de2a05fbdecbbb@google.com/
> Acked-by: Jeff Xu <jeffxu@...gle.com>
> Signed-off-by: Kees Cook <keescook@...omium.org>
> ---
> 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!
Powered by blists - more mailing lists