| 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
| ||
|
Message-Id: <CR6IV4NTEN8Z.1V1RNR667SWJ7@vincent-arch>
Date: Wed, 15 Mar 2023 01:15:03 +0100
From: "Vincenzo Palazzo" <vincenzopalazzodev@...il.com>
To: "Andrea Righi" <andrea.righi@...onical.com>,
Michal Suchánek <msuchanek@...e.de>
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
> In practice, this is what I'm testing at the moment:
>
> ---
> diff --git a/arch/powerpc/kernel/module_64.c b/arch/powerpc/kernel/module_64.c
> index ff045644f13f..ea6c830ed1e7 100644
> --- a/arch/powerpc/kernel/module_64.c
> +++ b/arch/powerpc/kernel/module_64.c
> @@ -234,12 +234,13 @@ static unsigned long get_stubs_size(const Elf64_Ehdr *hdr,
> static void dedotify_versions(struct modversion_info *vers,
> unsigned long size)
> {
> - struct modversion_info *end;
> + struct modversion_info *end = (void *)vers + size;
>
> - for (end = (void *)vers + size; vers < end; vers++)
> + for (; vers < end && vers->next; vers = (void *)vers + vers->next) {
> if (vers->name[0] == '.') {
> memmove(vers->name, vers->name+1, strlen(vers->name));
> }
> + }
> }
>
> /*
> diff --git a/include/linux/module.h b/include/linux/module.h
> index 8c5909c0076c..4744901bdf63 100644
> --- a/include/linux/module.h
> +++ b/include/linux/module.h
> @@ -34,9 +34,11 @@
> #define MODULE_NAME_LEN MAX_PARAM_PREFIX_LEN
>
> 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];
> +} __packed;
>
> struct module;
> struct exception_table_entry;
> diff --git a/kernel/module/version.c b/kernel/module/version.c
> index 53f43ac5a73e..5528f98c42dc 100644
> --- a/kernel/module/version.c
> +++ b/kernel/module/version.c
> @@ -17,32 +17,30 @@ int check_version(const struct load_info *info,
> {
> Elf_Shdr *sechdrs = info->sechdrs;
> unsigned int versindex = info->index.vers;
> - unsigned int i, num_versions;
> - struct modversion_info *versions;
> + struct modversion_info *versions, *end;
> + u32 crcval;
>
> /* Exporting module didn't supply crcs? OK, we're already tainted. */
> if (!crc)
> return 1;
> + crcval = *crc;
>
> /* No versions at all? modprobe --force does this. */
> if (versindex == 0)
> return try_to_force_load(mod, symname) == 0;
>
> versions = (void *)sechdrs[versindex].sh_addr;
> - num_versions = sechdrs[versindex].sh_size
> - / sizeof(struct modversion_info);
> + end = (void *)versions + sechdrs[versindex].sh_size;
>
> - for (i = 0; i < num_versions; i++) {
> - u32 crcval;
> -
> - if (strcmp(versions[i].name, symname) != 0)
> + for (; versions < end && versions->next;
> + versions = (void *)versions + versions->next) {
> + if (strcmp(versions->name, symname) != 0)
> continue;
>
> - crcval = *crc;
> - if (versions[i].crc == crcval)
> + if (versions->crc == crcval)
> return 1;
> - pr_debug("Found checksum %X vs module %lX\n",
> - crcval, versions[i].crc);
> + pr_debug("Found checksum %X vs module %X\n",
> + crcval, versions->crc);
> goto bad_version;
> }
>
> diff --git a/scripts/export_report.pl b/scripts/export_report.pl
> index feb3d5542a62..1117646f3141 100755
> --- a/scripts/export_report.pl
> +++ b/scripts/export_report.pl
> @@ -116,18 +116,19 @@ foreach my $thismod (@allcfiles) {
> while ( <$module> ) {
> chomp;
> if ($state == 0) {
> - $state = 1 if ($_ =~ /static const struct modversion_info/);
> + $state = 1 if ($_ =~ /static const char ____versions/);
> next;
> }
> if ($state == 1) {
> - $state = 2 if ($_ =~ /__attribute__\(\(section\("__versions"\)\)\)/);
> + $state = 2 if ($_ =~ /__used __section\("__versions"\)/);
> next;
> }
> if ($state == 2) {
> - if ( $_ !~ /0x[0-9a-f]+,/ ) {
> + if ( $_ !~ /\\0"/ ) {
> + last if ($_ =~ /;/);
> next;
> }
> - my $sym = (split /([,"])/,)[4];
> + my $sym = (split /(["\\])/,)[2];
> my ($module, $value, $symbol, $gpl) = @{$SYMBOL{$sym}};
> $SYMBOL{ $sym } = [ $module, $value+1, $symbol, $gpl];
> push(@{$MODULE{$thismod}} , $sym);
> diff --git a/scripts/mod/modpost.c b/scripts/mod/modpost.c
> index efff8078e395..55335ae98f4f 100644
> --- a/scripts/mod/modpost.c
> +++ b/scripts/mod/modpost.c
> @@ -2046,13 +2046,17 @@ static void add_exported_symbols(struct buffer *buf, struct module *mod)
> static void add_versions(struct buffer *b, struct module *mod)
> {
> struct symbol *s;
> + unsigned int name_len;
> + unsigned int name_len_padded;
> + unsigned int tmp;
> + unsigned char *tmp_view = (unsigned char *)&tmp;
>
> if (!modversions)
> return;
>
> buf_printf(b, "\n");
> - buf_printf(b, "static const struct modversion_info ____versions[]\n");
> - buf_printf(b, "__used __section(\"__versions\") = {\n");
> + buf_printf(b, "static const char ____versions[]\n");
> + buf_printf(b, "__used __section(\"__versions\") =\n");
>
> list_for_each_entry(s, &mod->unresolved_symbols, list) {
> if (!s->module)
> @@ -2062,16 +2066,27 @@ static void add_versions(struct buffer *b, struct module *mod)
> s->name, mod->name);
> continue;
> }
> - if (strlen(s->name) >= MODULE_NAME_LEN) {
> - error("too long symbol \"%s\" [%s.ko]\n",
> - s->name, mod->name);
> - break;
> - }
> - buf_printf(b, "\t{ %#8x, \"%s\" },\n",
> - s->crc, s->name);
> + name_len = strlen(s->name);
> + name_len_padded = (name_len + 1 + 3) & ~3;
> +
> + /* Offset to next entry */
> + tmp = 8 + name_len_padded;
> + tmp = TO_NATIVE(tmp);
> + buf_printf(b, "\t\"\\x%02x\\x%02x\\x%02x\\x%02x",
> + tmp_view[0], tmp_view[1], tmp_view[2], tmp_view[3]);
> +
> + tmp = TO_NATIVE(s->crc);
> + buf_printf(b, "\\x%02x\\x%02x\\x%02x\\x%02x\"\n",
> + tmp_view[0], tmp_view[1], tmp_view[2], tmp_view[3]);
> +
> + buf_printf(b, "\t\"%s", s->name);
> + for (; name_len < name_len_padded; name_len++)
> + buf_printf(b, "\\0");
> + buf_printf(b, "\"\n");
> }
>
> - buf_printf(b, "};\n");
> + /* Always end with a dummy NULL entry */
> + buf_printf(b, "\t\"\\x00\\x00\\x00\\x00\\x00\\x00\\x00\\x00\";\n");
> }
>
> static void add_depends(struct buffer *b, struct module *mod)
This looks promissing, do you had a small reproducer to this just to
make the code a little bit clearn to the reader?
Ciao,
Vincent.
Powered by blists - more mailing lists