Linux-NVME Archive mirror
 help / color / mirror / Atom feed
* [PATCHv2] nvme: authentication error are always non-retryable
@ 2024-02-26  8:06 Hannes Reinecke
  2024-02-26 10:24 ` Daniel Wagner
  0 siblings, 1 reply; 6+ messages in thread
From: Hannes Reinecke @ 2024-02-26  8:06 UTC (permalink / raw
  To: Christoph Hellwig
  Cc: Sagi Grimberg, Keith Busch, linux-nvme, Hannes Reinecke,
	Hannes Reinecke

Any authentication errors which are generated internally are always
non-retryable, so set the DNR bit to ensure they are not retried.

Signed-off-by: Hannes Reinecke <hare@suse.de>
---
 drivers/nvme/host/core.c    |  6 +++---
 drivers/nvme/host/fabrics.c | 22 ++++++++++++----------
 drivers/nvme/host/nvme.h    |  2 +-
 3 files changed, 16 insertions(+), 14 deletions(-)

diff --git a/drivers/nvme/host/core.c b/drivers/nvme/host/core.c
index 5e9a0cf43636..0a8a595070c0 100644
--- a/drivers/nvme/host/core.c
+++ b/drivers/nvme/host/core.c
@@ -374,14 +374,14 @@ static inline enum nvme_disposition nvme_decide_disposition(struct request *req)
 	if (likely(nvme_req(req)->status == 0))
 		return COMPLETE;
 
-	if ((nvme_req(req)->status & 0x7ff) == NVME_SC_AUTH_REQUIRED)
-		return AUTHENTICATE;
-
 	if (blk_noretry_request(req) ||
 	    (nvme_req(req)->status & NVME_SC_DNR) ||
 	    nvme_req(req)->retries >= nvme_max_retries)
 		return COMPLETE;
 
+	if ((nvme_req(req)->status & 0x7ff) == NVME_SC_AUTH_REQUIRED)
+		return AUTHENTICATE;
+
 	if (req->cmd_flags & REQ_NVME_MPATH) {
 		if (nvme_is_path_error(nvme_req(req)->status) ||
 		    blk_queue_dying(req->q))
diff --git a/drivers/nvme/host/fabrics.c b/drivers/nvme/host/fabrics.c
index ab5ac219b70a..a7d0d2841b38 100644
--- a/drivers/nvme/host/fabrics.c
+++ b/drivers/nvme/host/fabrics.c
@@ -467,7 +467,7 @@ int nvmf_connect_admin_queue(struct nvme_ctrl *ctrl)
 		if (result & NVME_CONNECT_AUTHREQ_ASCR) {
 			dev_warn(ctrl->device,
 				 "qid 0: secure concatenation is not supported\n");
-			ret = NVME_SC_AUTH_REQUIRED;
+			ret = NVME_SC_AUTH_REQUIRED | NVME_SC_DNR;
 			goto out_free_data;
 		}
 		/* Authentication required */
@@ -475,13 +475,14 @@ int nvmf_connect_admin_queue(struct nvme_ctrl *ctrl)
 		if (ret) {
 			dev_warn(ctrl->device,
 				 "qid 0: authentication setup failed\n");
-			ret = NVME_SC_AUTH_REQUIRED;
+			ret = NVME_SC_AUTH_REQUIRED | NVME_SC_DNR;
 			goto out_free_data;
 		}
 		ret = nvme_auth_wait(ctrl, 0);
 		if (ret)
 			dev_warn(ctrl->device,
-				 "qid 0: authentication failed\n");
+				 "qid 0: authentication failed, error %d\n",
+				 ret);
 		else
 			dev_info(ctrl->device,
 				 "qid 0: authenticated\n");
@@ -541,7 +542,7 @@ int nvmf_connect_io_queue(struct nvme_ctrl *ctrl, u16 qid)
 		if (result & NVME_CONNECT_AUTHREQ_ASCR) {
 			dev_warn(ctrl->device,
 				 "qid 0: secure concatenation is not supported\n");
-			ret = NVME_SC_AUTH_REQUIRED;
+			ret = NVME_SC_AUTH_REQUIRED | NVME_SC_DNR;
 			goto out_free_data;
 		}
 		/* Authentication required */
@@ -549,13 +550,14 @@ int nvmf_connect_io_queue(struct nvme_ctrl *ctrl, u16 qid)
 		if (ret) {
 			dev_warn(ctrl->device,
 				 "qid %d: authentication setup failed\n", qid);
-			ret = NVME_SC_AUTH_REQUIRED;
-		} else {
-			ret = nvme_auth_wait(ctrl, qid);
-			if (ret)
-				dev_warn(ctrl->device,
-					 "qid %u: authentication failed\n", qid);
+			ret = NVME_SC_AUTH_REQUIRED | NVME_SC_DNR;
+			goto out_free_data;
 		}
+		ret = nvme_auth_wait(ctrl, qid);
+		if (ret)
+			dev_warn(ctrl->device,
+				 "qid %u: authentication failed, error %d\n",
+				 qid, ret);
 	}
 out_free_data:
 	kfree(data);
diff --git a/drivers/nvme/host/nvme.h b/drivers/nvme/host/nvme.h
index 6ed2ca6b35e4..c08c220a68cf 100644
--- a/drivers/nvme/host/nvme.h
+++ b/drivers/nvme/host/nvme.h
@@ -1123,7 +1123,7 @@ static inline int nvme_auth_negotiate(struct nvme_ctrl *ctrl, int qid)
 }
 static inline int nvme_auth_wait(struct nvme_ctrl *ctrl, int qid)
 {
-	return NVME_SC_AUTH_REQUIRED;
+	return NVME_SC_AUTH_REQUIRED | NVME_SC_DNR;
 }
 static inline void nvme_auth_free(struct nvme_ctrl *ctrl) {};
 #endif
-- 
2.35.3



^ permalink raw reply related	[flat|nested] 6+ messages in thread

* Re: [PATCHv2] nvme: authentication error are always non-retryable
  2024-02-26  8:06 [PATCHv2] nvme: authentication error are always non-retryable Hannes Reinecke
@ 2024-02-26 10:24 ` Daniel Wagner
  2024-02-26 10:36   ` Daniel Wagner
  0 siblings, 1 reply; 6+ messages in thread
From: Daniel Wagner @ 2024-02-26 10:24 UTC (permalink / raw
  To: Hannes Reinecke
  Cc: Christoph Hellwig, Sagi Grimberg, Keith Busch, linux-nvme,
	Hannes Reinecke

On Mon, Feb 26, 2024 at 09:06:28AM +0100, Hannes Reinecke wrote:
> Any authentication errors which are generated internally are always
> non-retryable, so set the DNR bit to ensure they are not retried.
> 
> Signed-off-by: Hannes Reinecke <hare@suse.de>

Tested-by: Daniel Wagner <dwagner@suse.de>
Reviewed-by: Daniel Wagner <dwagner@suse.de>

> @@ -541,7 +542,7 @@ int nvmf_connect_io_queue(struct nvme_ctrl *ctrl, u16 qid)
>  		if (result & NVME_CONNECT_AUTHREQ_ASCR) {
>  			dev_warn(ctrl->device,
>  				 "qid 0: secure concatenation is not supported\n");

Maybe also updating the warn message to "qid %u: ..." wouldn't hurt.

> -			ret = NVME_SC_AUTH_REQUIRED;
> +			ret = NVME_SC_AUTH_REQUIRED | NVME_SC_DNR;
>  			goto out_free_data;


^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCHv2] nvme: authentication error are always non-retryable
  2024-02-26 10:24 ` Daniel Wagner
@ 2024-02-26 10:36   ` Daniel Wagner
  2024-02-26 10:54     ` Hannes Reinecke
  0 siblings, 1 reply; 6+ messages in thread
From: Daniel Wagner @ 2024-02-26 10:36 UTC (permalink / raw
  To: Hannes Reinecke
  Cc: Christoph Hellwig, Sagi Grimberg, Keith Busch, linux-nvme,
	Hannes Reinecke

On Mon, Feb 26, 2024 at 11:24:35AM +0100, Daniel Wagner wrote:
> On Mon, Feb 26, 2024 at 09:06:28AM +0100, Hannes Reinecke wrote:
> > Any authentication errors which are generated internally are always
> > non-retryable, so set the DNR bit to ensure they are not retried.
> > 
> > Signed-off-by: Hannes Reinecke <hare@suse.de>
> 
> Tested-by: Daniel Wagner <dwagner@suse.de>

Well, I called it success too early.

Now other tests are starting to fail, e.g. nvme/044 for loop and fc is
looping now:

loop:
[   67.481520] nvmet: ctrl 1 fatal error occurred!
[   67.484296] nvme nvme0: qid 0: authentication failed, error -111
[   67.531033] [1538] nvmet: ctrl 1 stop keep-alive
[   67.532903] ==================================================================
[   67.534283] BUG: KASAN: double-free in nvme_loop_create_ctrl+0x87c/0xbe0 [nvme_loop]
[   67.535591] Free of addr ffff888101b1c000 by task nvme/1538

[   67.536454] CPU: 0 PID: 1538 Comm: nvme Tainted: G        W          6.8.0-rc3+ #39 3d0b6128d1ea3c6026a2c1de70ba6c7dc10623c3
[   67.538326] Hardware name: QEMU Standard PC (Q35 + ICH9, 2009), BIOS unknown 2/2/2022
[   67.538326] Call Trace:
[   67.538326]  <TASK>
[   67.538326]  dump_stack_lvl+0x5b/0x80
[   67.538326]  print_report+0x163/0x800
[   67.538326]  ? __virt_addr_valid+0x2f3/0x340
[   67.538326]  ? nvme_loop_create_ctrl+0x87c/0xbe0 [nvme_loop e19ff72683e84a1bbd49637ed7792592f1a14d32]
[   67.538326]  kasan_report_invalid_free+0xa7/0xe0
[   67.538326]  ? nvme_loop_create_ctrl+0x87c/0xbe0 [nvme_loop e19ff72683e84a1bbd49637ed7792592f1a14d32]
[   67.538326]  ? nvme_loop_create_ctrl+0x87c/0xbe0 [nvme_loop e19ff72683e84a1bbd49637ed7792592f1a14d32]
[   67.538326]  poison_slab_object+0x11c/0x180
[   67.538326]  __kasan_slab_free+0x33/0x80
[   67.538326]  ? nvme_loop_create_ctrl+0x87c/0xbe0 [nvme_loop e19ff72683e84a1bbd49637ed7792592f1a14d32]
[   67.538326]  kfree+0x119/0x310
[   67.538326]  nvme_loop_create_ctrl+0x87c/0xbe0 [nvme_loop e19ff72683e84a1bbd49637ed7792592f1a14d32]
[   67.554302]  nvmf_dev_write+0x22ff/0x2ae0 [nvme_fabrics cd628fee4abd75b95095eaf559203a0a73425269]
[   67.554302]  ? common_file_perm+0x14e/0x210
[   67.554302]  vfs_write+0x1cd/0xb60
[   67.554302]  ? kasan_quarantine_put+0xb4/0x1c0
[   67.554302]  ? kmem_cache_free+0x11e/0x2e0
[   67.554302]  ksys_write+0xd7/0x1a0
[   67.554302]  do_syscall_64+0xb1/0x180
[   67.554302]  ? syscall_exit_to_user_mode+0x24d/0x350
[   67.554302]  ? do_syscall_64+0xc0/0x180
[   67.554302]  ? kmem_cache_free+0x11e/0x2e0
[   67.554302]  ? syscall_exit_to_user_mode+0x24d/0x350
[   67.554302]  ? do_syscall_64+0xc0/0x180
[   67.554302]  entry_SYSCALL_64_after_hwframe+0x6e/0x76
[   67.554302] RIP: 0033:0x7fc80410afd4
[   67.554302] Code: 84 00 00 00 00 00 90 90 90 90 90 90 90 90 90 90 90 90 90 90 f3 0f 1e fa 90 90 80 3d 55 ea 0e 00 00 74 13 b8 01 00 00
 00 0f 05 <48> 3d 00 f0 ff ff 77 54 c3 0f 1f 00 48 83 ec 28 48 89 54 24 18 48
[   67.554302] RSP: 002b:00007ffd31d002c8 EFLAGS: 00000202 ORIG_RAX: 0000000000000001
[   67.554302] RAX: ffffffffffffffda RBX: 0000000000000000 RCX: 00007fc80410afd4
[   67.574292] RDX: 0000000000000149 RSI: 0000000001bee310 RDI: 0000000000000003
[   67.574292] RBP: 0000000000000000 R08: 0000000000000149 R09: 0000000001bee310
[   67.574292] R10: 00007fc80400b2a8 R11: 0000000000000202 R12: 0000000000000000
[   67.574292] R13: 0000000000000000 R14: 00007fc8047cd000 R15: 00000000005659f8
[   67.579476]  </TASK>


fc:
[ 1215.072441] nvme nvme6: qid 0: authentication failed, error -111
[ 1215.074837] nvme nvme6: NVME-FC{0}: create_assoc failed, assoc_id 157f1b1ef3270000 ret -111
[ 1215.077495] nvme_fc: __nvme_fc_send_ls_req:1123      get 3 rport 0000000063a0d5d0
[ 1215.079555] nvme nvme6: NVME-FC{0}: reset: Reconnect attempt failed (-111)
[ 1215.081383] nvme_fc: nvme_fc_reconnect_or_delete:3356 ctrl 000000005d843865 status -111


^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCHv2] nvme: authentication error are always non-retryable
  2024-02-26 10:36   ` Daniel Wagner
@ 2024-02-26 10:54     ` Hannes Reinecke
  2024-02-26 14:01       ` Daniel Wagner
  0 siblings, 1 reply; 6+ messages in thread
From: Hannes Reinecke @ 2024-02-26 10:54 UTC (permalink / raw
  To: Daniel Wagner, Hannes Reinecke
  Cc: Christoph Hellwig, Sagi Grimberg, Keith Busch, linux-nvme

On 2/26/24 11:36, Daniel Wagner wrote:
> On Mon, Feb 26, 2024 at 11:24:35AM +0100, Daniel Wagner wrote:
>> On Mon, Feb 26, 2024 at 09:06:28AM +0100, Hannes Reinecke wrote:
>>> Any authentication errors which are generated internally are always
>>> non-retryable, so set the DNR bit to ensure they are not retried.
>>>
>>> Signed-off-by: Hannes Reinecke <hare@suse.de>
>>
>> Tested-by: Daniel Wagner <dwagner@suse.de>
> 
> Well, I called it success too early.
> 
> Now other tests are starting to fail, e.g. nvme/044 for loop and fc is
> looping now:
> 
> loop:
> [   67.481520] nvmet: ctrl 1 fatal error occurred!
> [   67.484296] nvme nvme0: qid 0: authentication failed, error -111
> [   67.531033] [1538] nvmet: ctrl 1 stop keep-alive
> [   67.532903] ==================================================================
> [   67.534283] BUG: KASAN: double-free in nvme_loop_create_ctrl+0x87c/0xbe0 [nvme_loop]
> [   67.535591] Free of addr ffff888101b1c000 by task nvme/1538
> 
> [   67.536454] CPU: 0 PID: 1538 Comm: nvme Tainted: G        W          6.8.0-rc3+ #39 3d0b6128d1ea3c6026a2c1de70ba6c7dc10623c3
> [   67.538326] Hardware name: QEMU Standard PC (Q35 + ICH9, 2009), BIOS unknown 2/2/2022
> [   67.538326] Call Trace:
> [   67.538326]  <TASK>
> [   67.538326]  dump_stack_lvl+0x5b/0x80
> [   67.538326]  print_report+0x163/0x800
> [   67.538326]  ? __virt_addr_valid+0x2f3/0x340
> [   67.538326]  ? nvme_loop_create_ctrl+0x87c/0xbe0 [nvme_loop e19ff72683e84a1bbd49637ed7792592f1a14d32]
> [   67.538326]  kasan_report_invalid_free+0xa7/0xe0
> [   67.538326]  ? nvme_loop_create_ctrl+0x87c/0xbe0 [nvme_loop e19ff72683e84a1bbd49637ed7792592f1a14d32]
> [   67.538326]  ? nvme_loop_create_ctrl+0x87c/0xbe0 [nvme_loop e19ff72683e84a1bbd49637ed7792592f1a14d32]
> [   67.538326]  poison_slab_object+0x11c/0x180
> [   67.538326]  __kasan_slab_free+0x33/0x80
> [   67.538326]  ? nvme_loop_create_ctrl+0x87c/0xbe0 [nvme_loop e19ff72683e84a1bbd49637ed7792592f1a14d32]
> [   67.538326]  kfree+0x119/0x310
> [   67.538326]  nvme_loop_create_ctrl+0x87c/0xbe0 [nvme_loop e19ff72683e84a1bbd49637ed7792592f1a14d32]
> [   67.554302]  nvmf_dev_write+0x22ff/0x2ae0 [nvme_fabrics cd628fee4abd75b95095eaf559203a0a73425269]
> [   67.554302]  ? common_file_perm+0x14e/0x210
> [   67.554302]  vfs_write+0x1cd/0xb60
> [   67.554302]  ? kasan_quarantine_put+0xb4/0x1c0
> [   67.554302]  ? kmem_cache_free+0x11e/0x2e0
> [   67.554302]  ksys_write+0xd7/0x1a0
> [   67.554302]  do_syscall_64+0xb1/0x180
> [   67.554302]  ? syscall_exit_to_user_mode+0x24d/0x350
> [   67.554302]  ? do_syscall_64+0xc0/0x180
> [   67.554302]  ? kmem_cache_free+0x11e/0x2e0
> [   67.554302]  ? syscall_exit_to_user_mode+0x24d/0x350
> [   67.554302]  ? do_syscall_64+0xc0/0x180
> [   67.554302]  entry_SYSCALL_64_after_hwframe+0x6e/0x76
> [   67.554302] RIP: 0033:0x7fc80410afd4
> [   67.554302] Code: 84 00 00 00 00 00 90 90 90 90 90 90 90 90 90 90 90 90 90 90 f3 0f 1e fa 90 90 80 3d 55 ea 0e 00 00 74 13 b8 01 00 00
>   00 0f 05 <48> 3d 00 f0 ff ff 77 54 c3 0f 1f 00 48 83 ec 28 48 89 54 24 18 48
> [   67.554302] RSP: 002b:00007ffd31d002c8 EFLAGS: 00000202 ORIG_RAX: 0000000000000001
> [   67.554302] RAX: ffffffffffffffda RBX: 0000000000000000 RCX: 00007fc80410afd4
> [   67.574292] RDX: 0000000000000149 RSI: 0000000001bee310 RDI: 0000000000000003
> [   67.574292] RBP: 0000000000000000 R08: 0000000000000149 R09: 0000000001bee310
> [   67.574292] R10: 00007fc80400b2a8 R11: 0000000000000202 R12: 0000000000000000
> [   67.574292] R13: 0000000000000000 R14: 00007fc8047cd000 R15: 00000000005659f8
> [   67.579476]  </TASK>
> 
> 
Maybe this?

diff --git a/drivers/nvme/host/core.c b/drivers/nvme/host/core.c
index 0a8a595070c0..ca1984620492 100644
--- a/drivers/nvme/host/core.c
+++ b/drivers/nvme/host/core.c
@@ -4413,6 +4413,8 @@ void nvme_remove_admin_tag_set(struct nvme_ctrl *ctrl)
                 blk_put_queue(ctrl->fabrics_q);
         }
         blk_mq_free_tag_set(ctrl->admin_tagset);
+       ctrl->admin_q = NULL;
+       ctrl->fabrics_q = NULL;
  }
  EXPORT_SYMBOL_GPL(nvme_remove_admin_tag_set);

Cheers,

Hannes



^ permalink raw reply related	[flat|nested] 6+ messages in thread

* Re: [PATCHv2] nvme: authentication error are always non-retryable
  2024-02-26 10:54     ` Hannes Reinecke
@ 2024-02-26 14:01       ` Daniel Wagner
  2024-03-07 10:37         ` Sagi Grimberg
  0 siblings, 1 reply; 6+ messages in thread
From: Daniel Wagner @ 2024-02-26 14:01 UTC (permalink / raw
  To: Hannes Reinecke
  Cc: Hannes Reinecke, Christoph Hellwig, Sagi Grimberg, Keith Busch,
	linux-nvme

On Mon, Feb 26, 2024 at 11:54:13AM +0100, Hannes Reinecke wrote:
> Maybe this?

nvme_uninit_ctrl already puts the ref, thus the nvme_put_ctrl in
nvme_loop_create_crtl is one too much:

diff --git a/drivers/nvme/target/loop.c b/drivers/nvme/target/loop.c
index de2ff7ed0657..bcda781e52f2 100644
--- a/drivers/nvme/target/loop.c
+++ b/drivers/nvme/target/loop.c
@@ -615,7 +615,6 @@ static struct nvme_ctrl *nvme_loop_create_ctrl(struct device *dev,
        kfree(ctrl->queues);
 out_uninit_ctrl:
        nvme_uninit_ctrl(&ctrl->ctrl);
-       nvme_put_ctrl(&ctrl->ctrl);
 out_free_opts:
        nvmf_ctrl_options_put(opts);
 out_free_ctrl:


This leaves us with -111 (-ECONNREFUSED)

[  274.702672] nvmet: ctrl 1 fatal error occurred!
[  274.704673] nvme nvme2: qid 0: authentication failed, error -111
[  274.706212] nvme nvme2: NVME-FC{0}: create_assoc failed, assoc_id 2334a5aaec310000 ret -111
[  274.711653] nvme nvme2: NVME-FC{0}: reset: Reconnect attempt failed (-111)
[  274.713119] nvme_fc: nvme_fc_reconnect_or_delete:3356 ctrl 000000009ff229b7 status -111
[  274.714719] nvme nvme2: NVME-FC{0}: Reconnect attempt in 2 seconds


^ permalink raw reply related	[flat|nested] 6+ messages in thread

* Re: [PATCHv2] nvme: authentication error are always non-retryable
  2024-02-26 14:01       ` Daniel Wagner
@ 2024-03-07 10:37         ` Sagi Grimberg
  0 siblings, 0 replies; 6+ messages in thread
From: Sagi Grimberg @ 2024-03-07 10:37 UTC (permalink / raw
  To: Daniel Wagner, Hannes Reinecke
  Cc: Hannes Reinecke, Christoph Hellwig, Keith Busch, linux-nvme



On 26/02/2024 16:01, Daniel Wagner wrote:
> On Mon, Feb 26, 2024 at 11:54:13AM +0100, Hannes Reinecke wrote:
>> Maybe this?
> nvme_uninit_ctrl already puts the ref, thus the nvme_put_ctrl in
> nvme_loop_create_crtl is one too much:
>
> diff --git a/drivers/nvme/target/loop.c b/drivers/nvme/target/loop.c
> index de2ff7ed0657..bcda781e52f2 100644
> --- a/drivers/nvme/target/loop.c
> +++ b/drivers/nvme/target/loop.c
> @@ -615,7 +615,6 @@ static struct nvme_ctrl *nvme_loop_create_ctrl(struct device *dev,
>          kfree(ctrl->queues);
>   out_uninit_ctrl:
>          nvme_uninit_ctrl(&ctrl->ctrl);
> -       nvme_put_ctrl(&ctrl->ctrl);

Are you sure this does not cause a leak in some cases?


^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2024-03-07 10:37 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2024-02-26  8:06 [PATCHv2] nvme: authentication error are always non-retryable Hannes Reinecke
2024-02-26 10:24 ` Daniel Wagner
2024-02-26 10:36   ` Daniel Wagner
2024-02-26 10:54     ` Hannes Reinecke
2024-02-26 14:01       ` Daniel Wagner
2024-03-07 10:37         ` Sagi Grimberg

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 read-only IMAP folder(s) and NNTP newsgroup(s).