Received-SPF: pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67;
From:   Chris Packham <Chris.Packham@alliedtelesis.co.nz>
To:     Eric Dumazet <eric.dumazet@gmail.com>,
        "jon.maloy@ericsson.com" <jon.maloy@ericsson.com>,
        "ying.xue@windriver.com" <ying.xue@windriver.com>,
        "davem@davemloft.net" <davem@davemloft.net>
CC:     "netdev@vger.kernel.org" <netdev@vger.kernel.org>,
        "tipc-discussion@lists.sourceforge.net" 
        <tipc-discussion@lists.sourceforge.net>,
        "linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>
Subject: Re: [PATCH] tipc: ensure skb->lock is initialised
Thread-Topic: [PATCH] tipc: ensure skb->lock is initialised
Thread-Index: AQHVNRbM8H0dBbuBFkyTwxApCbaMkw==
Date:   Mon, 8 Jul 2019 21:13:45 +0000
Message-ID: <87fd2150548041219fc42bce80b63c9c@svr-chch-ex1.atlnz.lc>
References: <20190707225328.15852-1-chris.packham@alliedtelesis.co.nz>
 <2298b9eb-100f-6130-60c4-0e5e2c7b84d1@gmail.com>
 <361940337b0d4833a5634ddd1e1896a9@svr-chch-ex1.atlnz.lc>
Accept-Language: en-NZ, en-US
Content-Language: en-US
Content-Type: text/plain; charset="us-ascii"
Content-Transfer-Encoding: quoted-printable
MIME-Version: 1.0
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk

On 9/07/19 8:43 AM, Chris Packham wrote:=0A=
> On 8/07/19 8:18 PM, Eric Dumazet wrote:=0A=
>>=0A=
>>=0A=
>> On 7/8/19 12:53 AM, Chris Packham wrote:=0A=
>>> tipc_named_node_up() creates a skb list. It passes the list to=0A=
>>> tipc_node_xmit() which has some code paths that can call=0A=
>>> skb_queue_purge() which relies on the list->lock being initialised.=0A=
>>> Ensure tipc_named_node_up() uses skb_queue_head_init() so that the lock=
=0A=
>>> is explicitly initialised.=0A=
>>>=0A=
>>> Signed-off-by: Chris Packham <chris.packham@alliedtelesis.co.nz>=0A=
>>=0A=
>> I would rather change the faulty skb_queue_purge() to __skb_queue_purge(=
)=0A=
>>=0A=
> =0A=
> Makes sense. I'll look at that for v2.=0A=
> =0A=
=0A=
Actually maybe not. tipc_rcast_xmit(), tipc_node_xmit_skb(), =0A=
tipc_send_group_msg(), __tipc_sendmsg(), __tipc_sendstream(), and =0A=
tipc_sk_timeout() all use skb_queue_head_init(). So my original change =0A=
brings tipc_named_node_up() into line with them.=0A=
=0A=
I think it should be safe for tipc_node_xmit() to use =0A=
__skb_queue_purge() since all the callers seem to have exclusive access =0A=
to the list of skbs. It still seems that the callers should all use =0A=
skb_queue_head_init() for consistency.=0A=