linux-kernel - Re: [RFC PATCH 6/7] tpm: add __always_inline for tpm_is_hwrng

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] [day] [month] [year] [list]
Message-ID: <CAC1kPDM4QfqLduSYG2bT21kCbAQzThRyErZOED+Rw1=i-X8dmA@mail.gmail.com>
Date: Tue, 15 Apr 2025 10:28:56 +0800
From: Chen Linxuan <chenlinxuan@...ontech.com>
To: Jarkko Sakkinen <jarkko@...nel.org>
Cc: Chen Linxuan <chenlinxuan@...ontech.com>, Peter Huewe <peterhuewe@....de>, 
	Jason Gunthorpe <jgg@...pe.ca>, Winston Wen <wentao@...ontech.com>, linux-integrity@...r.kernel.org, 
	linux-kernel@...r.kernel.org
Subject: Re: [RFC PATCH 6/7] tpm: add __always_inline for tpm_is_hwrng_enabled

Jarkko Sakkinen <jarkko@...nel.org> 于2025年4月12日周六 08:47写道：
>
> On Fri, Apr 11, 2025 at 06:54:54PM +0800, Chen Linxuan wrote:
> > From: Winston Wen <wentao@...ontech.com>
> >
> > On x86_64 with gcc version 13.3.0, I compile kernel with:
>
> Use passive:
>
> "Presume that kernel is compiled for x86_64 with gcc version 13.3.0:"
>
> >
> >   make defconfig
> >   ./scripts/kconfig/merge_config.sh .config <(
> >     echo CONFIG_TCG_TPM=y
> >     echo CONFIG_HW_RANDOM=m
> >   )
> >   make KCFLAGS="-fno-inline-small-functions -fno-inline-functions-called-once"
> >
> > Then I get a link error:
>
> "This results a link error:"
>

I will update commit message when I send v2.

> >
> >   ld: vmlinux.o: in function `tpm_add_hwrng':
> >   tpm-chip.c:(.text+0x6c5924): undefined reference to `hwrng_register'
> >   ld: vmlinux.o: in function `tpm_chip_unregister':
> >   (.text+0x6c5bc9): undefined reference to `hwrng_unregister'
> >   ld: vmlinux.o: in function `tpm_chip_register':
> >   (.text+0x6c5c9b): undefined reference to `hwrng_unregister'
>
> The resolution is lacking i.e., why adding __always_inline addresses
> the linking problem.

Regarding your comment about the resolution,
here’s a detailed explanation of
 why adding the `__always_inline` attribute addresses the linking issue:

With `CONFIG_TCG_TPM=y` and `CONFIG_HW_RANDOM=m`,
the functions `tpm_add_hwrng`, `tpm_chip_unregister`, and
`tpm_chip_register` are compiled into `vmlinux.o`
and reference the symbols `hwrng_register` and `hwrng_unregister`.
These symbols, however, are compiled into `rng-core.ko`, which results
in the linking error.

I am not sure but I think this weird linking error only arises when
auto inlining is disabled because of some dead code elimination.

`CONFIG_TCG_TPM=y` and `CONFIG_HW_RANDOM=m` set `CONFIG_HW_RANDOM_TPM=n`.
This causes the function `tpm_is_hwrng_enabled` to always return
`false`, as shown below:

```c
static bool tpm_is_hwrng_enabled(struct tpm_chip *chip)
{
    if (!IS_ENABLED(CONFIG_HW_RANDOM_TPM))
        return false;
    if (tpm_is_firmware_upgrade(chip))
        return false;
    if (chip->flags & TPM_CHIP_FLAG_HWRNG_DISABLED)
        return false;
    return true;
}
```

When `tpm_is_hwrng_enabled` is inlined, dead code elimination
optimizations are applied and the reference to the `hwrng_*` functions
will been removed.
For instance, in the `tpm_chip_unregister` function:

```c
void tpm_chip_unregister(struct tpm_chip *chip)
{
#ifdef CONFIG_TCG_TPM2_HMAC
    int rc;

    rc = tpm_try_get_ops(chip);
    if (!rc) {
        tpm2_end_auth_session(chip);
        tpm_put_ops(chip);
    }
#endif

    tpm_del_legacy_sysfs(chip);
    if (tpm_is_hwrng_enabled(chip))
        hwrng_unregister(&chip->hwrng);
    tpm_bios_log_teardown(chip);
    if (chip->flags & TPM_CHIP_FLAG_TPM2 && !tpm_is_firmware_upgrade(chip))
        tpm_devs_remove(chip);
    tpm_del_char_device(chip);
}
```

When `tpm_is_hwrng_enabled` is inlined and always returns `false`,
the call to `hwrng_unregister` is effectively part of a `if (false)` block,
which I guess that will be then optimized out.

However, when the `-fno-inline-small-functions` and
`-fno-inline-functions-called-once` flags are used,
tpm_is_hwrng_enabled is not inline.

And this optimization some how cannot occur,
leading to the undefined reference errors during linking.

Adding the `__always_inline` attribute ensures that
`tpm_is_hwrng_enabled` is inlined regardless of the compiler flags.
This allows the dead code elimination to proceed as expected,
resolving the linking issue.

There might be better ways to fix this.
But it is directly caused by adding `-fno-inline-small-functions` and
`-fno-inline-functions-called-once` flags,
I think add `__always_inline` is good enough.

>
> >
> > Signed-off-by: Winston Wen <wentao@...ontech.com>
> > Co-Developed-by: Chen Linxuan <chenlinxuan@...ontech.com>
> > Signed-off-by: Chen Linxuan <chenlinxuan@...ontech.com>
> > ---
> >  drivers/char/tpm/tpm-chip.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/drivers/char/tpm/tpm-chip.c b/drivers/char/tpm/tpm-chip.c
> > index e25daf2396d3..48cc74d84247 100644
> > --- a/drivers/char/tpm/tpm-chip.c
> > +++ b/drivers/char/tpm/tpm-chip.c
> > @@ -534,7 +534,7 @@ static int tpm_hwrng_read(struct hwrng *rng, void *data, size_t max, bool wait)
> >       return tpm_get_random(chip, data, max);
> >  }
> >
> > -static bool tpm_is_hwrng_enabled(struct tpm_chip *chip)
> > +static __always_inline bool tpm_is_hwrng_enabled(struct tpm_chip *chip)
> >  {
> >       if (!IS_ENABLED(CONFIG_HW_RANDOM_TPM))
> >               return false;
> > --
> > 2.48.1
> >
>
> BR, Jarkko
>
>