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

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]

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!