netdev - Re: [PATCH 2/2] net-timestamp: Make the clone operation stand-alone from phy timestamping

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]

Date:	Wed, 03 Sep 2014 16:02:39 -0700
From:	Alexander Duyck <alexander.h.duyck@...el.com>
To:	Eric Dumazet <eric.dumazet@...il.com>
CC:	netdev@...r.kernel.org, richardcochran@...il.com,
	davem@...emloft.net, willemb@...gle.com
Subject: Re: [PATCH 2/2] net-timestamp: Make the clone operation stand-alone
 from phy timestamping

On 09/03/2014 03:05 PM, Eric Dumazet wrote:
> On Wed, 2014-09-03 at 14:07 -0700, Eric Dumazet wrote:
> 
>>
>> Normally, if one skb holds a reference to a socket, it should have
>> skb->destructor set to a cleanup function.
>>
>> Otherwise, we rely on callers following a convention, to release sk
>> reference.
>>
>> If you believe its needed, it should be dully documented.
> 
> 
> BTW, skb_orphan() would BUG() on this case (skb->destrutor == NULL and
> skb->sk != NULL)

That was why we were setting skb->sk to null before passing it to
sock_queue_err_skb.  I think I found the reason why things were done the
way they were.  It looks like skb_orphan is called in
sock_queue_err_skb.  So any destructor added would be fired there before
before being replaced.

The only part I am not sure about is if that would actually be any sort
of issue.  Do we really need to hold the extra reference to the socket
all the way through the sock_queue_err_skb call or can we just let it
get dropped with the call to skb_orphan and let the fact that the
message is being queued be good enough?

I'm not sure due to the comment about "before exiting rcu section, make
sure dst is refcounted".  It kind of implies I should be able to hand
off the reference counts without worrying about the socket being freed
out from under me.

Thanks,

Alex

--
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