[PATCH net] nvme-tcp: Fix comma-related oops

[PATCH net] nvme-tcp: Fix comma-related oops

[David Howells]

Fix a comma that should be a semicolon.  The comma is at the end of an
if-body and thus makes the statement after (a bvec_set_page()) conditional
too, resulting in an oops because we didn't fill out the bio_vec[]:

    BUG: kernel NULL pointer dereference, address: 0000000000000008
    #PF: supervisor read access in kernel mode
    #PF: error_code(0x0000) - not-present page
    ...
    Workqueue: nvme_tcp_wq nvme_tcp_io_work [nvme_tcp]
    RIP: 0010:skb_splice_from_iter+0xf1/0x370
    ...
    Call Trace:
     tcp_sendmsg_locked+0x3a6/0xdd0
     tcp_sendmsg+0x31/0x50
     inet_sendmsg+0x47/0x80
     sock_sendmsg+0x99/0xb0
     nvme_tcp_try_send_data+0x149/0x490 [nvme_tcp]
     nvme_tcp_try_send+0x1b7/0x300 [nvme_tcp]
     nvme_tcp_io_work+0x40/0xc0 [nvme_tcp]
     process_one_work+0x21c/0x430
     worker_thread+0x54/0x3e0
     kthread+0xf8/0x130

Fixes: 7769887817c3 ("nvme-tcp: Use sendmsg(MSG_SPLICE_PAGES) rather then sendpage")
Reported-by: Aurelien Aptel <aaptel@nvidia.com>
Link: https://lore.kernel.org/r/253mt0il43o.fsf@mtr-vdi-124.i-did-not-set--mail-host-address--so-tickle-me/
Signed-off-by: David Howells <dhowells@redhat.com>
cc: Sagi Grimberg <sagi@grimberg.me>
cc: Willem de Bruijn <willemb@google.com>
cc: Keith Busch <kbusch@kernel.org>
cc: Jens Axboe <axboe@fb.com>
cc: Christoph Hellwig <hch@lst.de>
cc: Chaitanya Kulkarni <kch@nvidia.com>
cc: "David S. Miller" <davem@davemloft.net>
cc: Eric Dumazet <edumazet@google.com>
cc: Jakub Kicinski <kuba@kernel.org>
cc: Paolo Abeni <pabeni@redhat.com>
cc: Jens Axboe <axboe@kernel.dk>
cc: Jens Axboe <axboe@kernel.dk>
cc: Matthew Wilcox <willy@infradead.org>
cc: linux-nvme@lists.infradead.org
cc: netdev@vger.kernel.org
---
 drivers/nvme/host/tcp.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/nvme/host/tcp.c b/drivers/nvme/host/tcp.c
index 3e7dd6f91832..9ce417cd32a7 100644
--- a/drivers/nvme/host/tcp.c
+++ b/drivers/nvme/host/tcp.c
@@ -1014,7 +1014,7 @@ static int nvme_tcp_try_send_data(struct nvme_tcp_request *req)
 			msg.msg_flags |= MSG_MORE;
 
 		if (!sendpage_ok(page))
-			msg.msg_flags &= ~MSG_SPLICE_PAGES,
+			msg.msg_flags &= ~MSG_SPLICE_PAGES;
 
 		bvec_set_page(&bvec, page, len, offset);
 		iov_iter_bvec(&msg.msg_iter, ITER_SOURCE, &bvec, 1, len);

Re: [PATCH net] nvme-tcp: Fix comma-related oops

[Chaitanya Kulkarni]

On 6/29/2023 2:47 PM, David Howells wrote:
> Fix a comma that should be a semicolon.  The comma is at the end of an
> if-body and thus makes the statement after (a bvec_set_page()) conditional
> too, resulting in an oops because we didn't fill out the bio_vec[]:
> 
>      BUG: kernel NULL pointer dereference, address: 0000000000000008
>      #PF: supervisor read access in kernel mode
>      #PF: error_code(0x0000) - not-present page
>      ...
>      Workqueue: nvme_tcp_wq nvme_tcp_io_work [nvme_tcp]
>      RIP: 0010:skb_splice_from_iter+0xf1/0x370
>      ...
>      Call Trace:
>       tcp_sendmsg_locked+0x3a6/0xdd0
>       tcp_sendmsg+0x31/0x50
>       inet_sendmsg+0x47/0x80
>       sock_sendmsg+0x99/0xb0
>       nvme_tcp_try_send_data+0x149/0x490 [nvme_tcp]
>       nvme_tcp_try_send+0x1b7/0x300 [nvme_tcp]
>       nvme_tcp_io_work+0x40/0xc0 [nvme_tcp]
>       process_one_work+0x21c/0x430
>       worker_thread+0x54/0x3e0
>       kthread+0xf8/0x130
> 
> Fixes: 7769887817c3 ("nvme-tcp: Use sendmsg(MSG_SPLICE_PAGES) rather then sendpage")
> Reported-by: Aurelien Aptel <aaptel@nvidia.com>
> Link: https://lore.kernel.org/r/253mt0il43o.fsf@mtr-vdi-124.i-did-not-set--mail-host-address--so-tickle-me/
> Signed-off-by: David Howells <dhowells@redhat.com>
> cc: Sagi Grimberg <sagi@grimberg.me>
> cc: Willem de Bruijn <willemb@google.com>
> cc: Keith Busch <kbusch@kernel.org>
> cc: Jens Axboe <axboe@fb.com>
> cc: Christoph Hellwig <hch@lst.de>
> cc: Chaitanya Kulkarni <kch@nvidia.com>
> cc: "David S. Miller" <davem@davemloft.net>
> cc: Eric Dumazet <edumazet@google.com>
> cc: Jakub Kicinski <kuba@kernel.org>
> cc: Paolo Abeni <pabeni@redhat.com>
> cc: Jens Axboe <axboe@kernel.dk>
> cc: Jens Axboe <axboe@kernel.dk>
> cc: Matthew Wilcox <willy@infradead.org>
> cc: linux-nvme@lists.infradead.org
> cc: netdev@vger.kernel.org
> ---
>

Reviewed-by: Chaitanya Kulkarni <kch@nvidia.com>

-ck

Re: [PATCH net] nvme-tcp: Fix comma-related oops

[Keith Busch]

On Thu, Jun 29, 2023 at 10:47:53PM +0100, David Howells wrote:
> Fix a comma that should be a semicolon.  The comma is at the end of an
> if-body and thus makes the statement after (a bvec_set_page()) conditional
> too, resulting in an oops because we didn't fill out the bio_vec[]:
> 
>     BUG: kernel NULL pointer dereference, address: 0000000000000008
>     #PF: supervisor read access in kernel mode
>     #PF: error_code(0x0000) - not-present page
>     ...
>     Workqueue: nvme_tcp_wq nvme_tcp_io_work [nvme_tcp]
>     RIP: 0010:skb_splice_from_iter+0xf1/0x370
>     ...
>     Call Trace:
>      tcp_sendmsg_locked+0x3a6/0xdd0
>      tcp_sendmsg+0x31/0x50
>      inet_sendmsg+0x47/0x80
>      sock_sendmsg+0x99/0xb0
>      nvme_tcp_try_send_data+0x149/0x490 [nvme_tcp]
>      nvme_tcp_try_send+0x1b7/0x300 [nvme_tcp]
>      nvme_tcp_io_work+0x40/0xc0 [nvme_tcp]
>      process_one_work+0x21c/0x430
>      worker_thread+0x54/0x3e0
>      kthread+0xf8/0x130
> 
> Fixes: 7769887817c3 ("nvme-tcp: Use sendmsg(MSG_SPLICE_PAGES) rather then sendpage")

We don't have this breaking commit in the nvme tree just yet, so feel
free to take the fix through net if this can't wait for the next nvme
rebase (we're based on the block tree).

Re: [PATCH net] nvme-tcp: Fix comma-related oops

[Jakub Kicinski]

On Fri, 30 Jun 2023 09:20:40 -0600 Keith Busch wrote:
> We don't have this breaking commit in the nvme tree just yet, so feel
> free to take the fix through net if this can't wait for the next nvme
> rebase (we're based on the block tree).

Ack, will do, thanks! We'll most likely send it in a PR later today.

Re: [PATCH net] nvme-tcp: Fix comma-related oops

[patchwork-bot+netdevbpf]

Hello:

This patch was applied to netdev/net.git (main)
by David S. Miller <davem@davemloft.net>:

On Thu, 29 Jun 2023 22:47:53 +0100 you wrote:
> Fix a comma that should be a semicolon.  The comma is at the end of an
> if-body and thus makes the statement after (a bvec_set_page()) conditional
> too, resulting in an oops because we didn't fill out the bio_vec[]:
> 
>     BUG: kernel NULL pointer dereference, address: 0000000000000008
>     #PF: supervisor read access in kernel mode
>     #PF: error_code(0x0000) - not-present page
>     ...
>     Workqueue: nvme_tcp_wq nvme_tcp_io_work [nvme_tcp]
>     RIP: 0010:skb_splice_from_iter+0xf1/0x370
>     ...
>     Call Trace:
>      tcp_sendmsg_locked+0x3a6/0xdd0
>      tcp_sendmsg+0x31/0x50
>      inet_sendmsg+0x47/0x80
>      sock_sendmsg+0x99/0xb0
>      nvme_tcp_try_send_data+0x149/0x490 [nvme_tcp]
>      nvme_tcp_try_send+0x1b7/0x300 [nvme_tcp]
>      nvme_tcp_io_work+0x40/0xc0 [nvme_tcp]
>      process_one_work+0x21c/0x430
>      worker_thread+0x54/0x3e0
>      kthread+0xf8/0x130
> 
> [...]

Here is the summary with links:
  - [net] nvme-tcp: Fix comma-related oops
    https://git.kernel.org/netdev/net/c/c97d3fb9e0e7

You are awesome, thank you!
-- 
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html