netdev - Re: [net-next PATCH v2 02/15] net: rcu-ify tcf

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: <5406B3DB.8010505@gmail.com>
Date:	Tue, 02 Sep 2014 23:23:23 -0700
From:	John Fastabend <john.fastabend@...il.com>
To:	David Miller <davem@...emloft.net>
CC:	xiyou.wangcong@...il.com, jhs@...atatu.com, eric.dumazet@...il.com,
	netdev@...r.kernel.org, paulmck@...ux.vnet.ibm.com,
	brouer@...hat.com
Subject: Re: [net-next PATCH v2 02/15] net: rcu-ify tcf_proto

On 09/02/2014 01:52 PM, David Miller wrote:
> From: John Fastabend <john.fastabend@...il.com>
> Date: Mon, 01 Sep 2014 18:39:13 -0700
>
>> On 08/24/2014 10:31 PM, David Miller wrote:
>>> From: John Fastabend <john.fastabend@...il.com>
>>> Date: Sun, 24 Aug 2014 17:48:31 -0700
>>>
>>>> @@ -722,8 +724,9 @@ static void sfq_free(void *addr)
>>>>    static void sfq_destroy(struct Qdisc *sch)
>>>>    {
>>>>    	struct sfq_sched_data *q = qdisc_priv(sch);
>>>> +	struct tcf_proto *fl = rtnl_dereference(q->filter_list);
>>>>
>>>> -	tcf_destroy_chain(&q->filter_list);
>>>> +	tcf_destroy_chain(&fl);
>>
>> Sorry for the delayed reply...
>>
>>>
>>> This will cause tcf_destroy_chain() to set the local variable
>>> 'fl' to NULL rather than q->filter_list.
>>>
>>> I don't see how this can be correct at all.
>>
>> Right now (without these patches) nothing sets q->filter_list
>> to NULL and we only call this when the qdisc is being destroyed.
>
> Yes, it does set it to NULL, by virtue of how the loop iterates.
>

yes of course its obvious now that you spell it out for me.

> It iterates by setting the "*fl" to tp->next until that evaluates to
> NULL.  Thereby setting *fl to NULL.
>
> So the old code would set ->filter_list to NULL, your code will not.
>

right I will fix this in the morning along with the sparse
warnings related to the tcf_find signatures.

Thanks
John


-- 
John Fastabend         Intel Corporation
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html