linux-kernel - Re: [PATCH net-next 2/2] net sched act_vlan: VLAN action rewrite to use RCU lock/unlock and update

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: <20171011204100.GB9297@nanopsycho>
Date:   Wed, 11 Oct 2017 22:41:00 +0200
From:   Jiri Pirko <jiri@...nulli.us>
To:     Cong Wang <xiyou.wangcong@...il.com>
Cc:     Manish Kurup <kurup.manish@...il.com>,
        Jamal Hadi Salim <jhs@...atatu.com>,
        David Miller <davem@...emloft.net>,
        Linux Kernel Network Developers <netdev@...r.kernel.org>,
        LKML <linux-kernel@...r.kernel.org>,
        Alexander Aring <aring@...atatu.com>,
        Roman Mashak <mrv@...atatu.com>, manish.kurup@...izon.com
Subject: Re: [PATCH net-next 2/2] net sched act_vlan: VLAN action rewrite to
 use RCU lock/unlock and update

Wed, Oct 11, 2017 at 06:27:07PM CEST, xiyou.wangcong@...il.com wrote:
>On Tue, Oct 10, 2017 at 7:33 PM, Manish Kurup <kurup.manish@...il.com> wrote:

[...]


>> @@ -187,16 +196,33 @@ static int tcf_vlan_init(struct net *net, struct nlattr *nla,
>>
>>         v = to_vlan(*a);
>>
>> -       spin_lock_bh(&v->tcf_lock);
>> -
>> -       v->tcfv_action = action;
>> -       v->tcfv_push_vid = push_vid;
>> -       v->tcfv_push_prio = push_prio;
>> -       v->tcfv_push_proto = push_proto;
>> +       ASSERT_RTNL();
>> +       p = kzalloc(sizeof(*p), GFP_KERNEL);
>> +       if (unlikely(!p)) {
>> +               if (ovr)
>> +                       tcf_idr_release(*a, bind);
>> +               return -ENOMEM;
>> +       }
>>
>>         v->tcf_action = parm->action;
>>
>> -       spin_unlock_bh(&v->tcf_lock);
>> +       p_old = rtnl_dereference(v->vlan_p);
>> +
>> +       if (ovr)
>> +               spin_lock_bh(&v->tcf_lock);
>
>Why still take spinlock when you already have RTNL lock?
>What's the point?

Yeah, I believe this is copy&paste bug from act_skbmod