From: David Howells <dhowells@redhat.com>
To: Christian Brauner <christian@brauner.io>,
Steve French <sfrench@samba.org>
Cc: David Howells <dhowells@redhat.com>,
Paulo Alcantara <pc@manguebit.com>,
netfs@lists.linux.dev, linux-afs@lists.infradead.org,
linux-cifs@vger.kernel.org, linux-nfs@vger.kernel.org,
ceph-devel@vger.kernel.org, v9fs@lists.linux.dev,
linux-fsdevel@vger.kernel.org, linux-kernel@vger.kernel.org,
Stefan Metzmacher <metze@samba.org>,
Steve French <stfrench@microsoft.com>,
Tom Talpey <tom@talpey.com>, Matthew Wilcox <willy@infradead.org>
Subject: [PATCH v2 12/16] cifs: Fix reading into an ITER_FOLIOQ from the smbdirect code
Date: Wed, 25 Jun 2025 17:42:07 +0100 [thread overview]
Message-ID: <20250625164213.1408754-13-dhowells@redhat.com> (raw)
In-Reply-To: <20250625164213.1408754-1-dhowells@redhat.com>
When performing a file read from RDMA, smbd_recv() prints an "Invalid msg
type 4" error and fails the I/O. This is due to the switch-statement there
not handling the ITER_FOLIOQ handed down from netfslib.
Fix this by collapsing smbd_recv_buf() and smbd_recv_page() into
smbd_recv() and just using copy_to_iter() instead of memcpy(). This
future-proofs the function too, in case more ITER_* types are added.
Fixes: ee4cdf7ba857 ("netfs: Speed up buffered reading")
Reported-by: Stefan Metzmacher <metze@samba.org>
Signed-off-by: David Howells <dhowells@redhat.com>
cc: Steve French <stfrench@microsoft.com>
cc: Tom Talpey <tom@talpey.com>
cc: Paulo Alcantara (Red Hat) <pc@manguebit.com>
cc: Matthew Wilcox <willy@infradead.org>
cc: linux-cifs@vger.kernel.org
cc: netfs@lists.linux.dev
cc: linux-fsdevel@vger.kernel.org
---
fs/smb/client/smbdirect.c | 114 +++++++-------------------------------
1 file changed, 19 insertions(+), 95 deletions(-)
diff --git a/fs/smb/client/smbdirect.c b/fs/smb/client/smbdirect.c
index a976bcf61226..5fa46b2e682c 100644
--- a/fs/smb/client/smbdirect.c
+++ b/fs/smb/client/smbdirect.c
@@ -1770,35 +1770,39 @@ struct smbd_connection *smbd_get_connection(
}
/*
- * Receive data from receive reassembly queue
+ * Receive data from the transport's receive reassembly queue
* All the incoming data packets are placed in reassembly queue
- * buf: the buffer to read data into
+ * iter: the buffer to read data into
* size: the length of data to read
* return value: actual data read
- * Note: this implementation copies the data from reassebmly queue to receive
+ *
+ * Note: this implementation copies the data from reassembly queue to receive
* buffers used by upper layer. This is not the optimal code path. A better way
* to do it is to not have upper layer allocate its receive buffers but rather
* borrow the buffer from reassembly queue, and return it after data is
* consumed. But this will require more changes to upper layer code, and also
* need to consider packet boundaries while they still being reassembled.
*/
-static int smbd_recv_buf(struct smbd_connection *info, char *buf,
- unsigned int size)
+int smbd_recv(struct smbd_connection *info, struct msghdr *msg)
{
struct smbdirect_socket *sc = &info->socket;
struct smbd_response *response;
struct smbdirect_data_transfer *data_transfer;
+ size_t size = iov_iter_count(&msg->msg_iter);
int to_copy, to_read, data_read, offset;
u32 data_length, remaining_data_length, data_offset;
int rc;
+ if (WARN_ON_ONCE(iov_iter_rw(&msg->msg_iter) == WRITE))
+ return -EINVAL; /* It's a bug in upper layer to get there */
+
again:
/*
* No need to hold the reassembly queue lock all the time as we are
* the only one reading from the front of the queue. The transport
* may add more entries to the back of the queue at the same time
*/
- log_read(INFO, "size=%d info->reassembly_data_length=%d\n", size,
+ log_read(INFO, "size=%zd info->reassembly_data_length=%d\n", size,
info->reassembly_data_length);
if (info->reassembly_data_length >= size) {
int queue_length;
@@ -1836,7 +1840,10 @@ static int smbd_recv_buf(struct smbd_connection *info, char *buf,
if (response->first_segment && size == 4) {
unsigned int rfc1002_len =
data_length + remaining_data_length;
- *((__be32 *)buf) = cpu_to_be32(rfc1002_len);
+ __be32 rfc1002_hdr = cpu_to_be32(rfc1002_len);
+ if (copy_to_iter(&rfc1002_hdr, sizeof(rfc1002_hdr),
+ &msg->msg_iter) != sizeof(rfc1002_hdr))
+ return -EFAULT;
data_read = 4;
response->first_segment = false;
log_read(INFO, "returning rfc1002 length %d\n",
@@ -1845,10 +1852,9 @@ static int smbd_recv_buf(struct smbd_connection *info, char *buf,
}
to_copy = min_t(int, data_length - offset, to_read);
- memcpy(
- buf + data_read,
- (char *)data_transfer + data_offset + offset,
- to_copy);
+ if (copy_to_iter((char *)data_transfer + data_offset + offset,
+ to_copy, &msg->msg_iter) != to_copy)
+ return -EFAULT;
/* move on to the next buffer? */
if (to_copy == data_length - offset) {
@@ -1893,6 +1899,8 @@ static int smbd_recv_buf(struct smbd_connection *info, char *buf,
data_read, info->reassembly_data_length,
info->first_entry_offset);
read_rfc1002_done:
+ /* SMBDirect will read it all or nothing */
+ msg->msg_iter.count = 0;
return data_read;
}
@@ -1913,90 +1921,6 @@ static int smbd_recv_buf(struct smbd_connection *info, char *buf,
goto again;
}
-/*
- * Receive a page from receive reassembly queue
- * page: the page to read data into
- * to_read: the length of data to read
- * return value: actual data read
- */
-static int smbd_recv_page(struct smbd_connection *info,
- struct page *page, unsigned int page_offset,
- unsigned int to_read)
-{
- struct smbdirect_socket *sc = &info->socket;
- int ret;
- char *to_address;
- void *page_address;
-
- /* make sure we have the page ready for read */
- ret = wait_event_interruptible(
- info->wait_reassembly_queue,
- info->reassembly_data_length >= to_read ||
- sc->status != SMBDIRECT_SOCKET_CONNECTED);
- if (ret)
- return ret;
-
- /* now we can read from reassembly queue and not sleep */
- page_address = kmap_atomic(page);
- to_address = (char *) page_address + page_offset;
-
- log_read(INFO, "reading from page=%p address=%p to_read=%d\n",
- page, to_address, to_read);
-
- ret = smbd_recv_buf(info, to_address, to_read);
- kunmap_atomic(page_address);
-
- return ret;
-}
-
-/*
- * Receive data from transport
- * msg: a msghdr point to the buffer, can be ITER_KVEC or ITER_BVEC
- * return: total bytes read, or 0. SMB Direct will not do partial read.
- */
-int smbd_recv(struct smbd_connection *info, struct msghdr *msg)
-{
- char *buf;
- struct page *page;
- unsigned int to_read, page_offset;
- int rc;
-
- if (iov_iter_rw(&msg->msg_iter) == WRITE) {
- /* It's a bug in upper layer to get there */
- cifs_dbg(VFS, "Invalid msg iter dir %u\n",
- iov_iter_rw(&msg->msg_iter));
- rc = -EINVAL;
- goto out;
- }
-
- switch (iov_iter_type(&msg->msg_iter)) {
- case ITER_KVEC:
- buf = msg->msg_iter.kvec->iov_base;
- to_read = msg->msg_iter.kvec->iov_len;
- rc = smbd_recv_buf(info, buf, to_read);
- break;
-
- case ITER_BVEC:
- page = msg->msg_iter.bvec->bv_page;
- page_offset = msg->msg_iter.bvec->bv_offset;
- to_read = msg->msg_iter.bvec->bv_len;
- rc = smbd_recv_page(info, page, page_offset, to_read);
- break;
-
- default:
- /* It's a bug in upper layer to get there */
- cifs_dbg(VFS, "Invalid msg type %d\n",
- iov_iter_type(&msg->msg_iter));
- rc = -EINVAL;
- }
-
-out:
- /* SMBDirect will read it all or nothing */
- if (rc > 0)
- msg->msg_iter.count = 0;
- return rc;
-}
-
/*
* Send data to transport
* Each rqst is transported as a SMBDirect payload
next prev parent reply other threads:[~2025-06-25 16:43 UTC|newest]
Thread overview: 23+ messages / expand[flat|nested] mbox.gz Atom feed top
2025-06-25 16:41 [PATCH v2 00/16] netfs, cifs: Fixes to retry-related code and RDMA support David Howells
2025-06-25 16:41 ` [PATCH v2 01/16] netfs: Fix hang due to missing case in final DIO read result collection David Howells
2025-06-28 5:16 ` Steve French
2025-06-25 16:41 ` [PATCH v2 02/16] netfs: Put double put of request David Howells
2025-06-25 16:41 ` [PATCH v2 03/16] netfs: Provide helpers to perform NETFS_RREQ_IN_PROGRESS flag wangling David Howells
2025-06-25 16:41 ` [PATCH v2 04/16] netfs: Fix looping in wait functions David Howells
2025-06-25 16:42 ` [PATCH v2 05/16] netfs: Fix ref leak on inserted extra subreq in write retry David Howells
2025-06-25 16:42 ` [PATCH v2 06/16] smb: client: set missing retry flag in smb2_writev_callback() David Howells
2025-06-25 16:42 ` [PATCH v2 07/16] smb: client: set missing retry flag in cifs_readv_callback() David Howells
2025-06-25 16:42 ` [PATCH v2 08/16] smb: client: set missing retry flag in cifs_writev_callback() David Howells
2025-06-25 16:42 ` [PATCH v2 09/16] smb: client: fix regression with native SMB symlinks David Howells
2025-06-25 16:42 ` [PATCH v2 10/16] smb: client: fix warning when reconnecting channel David Howells
2025-06-25 16:42 ` [PATCH v2 11/16] smb: client: let smbd_post_send_iter() respect the peers max_send_size and transmit all data David Howells
2025-06-25 16:42 ` David Howells [this message]
2025-06-25 17:23 ` [PATCH v2 12/16] cifs: Fix reading into an ITER_FOLIOQ from the smbdirect code Stefan Metzmacher
2025-06-25 17:55 ` [PATCH v3 " David Howells
2025-06-25 18:07 ` Stefan Metzmacher
2025-06-25 18:24 ` Tom Talpey
2025-06-25 18:53 ` David Howells
2025-06-25 16:42 ` [PATCH v2 13/16] cifs: Fix the smbd_reponse slab to allow usercopy David Howells
2025-06-25 16:42 ` [PATCH v2 14/16] smb: client: fix potential deadlock when reconnecting channels David Howells
2025-06-25 16:42 ` [PATCH v2 15/16] netfs: Renumber the NETFS_RREQ_* flags to make traces easier to read David Howells
2025-06-25 16:42 ` [PATCH v2 16/16] netfs: Update tracepoints in a number of ways David Howells
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20250625164213.1408754-13-dhowells@redhat.com \
--to=dhowells@redhat.com \
--cc=ceph-devel@vger.kernel.org \
--cc=christian@brauner.io \
--cc=linux-afs@lists.infradead.org \
--cc=linux-cifs@vger.kernel.org \
--cc=linux-fsdevel@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-nfs@vger.kernel.org \
--cc=metze@samba.org \
--cc=netfs@lists.linux.dev \
--cc=pc@manguebit.com \
--cc=sfrench@samba.org \
--cc=stfrench@microsoft.com \
--cc=tom@talpey.com \
--cc=v9fs@lists.linux.dev \
--cc=willy@infradead.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).