netdev - Re: [PATCH 1/2] sctp: SCTP_SOCKOPT_PEELOFF return socket pointer for kernel users

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 for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <55A3BADD.9040303@gmail.com>
Date:	Mon, 13 Jul 2015 10:19:25 -0300
From:	Marcelo Ricardo Leitner <marcelo.leitner@...il.com>
To:	Neil Horman <nhorman@...driver.com>,
	David Miller <davem@...emloft.net>
CC:	netdev@...r.kernel.org, vyasevich@...il.com
Subject: Re: [PATCH 1/2] sctp: SCTP_SOCKOPT_PEELOFF return socket pointer
 for kernel users

On 13-07-2015 07:39, Neil Horman wrote:
> On Fri, Jul 10, 2015 at 06:21:14PM -0700, David Miller wrote:
>> From: Marcelo Ricardo Leitner <marcelo.leitner@...il.com>
>> Date: Thu,  9 Jul 2015 11:15:19 -0300
>>
>>> SCTP has this operation to peel off associations from a given socket and
>>> create a new socket using this association. We currently have two ways
>>> to use this operation:
>>> - via getsockopt(), on which it will also create and return a file
>>>    descriptor for this new socket
>>> - via sctp_do_peeloff(), which is for kernel only
>>>
>>> The caveat with using sctp_do_peeloff() directly is that it creates a
>>> dependency to SCTP module, while all other operations are handled via
>>> kernel_{socket,sendmsg,getsockopt...}() interface. This causes the
>>> kernel to load SCTP module even when it's not directly used
>>>
>>> This patch then updates SCTP_SOCKOPT_PEELOFF so that for kernel users of
>>> this protocol it will not allocate a file descriptor but instead just
>>> return the socket pointer directly.
>>>
>>> If called by an user application it will work as before.
>>>
>>> Signed-off-by: Marcelo Ricardo Leitner <marcelo.leitner@...il.com>
>>
>> I do not like this at all.
>>
>> Socket option implementations should not change their behavior or what
>> datastructures they consume or return just because the socket happens
>> to be a kernel socket.
>>
> But in this case its necessecary, as the kernel here can't allocate an fd, due
> to serious leakage (see commit 2f2d76cc3e938389feee671b46252dde6880b3b7).
> Initially Marcelo had created duplicate code paths, one to return an fd, one to
> return a file struct.  If you would rather go in that direction, I'm sure he can
> propose it again, but that seems less correct to me than this solution.

Yes.

dlm is the only user of this option within kernel today and it causes 
serious problems, as Neil just referenced. Another good result of this 
implementation is that we are preventing such leakage from happening 
again in the future.

>> I'm not applying this series, sorry.
>>
>> Also, your patch series lacked an intial "PATCH 0/N" posting, so you
>> could at least spend the time to discuss this patch series at a high
>> level and explain your overall motivations.
>>
> That was in the initial posting.  It should have been reposted, but if you're
> interested:
> http://marc.info/?l=linux-sctp&m=143449456219518&w=2

My bad. Won't happen again.

Thanks,
Marcelo

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