netdev - Re: [PATCH net-next v6 10/11] net: sched: atomically check-allocate action

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:   Tue, 7 Aug 2018 18:20:19 -0700
From:   Cong Wang <xiyou.wangcong@...il.com>
To:     Vlad Buslov <vladbu@...lanox.com>
Cc:     Linux Kernel Network Developers <netdev@...r.kernel.org>,
        David Miller <davem@...emloft.net>,
        Jamal Hadi Salim <jhs@...atatu.com>,
        Jiri Pirko <jiri@...nulli.us>,
        Alexei Starovoitov <ast@...nel.org>,
        Daniel Borkmann <daniel@...earbox.net>,
        Yevgeny Kliteynik <kliteyn@...lanox.com>,
        Jiri Pirko <jiri@...lanox.com>
Subject: Re: [PATCH net-next v6 10/11] net: sched: atomically check-allocate action

On Thu, Jul 5, 2018 at 7:24 AM Vlad Buslov <vladbu@...lanox.com> wrote:
>
> Implement function that atomically checks if action exists and either takes
> reference to it, or allocates idr slot for action index to prevent
> concurrent allocations of actions with same index. Use EBUSY error pointer
> to indicate that idr slot is reserved.

A dumb question:

How could "concurrent allocations of actions with same index" happen
as you already take idrinfo->lock for the whole tcf_idr_check_alloc()??

For me, it should be only one allocation could succeed, all others
should fail.

Maybe you are trying to prevent others treat it like existing one,
but in that case you can just hold the idinfo->lock for all idr operations.

And more importantly, upper layer is able to tell it is a creation or
just replace, you don't have to check this in this complicated way.

IOW, all of these complicated code should not exist.