[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <20150713103911.GA9631@hmsreliant.think-freely.org>
Date: Mon, 13 Jul 2015 06:39:11 -0400
From: Neil Horman <nhorman@...driver.com>
To: David Miller <davem@...emloft.net>
Cc: marcelo.leitner@...il.com, netdev@...r.kernel.org,
vyasevich@...il.com
Subject: Re: [PATCH 1/2] sctp: SCTP_SOCKOPT_PEELOFF return socket pointer for
kernel users
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.
> 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
Regards
Neil
--
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
Powered by blists - more mailing lists