On 2023-02-05 23:11, Tung Quang Nguyen wrote:
-----Original Message-----
From: Jon Maloy <jma...@redhat.com>
Sent: Saturday, February 4, 2023 1:02 AM
To: Tung Quang Nguyen <tung.q.ngu...@dektech.com.au>;
tipc-discussion@lists.sourceforge.net; ma...@donjonn.com;
ying....@windriver.com
Subject: Re: [tipc-discussion][PATCH v1 1/1] tipc: fix kernel warning when
sending SYN message
On 2023-01-31 23:03, Tung Nguyen wrote:
When sending a SYN message, this kernel stack trace is observed:
...
[ 13.396352] RIP: 0010:_copy_from_iter+0xb4/0x550
...
[ 13.398494] Call Trace:
[ 13.398630] <TASK>
[ 13.398630] ? __alloc_skb+0xed/0x1a0
[ 13.398630] tipc_msg_build+0x12c/0x670 [tipc]
[ 13.398630] ? shmem_add_to_page_cache.isra.71+0x151/0x290
[ 13.398630] __tipc_sendmsg+0x2d1/0x710 [tipc]
[ 13.398630] ? tipc_connect+0x1d9/0x230 [tipc]
[ 13.398630] ? __local_bh_enable_ip+0x37/0x80
[ 13.398630] tipc_connect+0x1d9/0x230 [tipc]
[ 13.398630] ? __sys_connect+0x9f/0xd0
[ 13.398630] __sys_connect+0x9f/0xd0
[ 13.398630] ? preempt_count_add+0x4d/0xa0
[ 13.398630] ? fpregs_assert_state_consistent+0x22/0x50
[ 13.398630] __x64_sys_connect+0x16/0x20
[ 13.398630] do_syscall_64+0x42/0x90
[ 13.398630] entry_SYSCALL_64_after_hwframe+0x63/0xcd
It is because commit a41dad905e5a ("iov_iter: saner checks for attempt to copy
to/from iterator")
has introduced sanity check for copying from/to iov iterator. Lacking
of copy direction from the iterator viewpoint would lead to kernel
stack trace like above.
This commit fixes this issue by initializing the iov iterator with
the correct copy direction.
Signed-off-by: Tung Nguyen <tung.q.ngu...@dektech.com.au>
---
net/tipc/msg.c | 4 ++++
1 file changed, 4 insertions(+)
diff --git a/net/tipc/msg.c b/net/tipc/msg.c
index 5c9fd4791c4b..f40cd9032b62 100644
--- a/net/tipc/msg.c
+++ b/net/tipc/msg.c
@@ -377,10 +377,14 @@ int tipc_msg_build(struct tipc_msg *mhdr, struct msghdr
*m, int offset,
int pktno = 1;
char *pktpos;
int pktsz;
+ struct iovec iov;
int rc;
msg_set_size(mhdr, msz);
+ if (!dsz)
+ iov_iter_init(&m->msg_iter, ITER_SOURCE, &iov, 0, 0);
+
/* No fragmentation needed? */
if (likely(msz <= pktmax)) {
skb = tipc_buf_acquire(msz, GFP_KERNEL);
I feel a little uncomfortable with adding an uninitialized struct iovec
here.
It is OK not using iovec, just passing NULL to iov_iter_init:
iov_iter_init(&m->msg_iter, ITER_SOURCE, NULL, 0, 0).
My intention was to highlight the copy source and no-copy (count = 0)
information.
Al Viro had a different proposal in his email from Dec 8th:
On 2022-12-11 04:38, Al Viro wrote:
On Thu, Dec 08, 2022 at 08:38:14PM +0100, Eric Dumazet wrote:
Exposes an old bug in tipc ?
Seems a new check added by Al in :
Author: Al Viro <v...@zeniv.linux.org.uk>
Date: Thu Sep 15 20:11:15 2022 -0400
iov_iter: saner checks for attempt to copy to/from iterator
instead of "don't do it to ITER_PIPE" check for ->data_source being
false on copying from iterator. Check for !->data_source for
copying to iterator, while we are at it.
Signed-off-by: Al Viro <v...@zeniv.linux.org.uk>
Lovely... zero-length sendmsg with uninitialized ->msg_data...
I would probably argue that it's a bug in tipc_connect(),
fixed by iov_iter_kvec(&m.msg_iter, ITER_SOURCE, NULL, 0, 0);
in there. Depends - if that kind of uninitialized msg_iter used
as zero length source or zero length destination is a frequent pattern,
might as well make zero-byte copy_...iter() succeed quietly;
I hope it isn't, but that's definitely something I'd missed
when doing that series.
I'll take a look tomorrow^Win the morning, after I get
some sleep...
Did you consider that one?
This function has the same effect as the one I used. But from the viewpoint of
function
tipc_msg_build(), we mainly want to copy data from user-space. Two exceptions
are SYN and ACK- with no data. So, I think using iov_iter_init() makes more
sense.
I suggest that we go with what I did (plus passing NULL for iovec) and CC Al
Viro to see if the guy has any objection.
What do you think ?
Yes, that makes sense. Go ahead.
///jon
///jon
_______________________________________________
tipc-discussion mailing list
tipc-discussion@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/tipc-discussion
