netdev - Re: [PATCH net-next v4] net: skb: prevent the split of kfree_skb

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]

Date:   Sat, 20 Aug 2022 19:00:41 +0800
From:   Menglong Dong <menglong8.dong@...il.com>
To:     Segher Boessenkool <segher@...nel.crashing.org>
Cc:     Nick Desaulniers <ndesaulniers@...gle.com>, kuba@...nel.org,
        miguel.ojeda.sandonis@...il.com, ojeda@...nel.org,
        davem@...emloft.net, edumazet@...gle.com, pabeni@...hat.com,
        asml.silence@...il.com, imagedong@...cent.com,
        luiz.von.dentz@...el.com, vasily.averin@...ux.dev,
        jk@...econstruct.com.au, linux-kernel@...r.kernel.org,
        netdev@...r.kernel.org, kernel test robot <lkp@...el.com>,
        linux-toolchains <linux-toolchains@...r.kernel.org>
Subject: Re: [PATCH net-next v4] net: skb: prevent the split of
 kfree_skb_reason() by gcc

Hello,
On Fri, Aug 19, 2022 at 11:24 PM Segher Boessenkool
<segher@...nel.crashing.org> wrote:
>
> Hi!
>
> On Fri, Aug 19, 2022 at 10:55:42PM +0800, Menglong Dong wrote:
> > Thanks for your explanation about the usage of 'noinline' and 'no_icf'!
> > I think 'noclone' seems enough in this case? As the function
> > 'kfree_skb_reason' we talk about is a global function, I think that the
> > compiler has no reason to make it inline, or be merged with another
> > function.
>
> Whether something is inlined is decided per instance (except for
> always_inline and noinline functions).  Of course the function body has
> to be available for anything to be inlined, so barring LTO this can only
> happen for function uses in the same source file.  Not very likely
> indeed, but not entirely impossible either.
>

I understand it now, the global function is indeed possible to be
made inline by the compiler, and 'noinline' seems necessary
here too.

Maybe I can add a new compiler attribute like this:

/*
 * Used by functions that use '__builtin_return_address'. These function
 * don't want to be splited or made inline, which can make
 * the '__builtin_return_address' got unexpected address.
 */
#define __fix_address noinline __noclone

> A function can be merged if there is another function that does exactly
> the same thing.  This is unlikely with functions that do some serious
> work of course, but it is likely with stub-like functions.
>

I understand how 'icf'(Identical Code Folding) works now. In the case
we talk about, It seems fine even if the function is merged. The only
effect of 'icf' is the change of function name, which doesn't affect
the result of '__builtin_return_address(0)'.

Thanks!
Menglong Dong

> gl;hf,
>
>
> Segher