netdev - Re: [PATCH] net: Fix suspicious RCU usage in bpf_sk_reuseport

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:   Wed, 17 Aug 2022 11:00:13 +0800
From:   Hawkins Jiawei <yin31149@...il.com>
To:     kuba@...nel.org
Cc:     andrii@...nel.org, ast@...nel.org, bpf@...r.kernel.org,
        daniel@...earbox.net, davem@...emloft.net, dhowells@...hat.com,
        edumazet@...gle.com, jakub@...udflare.com,
        john.fastabend@...il.com, kafai@...com, kpsingh@...nel.org,
        linux-kernel-mentees@...ts.linuxfoundation.org,
        linux-kernel@...r.kernel.org, netdev@...r.kernel.org,
        pabeni@...hat.com, songliubraving@...com, yhs@...com,
        yin31149@...il.com
Subject: Re: [PATCH] net: Fix suspicious RCU usage in bpf_sk_reuseport_detach()

On Wed, 17 Aug 2022 at 07:44, Jakub Kicinski <kuba@...nel.org> wrote:
>
> On Tue, 16 Aug 2022 22:16:46 +0100 David Howells wrote:
> > So either __rcu_dereference_sk_user_data_with_flags_check() has to be a macro,
> > or we need to go with something like the first version of my patch where I
> > don't pass the condition through.  Do you have a preference?
>
> I like your version because it documents what the lock protecting this
> field is.
In my opinion, I still think we'd better refactor it to a more
geniric function, to avoid adding new functions when meeting
the same problem. However, if this is just a standalone problem,
maybe David's version shoule be better.

So maybe apply the David's version, and refactor it later when
meeting the same problem next time if needed.

On Wed, 17 Aug 2022 at 08:43, Martin KaFai Lau <kafai@...com> wrote:
>
> On Tue, Aug 16, 2022 at 04:44:35PM -0700, Jakub Kicinski wrote:
> >
> > One the naming - maybe just drop the _with_flags() ? There's no version
> > of locked helper which does not take the flags. And not underscores?
> I am also good with a shorter name.
I also agree, the name is really long.