From: Lucas De Marchi <lucas.demarchi@intel.com> To: Masahiro Yamada <masahiroy@kernel.org> Cc: "Gary Guo" <gary@garyguo.net>, "Nicolas Schier" <nicolas@fjasle.eu>, "Guo Zhengkui" <guozhengkui@vivo.com>, "Wedson Almeida Filho" <wedsonaf@google.com>, "Joel Stanley" <joel@jms.id.au>, "Alex Gaynor" <alex.gaynor@gmail.com>, "Miguel Ojeda" <ojeda@kernel.org>, "Michal Suchánek" <msuchanek@suse.de>, "Wedson Almeida Filho" <wedsonaf@gmail.com>, "Kees Cook" <keescook@chromium.org>, rust-for-linux@vger.kernel.org, linux-kbuild@vger.kernel.org, "Boqun Feng" <boqun.feng@gmail.com>, "Nicholas Piggin" <npiggin@gmail.com>, "Björn Roy Baron" <bjorn3_gh@protonmail.com>, "Nathan Chancellor" <nathan@kernel.org>, "Nick Desaulniers" <ndesaulniers@google.com>, linux-kernel@vger.kernel.org, "Julia Lawall" <Julia.Lawall@inria.fr>, "Luis Chamberlain" <mcgrof@kernel.org>, linuxppc-dev@lists.ozlabs.org, linux-modules@vger.kernel.org Subject: Re: [PATCH] modpost: support arbitrary symbol length in modversion Date: Thu, 19 Jan 2023 11:02:08 -0800 [thread overview] Message-ID: <20230119190208.2towhelvz7s6dn3u@ldmartin-desk2.lan> (raw) In-Reply-To: <CAK7LNATsuszFR7JB5ZkqVS1W=hWr9=E7bTf+MvgJ+NXT3aZNwg@mail.gmail.com> On Wed, Jan 18, 2023 at 04:01:29PM +0900, Masahiro Yamada wrote: >On Wed, Jan 18, 2023 at 4:23 AM Lucas De Marchi ><lucas.demarchi@intel.com> wrote: >> >> On Tue, Jan 17, 2023 at 06:51:44PM +0100, Michal Suchánek wrote: >> >Hello, >> > >> >On Fri, Jan 13, 2023 at 06:18:41PM +0000, Gary Guo wrote: >> >> On Thu, 12 Jan 2023 14:40:59 -0700 >> >> Lucas De Marchi <lucas.demarchi@intel.com> wrote: >> >> >> >> > On Wed, Jan 11, 2023 at 04:11:51PM +0000, Gary Guo wrote: >> >> > > >> >> > > struct modversion_info { >> >> > >- unsigned long crc; >> >> > >- char name[MODULE_NAME_LEN]; >> >> > >+ /* Offset of the next modversion entry in relation to this one. */ >> >> > >+ u32 next; >> >> > >+ u32 crc; >> >> > >+ char name[0]; >> >> > >> >> > although not really exported as uapi, this will break userspace as this is >> >> > used in the elf file generated for the modules. I think >> >> > this change must be made in a backward compatible way and kmod updated >> >> > to deal with the variable name length: >> >> > >> >> > kmod $ git grep "\[64" >> >> > libkmod/libkmod-elf.c: char name[64 - sizeof(uint32_t)]; >> >> > libkmod/libkmod-elf.c: char name[64 - sizeof(uint64_t)]; >> >> > >> >> > in kmod we have both 32 and 64 because a 64-bit kmod can read both 32 >> >> > and 64 bit module, and vice versa. >> >> > >> >> >> >> Hi Lucas, >> >> >> >> Thanks for the information. >> >> >> >> The change can't be "truly" backward compatible, in a sense that >> >> regardless of the new format we choose, kmod would not be able to decode >> >> symbols longer than "64 - sizeof(long)" bytes. So the list it retrieves >> >> is going to be incomplete, isn't it? >> >> >> >> What kind of backward compatibility should be expected? It could be: >> >> * short symbols can still be found by old versions of kmod, but not >> >> long symbols; >> > >> >That sounds good. Not everyone is using rust, and with this option >> >people who do will need to upgrade tooling, and people who don't care >> >don't need to do anything. >> >> that could be it indeed. My main worry here is: >> >> "After the support is added in kmod, kmod needs to be able to output the >> correct information regardless if the module is from before/after the >> change in the kernel and also without relying on kernel version." >> Just changing the struct modversion_info doesn't make that possible. >> >> Maybe adding the long symbols in another section? Or ble >> just increase to 512 and add the size to a >> "__versions_hdr" section. If we then output a max size per module, >> this would offset a little bit the additional size gained for the >> modules using rust. And the additional 0's should compress well >> so I'm not sure the additional size is that much relevant here. > > > > > >I also thought of new section(s) for long symbols. > > > >One idea is to have separate sections for CRCs and symbol names. > > > > >section __version_crc: > 0x12345678 > 0x23456789 > 0x34567890 > > >section __version_sym: > "very_very_very_very_long_symbol" > "another_very_very_very_very_very_long_symbol" > "yet_another_very_very_very_very_very_long_symbol" > > > > >You can iterate in each section with this: > > crc += sizeof(u32); > name += strlen(name) + 1; > > >Benefits: > - No next pointer > - No padding > - *.mod.c is kept human readable. I like this option. It would be better than the current one. Lucas De Marchi > > > > > >BTW, the following is impossible >because the pointer reference to .rodata >is not available at this point? > >struct modversion_info { > u32 crc; > const char *name: >}; > > > >-- >Best Regards >Masahiro Yamada
WARNING: multiple messages have this Message-ID (diff)
From: Lucas De Marchi <lucas.demarchi@intel.com> To: Masahiro Yamada <masahiroy@kernel.org> Cc: "Michal Suchánek" <msuchanek@suse.de>, "Gary Guo" <gary@garyguo.net>, "Kees Cook" <keescook@chromium.org>, linux-kbuild@vger.kernel.org, "Wedson Almeida Filho" <wedsonaf@google.com>, "Joel Stanley" <joel@jms.id.au>, "Alex Gaynor" <alex.gaynor@gmail.com>, "Miguel Ojeda" <ojeda@kernel.org>, "Wedson Almeida Filho" <wedsonaf@gmail.com>, "Nicolas Schier" <nicolas@fjasle.eu>, rust-for-linux@vger.kernel.org, "Guo Zhengkui" <guozhengkui@vivo.com>, "Boqun Feng" <boqun.feng@gmail.com>, "Nicholas Piggin" <npiggin@gmail.com>, "Björn Roy Baron" <bjorn3_gh@protonmail.com>, "Nathan Chancellor" <nathan@kernel.org>, "Nick Desaulniers" <ndesaulniers@google.com>, linux-kernel@vger.kernel.org, "Julia Lawall" <Julia.Lawall@inria.fr>, "Luis Chamberlain" <mcgrof@kernel.org>, linuxppc-dev@lists.ozlabs.org, linux-modules@vger.kernel.org Subject: Re: [PATCH] modpost: support arbitrary symbol length in modversion Date: Thu, 19 Jan 2023 11:02:08 -0800 [thread overview] Message-ID: <20230119190208.2towhelvz7s6dn3u@ldmartin-desk2.lan> (raw) In-Reply-To: <CAK7LNATsuszFR7JB5ZkqVS1W=hWr9=E7bTf+MvgJ+NXT3aZNwg@mail.gmail.com> On Wed, Jan 18, 2023 at 04:01:29PM +0900, Masahiro Yamada wrote: >On Wed, Jan 18, 2023 at 4:23 AM Lucas De Marchi ><lucas.demarchi@intel.com> wrote: >> >> On Tue, Jan 17, 2023 at 06:51:44PM +0100, Michal Suchánek wrote: >> >Hello, >> > >> >On Fri, Jan 13, 2023 at 06:18:41PM +0000, Gary Guo wrote: >> >> On Thu, 12 Jan 2023 14:40:59 -0700 >> >> Lucas De Marchi <lucas.demarchi@intel.com> wrote: >> >> >> >> > On Wed, Jan 11, 2023 at 04:11:51PM +0000, Gary Guo wrote: >> >> > > >> >> > > struct modversion_info { >> >> > >- unsigned long crc; >> >> > >- char name[MODULE_NAME_LEN]; >> >> > >+ /* Offset of the next modversion entry in relation to this one. */ >> >> > >+ u32 next; >> >> > >+ u32 crc; >> >> > >+ char name[0]; >> >> > >> >> > although not really exported as uapi, this will break userspace as this is >> >> > used in the elf file generated for the modules. I think >> >> > this change must be made in a backward compatible way and kmod updated >> >> > to deal with the variable name length: >> >> > >> >> > kmod $ git grep "\[64" >> >> > libkmod/libkmod-elf.c: char name[64 - sizeof(uint32_t)]; >> >> > libkmod/libkmod-elf.c: char name[64 - sizeof(uint64_t)]; >> >> > >> >> > in kmod we have both 32 and 64 because a 64-bit kmod can read both 32 >> >> > and 64 bit module, and vice versa. >> >> > >> >> >> >> Hi Lucas, >> >> >> >> Thanks for the information. >> >> >> >> The change can't be "truly" backward compatible, in a sense that >> >> regardless of the new format we choose, kmod would not be able to decode >> >> symbols longer than "64 - sizeof(long)" bytes. So the list it retrieves >> >> is going to be incomplete, isn't it? >> >> >> >> What kind of backward compatibility should be expected? It could be: >> >> * short symbols can still be found by old versions of kmod, but not >> >> long symbols; >> > >> >That sounds good. Not everyone is using rust, and with this option >> >people who do will need to upgrade tooling, and people who don't care >> >don't need to do anything. >> >> that could be it indeed. My main worry here is: >> >> "After the support is added in kmod, kmod needs to be able to output the >> correct information regardless if the module is from before/after the >> change in the kernel and also without relying on kernel version." >> Just changing the struct modversion_info doesn't make that possible. >> >> Maybe adding the long symbols in another section? Or ble >> just increase to 512 and add the size to a >> "__versions_hdr" section. If we then output a max size per module, >> this would offset a little bit the additional size gained for the >> modules using rust. And the additional 0's should compress well >> so I'm not sure the additional size is that much relevant here. > > > > > >I also thought of new section(s) for long symbols. > > > >One idea is to have separate sections for CRCs and symbol names. > > > > >section __version_crc: > 0x12345678 > 0x23456789 > 0x34567890 > > >section __version_sym: > "very_very_very_very_long_symbol" > "another_very_very_very_very_very_long_symbol" > "yet_another_very_very_very_very_very_long_symbol" > > > > >You can iterate in each section with this: > > crc += sizeof(u32); > name += strlen(name) + 1; > > >Benefits: > - No next pointer > - No padding > - *.mod.c is kept human readable. I like this option. It would be better than the current one. Lucas De Marchi > > > > > >BTW, the following is impossible >because the pointer reference to .rodata >is not available at this point? > >struct modversion_info { > u32 crc; > const char *name: >}; > > > >-- >Best Regards >Masahiro Yamada
next prev parent reply other threads:[~2023-01-19 19:03 UTC|newest] Thread overview: 44+ messages / expand[flat|nested] mbox.gz Atom feed top 2023-01-11 16:11 [PATCH] modpost: support arbitrary symbol length in modversion Gary Guo 2023-01-11 16:11 ` Gary Guo 2023-01-12 21:40 ` Lucas De Marchi 2023-01-12 21:40 ` Lucas De Marchi 2023-01-13 18:18 ` Gary Guo 2023-01-13 18:18 ` Gary Guo 2023-01-17 17:51 ` Michal Suchánek 2023-01-17 17:51 ` Michal Suchánek 2023-01-17 19:22 ` Lucas De Marchi 2023-01-17 19:22 ` Lucas De Marchi 2023-01-18 7:01 ` Masahiro Yamada 2023-01-18 7:01 ` Masahiro Yamada 2023-01-19 19:02 ` Lucas De Marchi [this message] 2023-01-19 19:02 ` Lucas De Marchi 2023-01-19 19:19 ` Miguel Ojeda 2023-01-19 19:19 ` Miguel Ojeda 2023-01-19 15:09 ` Gary Guo 2023-01-19 15:09 ` Gary Guo 2023-01-19 15:18 ` Michal Suchánek 2023-01-19 15:18 ` Michal Suchánek 2023-01-19 15:57 ` Gary Guo 2023-01-19 15:57 ` Gary Guo 2023-03-13 20:32 ` Andrea Righi 2023-03-13 20:32 ` Andrea Righi 2023-03-13 21:48 ` Michal Suchánek 2023-03-13 21:48 ` Michal Suchánek 2023-03-13 21:53 ` Andrea Righi 2023-03-13 21:53 ` Andrea Righi 2023-03-13 22:02 ` Michal Suchánek 2023-03-13 22:02 ` Michal Suchánek 2023-03-13 22:09 ` Andrea Righi 2023-03-13 22:09 ` Andrea Righi 2023-03-14 14:38 ` Andrea Righi 2023-03-14 14:38 ` Andrea Righi 2023-03-14 14:59 ` Andrea Righi 2023-03-14 14:59 ` Andrea Righi 2023-03-15 0:15 ` Vincenzo Palazzo 2023-03-15 0:15 ` Vincenzo Palazzo 2023-03-15 6:07 ` Andrea Righi 2023-03-15 6:07 ` Andrea Righi 2023-03-15 16:18 ` Masahiro Yamada 2023-03-15 16:18 ` Masahiro Yamada 2023-03-15 21:00 ` Andrea Righi 2023-03-15 21:00 ` Andrea Righi
Reply instructions: You may reply publicly to this message via plain-text email using any one of the following methods: * Save the following mbox file, import it into your mail client, and reply-to-all from there: mbox Avoid top-posting and favor interleaved quoting: https://en.wikipedia.org/wiki/Posting_style#Interleaved_style * Reply using the --to, --cc, and --in-reply-to switches of git-send-email(1): git send-email \ --in-reply-to=20230119190208.2towhelvz7s6dn3u@ldmartin-desk2.lan \ --to=lucas.demarchi@intel.com \ --cc=Julia.Lawall@inria.fr \ --cc=alex.gaynor@gmail.com \ --cc=bjorn3_gh@protonmail.com \ --cc=boqun.feng@gmail.com \ --cc=gary@garyguo.net \ --cc=guozhengkui@vivo.com \ --cc=joel@jms.id.au \ --cc=keescook@chromium.org \ --cc=linux-kbuild@vger.kernel.org \ --cc=linux-kernel@vger.kernel.org \ --cc=linux-modules@vger.kernel.org \ --cc=linuxppc-dev@lists.ozlabs.org \ --cc=masahiroy@kernel.org \ --cc=mcgrof@kernel.org \ --cc=msuchanek@suse.de \ --cc=nathan@kernel.org \ --cc=ndesaulniers@google.com \ --cc=nicolas@fjasle.eu \ --cc=npiggin@gmail.com \ --cc=ojeda@kernel.org \ --cc=rust-for-linux@vger.kernel.org \ --cc=wedsonaf@gmail.com \ --cc=wedsonaf@google.com \ /path/to/YOUR_REPLY https://kernel.org/pub/software/scm/git/docs/git-send-email.html * If your mail client supports setting the In-Reply-To header via mailto: links, try the mailto: linkBe sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.