Re: [RFC][PATCHES] sock_alloc_file() cleanups and fixes
On Tue, Dec 05, 2017 at 02:44:43PM -0500, David Miller wrote: > From: Al Viro> Date: Mon, 4 Dec 2017 16:41:01 + > > > On Mon, Dec 04, 2017 at 10:35:24AM -0500, David Miller wrote: > >> From: Al Viro > >> Date: Fri, 1 Dec 2017 00:20:27 + > >> > >> > 1) massage sys_socketpair() (should be a pure cleanup) > >> > 2) fix and clean up kcm_clone() (-stable fodder) > >> > 3) switch sock_alloc_file() to new calling conventions. > >> > > >> > It got some local testing, but it certainly needs more review. > >> > Diffstat for the entire thing is > >> > >> Series looks great to me: > >> > >> Acked-by: David S. Miller > > > > How do you prefer it to be handled? KCM one should go into everything > > since 4.6 (with trivial modifications in 4.11 and 4.12 - both had > > massaged the place around the call of kcm_clone() a bit, but this fix > > overwrites the entire area and that can be dropped into earlier > > kernels without any problems). I've put that into vfs.git#net-fixes > > and have the other two in vfs.git#for-davem on top of that, with > > you merging the latter into net-next.git and the former - into net.git. > > Is that OK with you, or would you prefer some other way of handling > > that kind of stuff? > > Why don't you resubmit this to netdev as a non-RFC, I'll queue it up to > 'net' and -stable as well. Sent...
Re: [RFC][PATCHES] sock_alloc_file() cleanups and fixes
From: Al ViroDate: Mon, 4 Dec 2017 16:41:01 + > On Mon, Dec 04, 2017 at 10:35:24AM -0500, David Miller wrote: >> From: Al Viro >> Date: Fri, 1 Dec 2017 00:20:27 + >> >> >1) massage sys_socketpair() (should be a pure cleanup) >> >2) fix and clean up kcm_clone() (-stable fodder) >> >3) switch sock_alloc_file() to new calling conventions. >> > >> >It got some local testing, but it certainly needs more review. >> > Diffstat for the entire thing is >> >> Series looks great to me: >> >> Acked-by: David S. Miller > > How do you prefer it to be handled? KCM one should go into everything > since 4.6 (with trivial modifications in 4.11 and 4.12 - both had > massaged the place around the call of kcm_clone() a bit, but this fix > overwrites the entire area and that can be dropped into earlier > kernels without any problems). I've put that into vfs.git#net-fixes > and have the other two in vfs.git#for-davem on top of that, with > you merging the latter into net-next.git and the former - into net.git. > Is that OK with you, or would you prefer some other way of handling > that kind of stuff? Why don't you resubmit this to netdev as a non-RFC, I'll queue it up to 'net' and -stable as well. Thanks Al.
Re: [RFC][PATCHES] sock_alloc_file() cleanups and fixes
On Mon, Dec 04, 2017 at 10:35:24AM -0500, David Miller wrote: > From: Al Viro> Date: Fri, 1 Dec 2017 00:20:27 + > > > 1) massage sys_socketpair() (should be a pure cleanup) > > 2) fix and clean up kcm_clone() (-stable fodder) > > 3) switch sock_alloc_file() to new calling conventions. > > > > It got some local testing, but it certainly needs more review. > > Diffstat for the entire thing is > > Series looks great to me: > > Acked-by: David S. Miller How do you prefer it to be handled? KCM one should go into everything since 4.6 (with trivial modifications in 4.11 and 4.12 - both had massaged the place around the call of kcm_clone() a bit, but this fix overwrites the entire area and that can be dropped into earlier kernels without any problems). I've put that into vfs.git#net-fixes and have the other two in vfs.git#for-davem on top of that, with you merging the latter into net-next.git and the former - into net.git. Is that OK with you, or would you prefer some other way of handling that kind of stuff?
Re: [RFC][PATCHES] sock_alloc_file() cleanups and fixes
From: Al ViroDate: Fri, 1 Dec 2017 00:20:27 + > 1) massage sys_socketpair() (should be a pure cleanup) > 2) fix and clean up kcm_clone() (-stable fodder) > 3) switch sock_alloc_file() to new calling conventions. > > It got some local testing, but it certainly needs more review. > Diffstat for the entire thing is Series looks great to me: Acked-by: David S. Miller
[RFC][PATCHES] sock_alloc_file() cleanups and fixes
Almost all sock_alloc_file() callers want sock_release() in case of failure. Currently it consumes socket on success (it will be destroyed on final fput() of resulting file) and leaves it alone on failure. Making it consume socket in all cases makes for simpler life in callers. There are 3 exceptions: * sock_map_fd() calls sock_alloc_file(), but doesn't do sock_release() in case of failure. Its caller (sys_socket()) does, though, and it does get considerably simpler with sock_alloc_file() doing the cleanup in case of failure. * sys_socketpair(). Handling of sock_alloc_file() failures is complicated by attempts to share bits and pieces of failure exits between various points of failure in there. Reordering things a bit (reserving descriptors and copying them to userland before doing anything else) makes for simpler handling of failure exits and after such massage we get the situation when failure of sock_alloc_file() is immediately followed by sock_release(). * kcm_clone(). Badly broken in several respects - sk_alloc() failure ends up with double-free of struct socket (we do fput(), then sock_release()) and copy_to_user() failure uses sys_close() to undo fd_install(), which is something we should never do. Descriptor table might be shared and fd_install() should only be done past the last possible failure point. Fixing all of that is simple - we just need to move allocation of descriptor and fd_install() into the caller (before and after the call of kcm_clone(), resp.) and untangle the failure exits. Makes for much simpler calling conventions for kcm_clone(), while we are at it, and as a side effect we get "sock_release() in case of sock_alloc_file() failure" for that one as well. The patch series (in followups to this mail and in git://git.kernel.org/pub/scm/linux/kernel/git/viro/vfs.git#work.net) does the following: 1) massage sys_socketpair() (should be a pure cleanup) 2) fix and clean up kcm_clone() (-stable fodder) 3) switch sock_alloc_file() to new calling conventions. It got some local testing, but it certainly needs more review. Diffstat for the entire thing is drivers/staging/lustre/lnet/lnet/lib-socket.c | 8 ++--- net/9p/trans_fd.c | 1 - net/kcm/kcmsock.c | 68 ++--- net/sctp/socket.c | 1 - net/socket.c | 110 +++ 5 files changed, 69 insertions(+), 119 deletions(-) Please, review and comment.