netdev - Re: [PATCH net-next 11/12] net: sched: flower: track rtnl lock state

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]

Message-ID: <vbfmumtqw08.fsf@mellanox.com>
Date:   Mon, 18 Feb 2019 09:35:55 +0000
From:   Vlad Buslov <vladbu@...lanox.com>
To:     Stefano Brivio <sbrivio@...hat.com>
CC:     "netdev@...r.kernel.org" <netdev@...r.kernel.org>,
        "jhs@...atatu.com" <jhs@...atatu.com>,
        "xiyou.wangcong@...il.com" <xiyou.wangcong@...il.com>,
        "jiri@...nulli.us" <jiri@...nulli.us>,
        "davem@...emloft.net" <davem@...emloft.net>
Subject: Re: [PATCH net-next 11/12] net: sched: flower: track rtnl lock state


On Fri 15 Feb 2019 at 22:46, Stefano Brivio <sbrivio@...hat.com> wrote:
> On Thu, 14 Feb 2019 09:47:11 +0200
> Vlad Buslov <vladbu@...lanox.com> wrote:
>
>>  static int fl_hw_replace_filter(struct tcf_proto *tp,
>> -				struct cls_fl_filter *f,
>> +				struct cls_fl_filter *f, bool rtnl_held,
>>  				struct netlink_ext_ack *extack)
>>  {
>>  	struct tc_cls_flower_offload cls_flower = {};
>>  	struct tcf_block *block = tp->chain->block;
>>  	bool skip_sw = tc_skip_sw(f->flags);
>> -	int err;
>> +	int err = 0;
>> +
>> +	if (!rtnl_held)
>> +		rtnl_lock();
>>  
>>  	cls_flower.rule = flow_rule_alloc(tcf_exts_num_actions(&f->exts));
>>  	if (!cls_flower.rule)
>
>                 return -ENOMEM;
>
> Don't you need to:
>
> 		err = -ENOMEM;
> 		goto errout;
>
> here?
>
> Same...
>
>         err = tc_setup_flow_action(&cls_flower.rule->action, &f->exts);
>         if (err) {
>                 kfree(cls_flower.rule);
>                 if (skip_sw) {
>                         NL_SET_ERR_MSG_MOD(extack, "Failed to setup flow action");
>                         return err;
>
> here,
>
>                 }
>                 return 0;
>
> and here.

Thanks for catching this!
These error handlers were introduced by recent Pablo's patches and I
missed them during rebase.