netdev - Re: [Patch net] qrtr: orphan skb before queuing in xmit

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: <ee7386b1-fcdb-bd2d-fa8d-db87248dd7fd@gmail.com>
Date:   Thu, 23 Jul 2020 23:10:21 -0700
From:   Eric Dumazet <eric.dumazet@...il.com>
To:     Cong Wang <xiyou.wangcong@...il.com>,
        Eric Dumazet <eric.dumazet@...il.com>
Cc:     Linux Kernel Network Developers <netdev@...r.kernel.org>,
        syzbot <syzbot+6720d64f31c081c2f708@...kaller.appspotmail.com>,
        Bjorn Andersson <bjorn.andersson@...aro.org>
Subject: Re: [Patch net] qrtr: orphan skb before queuing in xmit



On 7/23/20 11:00 PM, Cong Wang wrote:
> On Thu, Jul 23, 2020 at 10:35 PM Eric Dumazet <eric.dumazet@...il.com> wrote:
>>
>>
>>
>> On 7/23/20 9:50 PM, Cong Wang wrote:
>>> Similar to tun_net_xmit(), we have to orphan the skb
>>> before queuing it, otherwise we may use the socket when
>>> purging the queue after it is freed by user-space.
>>
>> Which socket ?
> 
> sk->sk_wq points to &sock->wq. The socket is of course from
> qrtr_create().
> 
>>
>> By not calling skb_orphan(skb), this skb should own a reference on skb->sk preventing
>> skb->sk to disappear.
>>
> 
> I said socket, not sock. I believe the socket can be gone while the sock is
> still there.
> 
> 
>> It seems that instead of skb_orphan() here, we could avoid calling skb_set_owner_w() in the first place,
>> because this is confusing.
> 
> Not sure about this, at least tun calls skb_set_owner_w() too. More
> importantly, sock_alloc_send_skb() calls it too. :)
> 

tun is very different : skbs reaching it came come from all over the places, like TCP stack.
Their skb->sk is not pointing to the tun sock.


Here, the skbs are cooked from net/qrtr/qrtr.c locations only.