From: Stephen Rothwell <hidden> Date: 2015-03-13 02:15:52
Hi all,
Today's linux-next merge of the net-next tree got a conflict in
net/socket.c between commits 005139a14660 ("fs: remove ki_nbytes") and
e9eab93cc2dc ("fs: don't allow to complete sync iocbs through
aio_complete") from the vfs tree and commit 1b784140474e ("net: Remove
iocb argument from sendmsg and recvmsg") from the net-next tree.
I fixed it up (mainly using the net-next version - see below) and can
carry the fix as necessary (no action is required).
--
Cheers,
Stephen Rothwell sfr@canb.auug.org.au
diff --cc net/socket.c
index f6c519d7b3ba,95d3085cb477..000000000000
--- a/net/socket.c+++ b/net/socket.c
@@@ -855,11 -807,10 +807,10 @@@ static ssize_t sock_read_iter(struct ki
if (iocb->ki_pos != 0)
return -ESPIPE;
- if (iocb->ki_nbytes == 0) /* Match SYS5 behaviour */
+ if (!iov_iter_count(to)) /* Match SYS5 behaviour */
return 0;
- res = __sock_recvmsg(iocb, sock, &msg,
- iov_iter_count(to), msg.msg_flags);
- res = sock_recvmsg(sock, &msg, iocb->ki_nbytes, msg.msg_flags);
++ res = sock_recvmsg(sock, &msg, iov_iter_count(to), msg.msg_flags);
*to = msg.msg_iter;
return res;
}
@@@ -880,7 -831,7 +831,7 @@@ static ssize_t sock_write_iter(struct k
if (sock->type == SOCK_SEQPACKET)
msg.msg_flags |= MSG_EOR;
- res = __sock_sendmsg(iocb, sock, &msg, iov_iter_count(from));
- res = sock_sendmsg(sock, &msg, iocb->ki_nbytes);
++ res = sock_sendmsg(sock, &msg, iov_iter_count(from));
*from = msg.msg_iter;
return res;
}
From: David Miller <davem@davemloft.net> Date: 2015-03-13 03:24:30
From: Stephen Rothwell <redacted>
Date: Fri, 13 Mar 2015 13:15:43 +1100
Today's linux-next merge of the net-next tree got a conflict in
net/socket.c between commits 005139a14660 ("fs: remove ki_nbytes") and
e9eab93cc2dc ("fs: don't allow to complete sync iocbs through
aio_complete") from the vfs tree and commit 1b784140474e ("net: Remove
iocb argument from sendmsg and recvmsg") from the net-next tree.
I fixed it up (mainly using the net-next version - see below) and can
carry the fix as necessary (no action is required).
From: Al Viro <viro@ZenIV.linux.org.uk> Date: 2015-03-13 03:56:22
On Thu, Mar 12, 2015 at 11:24:26PM -0400, David Miller wrote:
From: Stephen Rothwell <redacted>
Date: Fri, 13 Mar 2015 13:15:43 +1100
quoted
Today's linux-next merge of the net-next tree got a conflict in
net/socket.c between commits 005139a14660 ("fs: remove ki_nbytes") and
e9eab93cc2dc ("fs: don't allow to complete sync iocbs through
aio_complete") from the vfs tree and commit 1b784140474e ("net: Remove
iocb argument from sendmsg and recvmsg") from the net-next tree.
I fixed it up (mainly using the net-next version - see below) and can
carry the fix as necessary (no action is required).
Al, how do you want to resolve this?
Hmm... I could backmerge 1b784140474e4fc94281a49e96c67d29df0efbde into
vfs.git#for-next, of course, but you've got quite a pile of stuff in front
of it... FWIW, the conflict resolution proposed by Stephen is correct;
the question is what should go into which tree.
Actually, prereqs of the commit in question on vfs.git side are mostly
-stable fodder; all it really needs is vfs.git#gadget and I was planning
to send that to Linus - fixes for leaks and use-after-free in gadgetfs
that had been there since forever, plus fixes for regression since 3.18
(->f_op flipping that had always been fishy and outright broke when we
started to FMODE_CAN_READ/FMODE_CAN_WRITE). USB folks seem to be OK
with it. Christoph's patch isn't a regression fix, but seeing that it's
(a) trivial and (b) ends up causing merge headache... Maybe it would
make sense to pull it into mainline and resolve the conflict on backmerge
from mainline to net-next. Linus? I've pushed that (gadget + ki_nbytes)
into vfs.git#for-linus-2; would you be OK with pulling that?
It's on
git://git.kernel.org/pub/scm/linux/kernel/git/viro/vfs.git for-linus-2
Shortlog:
Al Viro (8):
new helper: dup_iter()
move iov_iter.c from mm/ to lib/
gadget/function/f_fs.c: close leaks
gadget/function/f_fs.c: use put iov_iter into io_data
gadget/function/f_fs.c: switch to ->{read,write}_iter()
gadgetfs: use-after-free in ->aio_read()
gadget: switch ep_io_operations to ->read_iter/->write_iter
gadgetfs: get rid of flipping ->f_op in ep_config()
Alan Stern (1):
gadgetfs: really get rid of switching ->f_op
Christoph Hellwig (1):
fs: remove ki_nbytes
Diffstat:
drivers/usb/gadget/function/f_fs.c | 204 +++++++---------
drivers/usb/gadget/legacy/inode.c | 466 +++++++++++++++----------------------
fs/aio.c | 34 +--
fs/ceph/file.c | 2 +-
fs/nfs/direct.c | 2 +-
fs/ocfs2/file.c | 8 +-
fs/read_write.c | 8 -
fs/udf/file.c | 2 +-
include/linux/aio.h | 1 -
include/linux/uio.h | 2 +
kernel/printk/printk.c | 2 +-
lib/Makefile | 2 +-
{mm => lib}/iov_iter.c | 15 ++
mm/Makefile | 2 +-
mm/page_io.c | 1 -
net/socket.c | 6 +-
16 files changed, 319 insertions(+), 438 deletions(-)
rename {mm => lib}/iov_iter.c (97%)
From: Stephen Rothwell <hidden> Date: 2015-03-13 04:38:27
Hi Al,
On Fri, 13 Mar 2015 03:56:09 +0000 Al Viro [off-list ref] wrote:
On Thu, Mar 12, 2015 at 11:24:26PM -0400, David Miller wrote:
quoted
From: Stephen Rothwell <redacted>
Date: Fri, 13 Mar 2015 13:15:43 +1100
quoted
Today's linux-next merge of the net-next tree got a conflict in
net/socket.c between commits 005139a14660 ("fs: remove ki_nbytes") and
e9eab93cc2dc ("fs: don't allow to complete sync iocbs through
aio_complete") from the vfs tree and commit 1b784140474e ("net: Remove
iocb argument from sendmsg and recvmsg") from the net-next tree.
I fixed it up (mainly using the net-next version - see below) and can
carry the fix as necessary (no action is required).
Al, how do you want to resolve this?
Hmm... I could backmerge 1b784140474e4fc94281a49e96c67d29df0efbde into
vfs.git#for-next, of course, but you've got quite a pile of stuff in front
of it... FWIW, the conflict resolution proposed by Stephen is correct;
the question is what should go into which tree.
Actually, prereqs of the commit in question on vfs.git side are mostly
-stable fodder; all it really needs is vfs.git#gadget and I was planning
to send that to Linus - fixes for leaks and use-after-free in gadgetfs
that had been there since forever, plus fixes for regression since 3.18
(->f_op flipping that had always been fishy and outright broke when we
started to FMODE_CAN_READ/FMODE_CAN_WRITE). USB folks seem to be OK
with it. Christoph's patch isn't a regression fix, but seeing that it's
(a) trivial and (b) ends up causing merge headache... Maybe it would
make sense to pull it into mainline and resolve the conflict on backmerge
from mainline to net-next. Linus? I've pushed that (gadget + ki_nbytes)
into vfs.git#for-linus-2; would you be OK with pulling that?
It's on
git://git.kernel.org/pub/scm/linux/kernel/git/viro/vfs.git for-linus-2
Shortlog:
Al Viro (8):
new helper: dup_iter()
move iov_iter.c from mm/ to lib/
gadget/function/f_fs.c: close leaks
gadget/function/f_fs.c: use put iov_iter into io_data
gadget/function/f_fs.c: switch to ->{read,write}_iter()
gadgetfs: use-after-free in ->aio_read()
gadget: switch ep_io_operations to ->read_iter/->write_iter
gadgetfs: get rid of flipping ->f_op in ep_config()
Alan Stern (1):
gadgetfs: really get rid of switching ->f_op
Christoph Hellwig (1):
fs: remove ki_nbytes
There is also a conflict with e9eab93cc2dc ("fs: don't allow to
complete sync iocbs through aio_complete"), though it doesn't show up
in the resolution since I I just used the next-next tree bits. So a
common branch containing that as well could be merged into both trees.
But none of this is really that complex, maybe worth a hint to Linus
during the merge window. Unless there is more planned for that file?
--
Cheers,
Stephen Rothwell sfr@canb.auug.org.au
From: David Miller <davem@davemloft.net> Date: 2015-03-13 04:52:54
From: Al Viro <viro@ZenIV.linux.org.uk>
Date: Fri, 13 Mar 2015 03:56:09 +0000
On Thu, Mar 12, 2015 at 11:24:26PM -0400, David Miller wrote:
quoted
From: Stephen Rothwell <redacted>
Date: Fri, 13 Mar 2015 13:15:43 +1100
quoted
Today's linux-next merge of the net-next tree got a conflict in
net/socket.c between commits 005139a14660 ("fs: remove ki_nbytes") and
e9eab93cc2dc ("fs: don't allow to complete sync iocbs through
aio_complete") from the vfs tree and commit 1b784140474e ("net: Remove
iocb argument from sendmsg and recvmsg") from the net-next tree.
I fixed it up (mainly using the net-next version - see below) and can
carry the fix as necessary (no action is required).
Al, how do you want to resolve this?
Hmm... I could backmerge 1b784140474e4fc94281a49e96c67d29df0efbde into
vfs.git#for-next, of course, but you've got quite a pile of stuff in front
of it... FWIW, the conflict resolution proposed by Stephen is correct;
the question is what should go into which tree.
Actually, prereqs of the commit in question on vfs.git side are mostly
-stable fodder; all it really needs is vfs.git#gadget and I was planning
to send that to Linus - fixes for leaks and use-after-free in gadgetfs
that had been there since forever, plus fixes for regression since 3.18
(->f_op flipping that had always been fishy and outright broke when we
started to FMODE_CAN_READ/FMODE_CAN_WRITE). USB folks seem to be OK
with it. Christoph's patch isn't a regression fix, but seeing that it's
(a) trivial and (b) ends up causing merge headache... Maybe it would
make sense to pull it into mainline and resolve the conflict on backmerge
from mainline to net-next. Linus? I've pushed that (gadget + ki_nbytes)
into vfs.git#for-linus-2; would you be OK with pulling that?
Push your stuff to Linus, then I'll solve all of this by merging
Linus --> net --> net-next at some point, ok?
Meanwhile Stephen's fix will carry us over until then.
THanks!
From: Al Viro <viro@ZenIV.linux.org.uk> Date: 2015-03-13 16:37:25
On Fri, Mar 13, 2015 at 03:38:17PM +1100, Stephen Rothwell wrote:
There is also a conflict with e9eab93cc2dc ("fs: don't allow to
complete sync iocbs through aio_complete"), though it doesn't show up
in the resolution since I I just used the next-next tree bits. So a
common branch containing that as well could be merged into both trees.
OK, for now I've done just that (vfs.git#iocb in never-rebase mode).
I still think that vfs.git#gadget ought to go into mainline; arguments
for the rest of #iocb are weaker and merging it into net-next would
suffice; as the matter of fact, I have pending stuff for net-next touching
the same area (further reduction of ->sendmsg()/->recvmsg() argument lists;
total_len is redundant); might as well deal with that when feeding that
to Dave...
From: Al Viro <viro@ZenIV.linux.org.uk> Date: 2015-03-14 00:43:23
On Fri, Mar 13, 2015 at 04:37:07PM +0000, Al Viro wrote:
On Fri, Mar 13, 2015 at 03:38:17PM +1100, Stephen Rothwell wrote:
quoted
There is also a conflict with e9eab93cc2dc ("fs: don't allow to
complete sync iocbs through aio_complete"), though it doesn't show up
in the resolution since I I just used the next-next tree bits. So a
common branch containing that as well could be merged into both trees.
OK, for now I've done just that (vfs.git#iocb in never-rebase mode).
I still think that vfs.git#gadget ought to go into mainline; arguments
for the rest of #iocb are weaker and merging it into net-next would
suffice; as the matter of fact, I have pending stuff for net-next touching
the same area (further reduction of ->sendmsg()/->recvmsg() argument lists;
total_len is redundant); might as well deal with that when feeding that
to Dave...
FWIW, trying to resurrect ->recvmsg() side of those patches (broken by
the same 'remove iocb' series; sendmsg side also had been, but I've
already finished that one), I've run into an interesting question.
There seems to be a confusion about the flags we are passing. Unlike
sendmsg case, where we pass that in msg->msg_flags (and it's strictly an in
argument), here we have _two_ ways for passing them in - msg->msg_flags and
separate 'flags' argument.
AFAICS, the picture looks so:
recvmsg(2): flags comes from syscall argument, with DONTWAIT possibly
tacked on it. ->msg_flags comes it as well - CMSG_COMPAT and CMSG_CLOEXEC
bits. On the way out, ->msg_flags sans CMSG_COMPAT is copied to userland.
BTW, out of those two I would expect CMSG_COMPAT to be kept - after all,
it is related to the layout of what ->msg_control ends up pointing to...
recvmmsg(2): ditto
read(2) et.al.: ->msg_flags and flags alike get 0 or DONTWAIT, depending on
O_NONBLOCK state of file. On the way out ->msg_flags is discarded.
recvfrom(2): flags is done the same way as in recvmsg(2); ->msg_flags, AFAICS,
is simply uninitialized. Whatever had been on the stack. On the way out
->msg_flags is discarded.
kernel-side callers of kernel_recvmsg(): varies. flags is always explicitly
given; ->msg_flags is sometimes equal to it, sometimes zero, sometimes
uninitialized. AFAICS, the value of ->msg_flags on the way out is discarded,
except for three cases where we do further ->recvmsg on the same msghdr.
Now, the instances of ->recvmsg(), AFAICS, almost never check ->msg_flags
bits. Exceptions are CMSG_COMPAT, CMSG_CLOEXEC and (in one case each)
PEEK and TRUNC. In one really odd case (AF_CAIF/SOCK_SEQPACKET) - OOB
(as in "EOPNOTSUPP if it's set", very likely to have been cargo-culted
from the sendmsg side of things). PEEK one (in rxrpc_sendmsg()) is probably
also bogus; might be misspelled 'flags & MSG_PEEK'...
Places that check for CMSG_COMPAT and CMSG_CLOEXEC are only reached when
->msg_control is non-NULL, which excludes recvfrom(2) and, AFAICS, those
of kernel_recvmsg() callers that leave ->msg_flags uninitialized.
The place that checks for TRUNC (rawv6_recvmsg()) looks like it assumes
that ->msg_flags & MSG_TRUNC will be zero when it's called; relevant piece
is
copied = skb->len;
if (copied > len) {
copied = len;
msg->msg_flags |= MSG_TRUNC;
}
if (skb_csum_unnecessary(skb)) {
err = skb_copy_datagram_msg(skb, 0, msg, copied);
} else if (msg->msg_flags&MSG_TRUNC) {
if (__skb_checksum_complete(skb))
goto csum_copy_err;
err = skb_copy_datagram_msg(skb, 0, msg, copied);
} else {
and it smells like it means to check for skb->len > len...
If the above is true, all the places setting ->msg_flags to something non-zero
on the way into kernel_recvmsg() seem to be cargo-culting. Am I missing
something subtle here?