linux-ext4 - Re: simplify ext4_sb_read

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: <ZDnbW1qYmBLycefL@google.com>
Date:   Fri, 14 Apr 2023 16:01:47 -0700
From:   William McVicker <willmcvicker@...gle.com>
To:     Theodore Ts'o <tytso@....edu>, hch@....de,
        linux-ext4@...r.kernel.org
Cc:     "Stephen E. Baker" <baker.stephen.e@...il.com>,
        adilger.kernel@...ger.ca, hch@....de, linux-ext4@...r.kernel.org
Subject: Re: simplify ext4_sb_read_encoding regression

On 06/02/2022, Theodore Ts'o wrote:
> On Wed, Jun 01, 2022 at 10:06:04PM -0400, Stephen E. Baker wrote:
> > On Mon, May 30, 2022 at 9:37 PM Theodore Ts'o <tytso@....edu> wrote:
> > > > I don't know what to tell you.  I took your config, stripped out all
> > > > of the modules, and enabled CONFIG_HYPERVISOR_GUEST,
> > > > CONFIG_VIRTIO_MENU, and CONFIG_VIRTIO_BLK, and build a 5.16 kernel.
> > >
> > Maybe a silly question, but how do I enable CONFIG_HYPERVISOR_GUEST with
> > this config.
> 
> So let's make things easy.  Attached please find my minimal config.
> This is what I use when I normally build a test kernels, and I get it
> by running "kvm-xfstests install-kconfig".  I use it because it's fast
> to build, since it doesn't build extraneous stuff.  I've also attached
> the "seb-config", which is your configuration with the minimal changes
> needed so it can run under qemu.  The compressed size is twice as big,
> and it takes 2-3 times longer to build.
> 
> I can't reproduce the problem you are seeing using kvm-xfstests using
> either kernel config building on 5.17.
> 
> 					- Ted


Hi,

I believe I figured out what is going on here since I am hitting a similar
issue with CONFIG_UNICODE. If you take a look at the .config from Stephen's
message, you'll see that he sets:

  CONFIG_TRIM_UNUSED_KSYMS=y
  CONFIG_UNUSED_KSYMS_WHITELIST=""

When trimming is enabled, kbuild will strip all exported symbols that are not
listed in the whitelist. As a result, when utf8-core.c calls:

  um->tables = symbol_request(utf8_data_table);

it will fail since `utf8_data_table` doesn't exist in the exported section of
the kernel symbol table. For me on Android, this leads to the userdata
partition failing to mount. To be clear, this happens when CONFIG_UNICODE=y.

One question I have is -- Why are we using symbol_request()/symbol_put() when
`utf8_data_table` is exported? Why can't we directly reference the
`utf8_data_table` symbol?

If we need to use symbol_request() when CONFIG_UNICODE=m, then can we apply the
below patch to fix this when CONFIG_UNICODE=y? I have verified this works for
me with CONFIG_UNICODE=y and CONFIG_TRIM_UNUSED_KSYMS=y.

Thanks,
Will


diff --git a/fs/unicode/utf8-core.c b/fs/unicode/utf8-core.c
index 67aaadc3ab072..1631bffa51b2f 100644
--- a/fs/unicode/utf8-core.c
+++ b/fs/unicode/utf8-core.c
@@ -181,9 +181,15 @@ struct unicode_map *utf8_load(unsigned int version)
                return ERR_PTR(-ENOMEM);
        um->version = version;
 
+#if defined(CONFIG_UNICODE_MODULE)
        um->tables = symbol_request(utf8_data_table);
-       if (!um->tables)
+#else
+       um->tables = &utf8_data_table;
+#endif
+       if (!um->tables) {
+               pr_err("%s: WILL: Failed to find utf8_data_table symbol!\n", __func__);
                goto out_free_um;
+       }
 
        if (!utf8version_is_supported(um, version))
                goto out_symbol_put;
@@ -198,7 +204,9 @@ struct unicode_map *utf8_load(unsigned int version)
        return um;
 
 out_symbol_put:
+#if defined(CONFIG_UNICODE_MODULE)
        symbol_put(um->tables);
+#endif
 out_free_um:
        kfree(um);
        return ERR_PTR(-EINVAL);
@@ -208,7 +216,9 @@ EXPORT_SYMBOL(utf8_load);
 void utf8_unload(struct unicode_map *um)
 {
        if (um) {
+#if defined(CONFIG_UNICODE_MODULE)
                symbol_put(utf8_data_table);
+#endif
                kfree(um);
        }
 }