netdev - Re: [PATCH V3 5/8] sctp: use limited socket backlog

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:	Fri, 05 Mar 2010 08:30:01 -0500
From:	Vlad Yasevich <vladislav.yasevich@...com>
To:	"Zhu, Yi" <yi.zhu@...el.com>
CC:	Eric Dumazet <eric.dumazet@...il.com>,
	"davem@...emloft.net" <davem@...emloft.net>,
	"netdev@...r.kernel.org" <netdev@...r.kernel.org>,
	Sridhar Samudrala <sri@...ibm.com>
Subject: Re: [PATCH V3 5/8] sctp: use limited socket backlog



Zhu, Yi wrote:
> Eric Dumazet <eric.dumazet@...il.com> wrote:
> 
>> As advertized by comment, we should hold the association *before*
>> accessing backlog queue.
> 
>> If order is not important, comment should be relaxed somehow ?
> 
> I don't see how the order is important here. We are under sock_lock 
> here thus nobody will race with us. IMHO, the comment talks about
> if a packet is queued into the backlog, we need to increase the assoc/ep
> reference count. Otherwise the assoc/ep might be disappeared when
> we are about to process it (by sctp_backlog_rcv) sometime later.
> 
> Thanks,
> -yi

Yes, that's correct.  The order is not really important since we
are under lock and are actually already holding a ref.  However
the ref will be dropped once we exit the function, so the function
takes an additional ref that is held while the packet is backloged.

You could get rid of the extra nesting though by returning early
if backlog failed.

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