| Message ID | 59062.1688075273@warthog.procyon.org.uk (mailing list archive) |
|---|---|
| State | Accepted |
| Commit | c97d3fb9e0e716b77d5c0d3051d46e2f41dc6fe4 |
| Delegated to: | Netdev Maintainers |
| Headers | show |
| Series | [net] nvme-tcp: Fix comma-related oops | expand |
| Context | Check | Description |
|---|---|---|
| netdev/series_format | success | Single patches do not need cover letters |
| netdev/tree_selection | success | Clearly marked for net |
| netdev/fixes_present | success | Fixes tag present in non-next series |
| netdev/header_inline | success | No static functions without inline keyword in header files |
| netdev/build_32bit | success | Errors and warnings before: 8 this patch: 8 |
| netdev/cc_maintainers | success | CCed 8 of 8 maintainers |
| netdev/build_clang | fail | Errors and warnings before: 18 this patch: 18 |
| netdev/verify_signedoff | success | Signed-off-by tag matches author and committer |
| netdev/deprecated_api | success | None detected |
| netdev/check_selftest | success | No net selftest shell script |
| netdev/verify_fixes | success | Fixes tag looks correct |
| netdev/build_allmodconfig_warn | success | Errors and warnings before: 8 this patch: 8 |
| netdev/checkpatch | warning | WARNING: Duplicate signature |
| netdev/kdoc | success | Errors and warnings before: 0 this patch: 0 |
| netdev/source_inline | success | Was 0 now: 0 |
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
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).
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.
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!
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);
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(-)