linux-kernel - Re: [PATCH] modpost: support arbitrary symbol length in modversion

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: <20230313220234.GZ19419@kitsune.suse.cz>
Date:   Mon, 13 Mar 2023 23:02:34 +0100
From:   Michal Suchánek <msuchanek@...e.de>
To:     Andrea Righi <andrea.righi@...onical.com>
Cc:     Gary Guo <gary@...yguo.net>, Kees Cook <keescook@...omium.org>,
        Masahiro Yamada <masahiroy@...nel.org>,
        linux-kbuild@...r.kernel.org,
        Wedson Almeida Filho <wedsonaf@...gle.com>,
        Joel Stanley <joel@....id.au>,
        Alex Gaynor <alex.gaynor@...il.com>,
        Miguel Ojeda <ojeda@...nel.org>,
        Wedson Almeida Filho <wedsonaf@...il.com>,
        Nicolas Schier <nicolas@...sle.eu>,
        rust-for-linux@...r.kernel.org,
        Guo Zhengkui <guozhengkui@...o.com>,
        Boqun Feng <boqun.feng@...il.com>,
        Nicholas Piggin <npiggin@...il.com>,
        Björn Roy Baron <bjorn3_gh@...tonmail.com>,
        Nathan Chancellor <nathan@...nel.org>,
        Nick Desaulniers <ndesaulniers@...gle.com>,
        linux-kernel@...r.kernel.org, Julia Lawall <Julia.Lawall@...ia.fr>,
        Luis Chamberlain <mcgrof@...nel.org>,
        linuxppc-dev@...ts.ozlabs.org, linux-modules@...r.kernel.org
Subject: Re: [PATCH] modpost: support arbitrary symbol length in modversion

On Mon, Mar 13, 2023 at 10:53:34PM +0100, Andrea Righi wrote:
> On Mon, Mar 13, 2023 at 10:48:53PM +0100, Michal Suchánek wrote:
> > Hello,
> > 
> > On Mon, Mar 13, 2023 at 09:32:16PM +0100, Andrea Righi wrote:
> > > On Wed, Jan 11, 2023 at 04:11:51PM +0000, Gary Guo wrote:
> > > > Currently modversion uses a fixed size array of size (64 - sizeof(long))
> > > > to store symbol names, thus placing a hard limit on length of symbols.
> > > > Rust symbols (which encodes crate and module names) can be quite a bit
> > > > longer. The length limit in kallsyms is increased to 512 for this reason.
> > > > 
> > > > It's a waste of space to simply expand the fixed array size to 512 in
> > > > modversion info entries. I therefore make it variably sized, with offset
> > > > to the next entry indicated by the initial "next" field.
> > > > 
> > > > In addition to supporting longer-than-56/60 byte symbols, this patch also
> > > > reduce the size for short symbols by getting rid of excessive 0 paddings.
> > > > There are still some zero paddings to ensure "next" and "crc" fields are
> > > > properly aligned.
> > > > 
> > > > This patch does have a tiny drawback that it makes ".mod.c" files generated
> > > > a bit less easy to read, as code like
> > > > 
> > > > 	"\x08\x00\x00\x00\x78\x56\x34\x12"
> > > > 	"symbol\0\0"
> > > > 
> > > > is generated as opposed to
> > > > 
> > > > 	{ 0x12345678, "symbol" },
> > > > 
> > > > because the structure is now variable-length. But hopefully nobody reads
> > > > the generated file :)
> > > > 
> > > > Link: b8a94bfb3395 ("kallsyms: increase maximum kernel symbol length to 512")
> > > > Link: https://github.com/Rust-for-Linux/linux/pull/379
> > > > 
> > > > Signed-off-by: Gary Guo <gary@...yguo.net>
> > > 
> > > Is there any newer version of this patch?
> > > 
> > > I'm doing some tests with it, but I'm getting boot failures on ppc64
> > > with this applied (at boot kernel is spitting out lots of oops'es and
> > > unfortunately it's really hard to copy paste or just read them from the
> > > console).
> > 
> > Are you using the ELF ABI v1 or v2?
> > 
> > v1 may have some additional issues when it comes to these symbol tables.
> > 
> > Thanks
> > 
> > Michal
> 
> I have CONFIG_PPC64_ELF_ABI_V2=y in my .config, so I guess I'm using v2.
> 
> BTW, the issue seems to be in dedotify_versions(), as a silly test I
> tried to comment out this function completely to be a no-op and now my
> system boots fine (but I guess I'm probably breaking something else).

Probably not. You should not have the extra leading dot on ABI v2. So if
dedotify does something that means something generates and then expects
back symbols with a leading dot, and this workaround for ABI v1 breaks
that. Or maybe it is called when it shouldn't.

Thanks

Michal