linux-kernel - Re: linux-next: Tree for Sep 12 (bcachefs)

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: <202309131803.6A3C1D05A@keescook>
Date:   Wed, 13 Sep 2023 18:17:00 -0700
From:   Kees Cook <keescook@...omium.org>
To:     Stephen Rothwell <sfr@...b.auug.org.au>,
        Kent Overstreet <kent.overstreet@...ux.dev>
Cc:     Linux Next Mailing List <linux-next@...r.kernel.org>,
        Linux Kernel Mailing List <linux-kernel@...r.kernel.org>,
        linux-hardening@...r.kernel.org
Subject: Re: linux-next: Tree for Sep 12 (bcachefs)

On Tue, Sep 12, 2023 at 03:26:45PM +1000, Stephen Rothwell wrote:
> New tree: bcachefs

Thanks for going through and fixing all the fake flexible array members.
It looks much nicer. :)

I have some questions about the remaining "markers", for example:

$ git grep -A8 '\bkey_start\b' -- fs/bcachefs
fs/bcachefs/bcachefs_format.h:  __u8            key_start[0];
...
fs/bcachefs/bcachefs_format.h-  __u8            pad[sizeof(struct bkey) - 3];
--
fs/bcachefs/bkey.c:     u8 *l = k->key_start;

Why isn't this just:

			u8 *l = k->pad

and you can drop the marker?

And some seem entirely unused, like all of "struct bch_reflink_v".

And some are going to fail at runtime, since they're still zero-sized
and being used as an actual array:

struct bch_sb_field_journal_seq_blacklist {
        struct bch_sb_field     field;

        struct journal_seq_blacklist_entry start[0];
        __u64                   _data[];
};
...
                memmove(&bl->start[i],
                        &bl->start[i + 1],
                        sizeof(bl->start[0]) * (nr - i));

It looks like you just want a type union for the flexible array.
This can be done like this:

struct bch_sb_field_journal_seq_blacklist {
        struct bch_sb_field     field;

	union {
		DECLARE_FLEX_ARRAY(struct journal_seq_blacklist_entry, start);
		DECLARE_FLEX_ARRAY(__u64, _data);
	};
};

Hopefully that helps!

-Kees

-- 
Kees Cook