From 9679984ab028c5166c952b3cf8cfeea51a91295d Mon Sep 17 00:00:00 2001 From: Hannes Reinecke Date: Wed, 3 Sep 2025 10:45:23 +0200 Subject: [PATCH 1/4] nvme/tree: separate out __nvme_deconfigure_ctrl() Separate out __nvme_deconfigure_ctrl() to make clear which attributes are to be saved during nvme_reconfigure_ctrl(). Signed-off-by: Hannes Reinecke --- src/nvme/tree.c | 32 ++++++++++++-------------------- 1 file changed, 12 insertions(+), 20 deletions(-) diff --git a/src/nvme/tree.c b/src/nvme/tree.c index d79707439..6a59e433b 100644 --- a/src/nvme/tree.c +++ b/src/nvme/tree.c @@ -1311,7 +1311,7 @@ nvme_path_t nvme_ctrl_next_path(nvme_ctrl_t c, nvme_path_t p) #define FREE_CTRL_ATTR(a) \ do { free(a); (a) = NULL; } while (0) -void nvme_deconfigure_ctrl(nvme_ctrl_t c) +static void __nvme_deconfigure_ctrl(nvme_ctrl_t c) { nvme_ctrl_release_fd(c); FREE_CTRL_ATTR(c->name); @@ -1323,11 +1323,6 @@ void nvme_deconfigure_ctrl(nvme_ctrl_t c) FREE_CTRL_ATTR(c->queue_count); FREE_CTRL_ATTR(c->serial); FREE_CTRL_ATTR(c->sqsize); - FREE_CTRL_ATTR(c->dhchap_key); - FREE_CTRL_ATTR(c->dhchap_ctrl_key); - FREE_CTRL_ATTR(c->keyring); - FREE_CTRL_ATTR(c->tls_key_identity); - FREE_CTRL_ATTR(c->tls_key); FREE_CTRL_ATTR(c->address); FREE_CTRL_ATTR(c->dctype); FREE_CTRL_ATTR(c->cntrltype); @@ -1335,6 +1330,16 @@ void nvme_deconfigure_ctrl(nvme_ctrl_t c) FREE_CTRL_ATTR(c->phy_slot); } +void nvme_deconfigure_ctrl(nvme_ctrl_t c) +{ + __nvme_deconfigure_ctrl(c); + FREE_CTRL_ATTR(c->dhchap_key); + FREE_CTRL_ATTR(c->dhchap_ctrl_key); + FREE_CTRL_ATTR(c->keyring); + FREE_CTRL_ATTR(c->tls_key_identity); + FREE_CTRL_ATTR(c->tls_key); +} + int nvme_disconnect_ctrl(nvme_ctrl_t c) { nvme_root_t r = c->s && c->s->h ? c->s->h->r : NULL; @@ -2049,20 +2054,7 @@ static int nvme_reconfigure_ctrl(nvme_root_t r, nvme_ctrl_t c, const char *path, * It's necesssary to release any resources first because a ctrl * can be reused. */ - nvme_ctrl_release_fd(c); - FREE_CTRL_ATTR(c->name); - FREE_CTRL_ATTR(c->sysfs_dir); - FREE_CTRL_ATTR(c->firmware); - FREE_CTRL_ATTR(c->model); - FREE_CTRL_ATTR(c->state); - FREE_CTRL_ATTR(c->numa_node); - FREE_CTRL_ATTR(c->queue_count); - FREE_CTRL_ATTR(c->serial); - FREE_CTRL_ATTR(c->sqsize); - FREE_CTRL_ATTR(c->cntrltype); - FREE_CTRL_ATTR(c->cntlid); - FREE_CTRL_ATTR(c->dctype); - FREE_CTRL_ATTR(c->phy_slot); + __nvme_deconfigure_ctrl(c); d = opendir(path); if (!d) { From bfddbe8b750d90cc15253faa69c3f49516c4a220 Mon Sep 17 00:00:00 2001 From: Hannes Reinecke Date: Wed, 3 Sep 2025 11:05:50 +0200 Subject: [PATCH 2/4] nvme/tree: always set 'address' before nvme_reconfigure_ctrl() The 'address' attribute is required in nvme_reconfigure_ctrl(), so we need to update it before calling the function and must not modify it in nvme_reconfigure_ctrl(). Signed-off-by: Hannes Reinecke --- src/nvme/tree.c | 13 ++++++------- 1 file changed, 6 insertions(+), 7 deletions(-) diff --git a/src/nvme/tree.c b/src/nvme/tree.c index 6a59e433b..21f91e687 100644 --- a/src/nvme/tree.c +++ b/src/nvme/tree.c @@ -1323,7 +1323,6 @@ static void __nvme_deconfigure_ctrl(nvme_ctrl_t c) FREE_CTRL_ATTR(c->queue_count); FREE_CTRL_ATTR(c->serial); FREE_CTRL_ATTR(c->sqsize); - FREE_CTRL_ATTR(c->address); FREE_CTRL_ATTR(c->dctype); FREE_CTRL_ATTR(c->cntrltype); FREE_CTRL_ATTR(c->cntlid); @@ -1333,6 +1332,7 @@ static void __nvme_deconfigure_ctrl(nvme_ctrl_t c) void nvme_deconfigure_ctrl(nvme_ctrl_t c) { __nvme_deconfigure_ctrl(c); + FREE_CTRL_ATTR(c->address); FREE_CTRL_ATTR(c->dhchap_key); FREE_CTRL_ATTR(c->dhchap_ctrl_key); FREE_CTRL_ATTR(c->keyring); @@ -2102,16 +2102,17 @@ int nvme_init_ctrl(nvme_host_t h, nvme_ctrl_t c, int instance) return ret; } - ret = nvme_reconfigure_ctrl(h->r, c, path, name); - if (ret < 0) - return ret; - + FREE_CTRL_ATTR(c->address); c->address = nvme_get_attr(path, "address"); if (!c->address && strcmp(c->transport, "loop")) { errno = ENVME_CONNECT_INVAL_TR; return -1; } + ret = nvme_reconfigure_ctrl(h->r, c, path, name); + if (ret < 0) + return ret; + subsys_name = nvme_ctrl_lookup_subsystem_name(h->r, name); if (!subsys_name) { nvme_msg(h->r, LOG_ERR, @@ -2221,8 +2222,6 @@ static nvme_ctrl_t nvme_ctrl_alloc(nvme_root_t r, nvme_subsystem_t s, errno = ENODEV; return NULL; } - FREE_CTRL_ATTR(c->address); - c->address = xstrdup(addr); if (s->subsystype && !strcmp(s->subsystype, "discovery")) c->discovery_ctrl = true; ret = nvme_reconfigure_ctrl(r, c, path, name); From d806d2e05c5249e693e76008f63abce25ba8ad96 Mon Sep 17 00:00:00 2001 From: Hannes Reinecke Date: Wed, 3 Sep 2025 11:12:09 +0200 Subject: [PATCH 3/4] nvme/private.h: document controller attribute scopes The controller attributes have a different scope across reconfiguration; some are immutable, some persist across reconfiguration, and some will be reset when calling nvme_reconfigure_ctrl(). Signed-off-by: Hannes Reinecke --- src/nvme/private.h | 25 +++++++++++++++---------- 1 file changed, 15 insertions(+), 10 deletions(-) diff --git a/src/nvme/private.h b/src/nvme/private.h index f2ba299b6..8ec39b171 100644 --- a/src/nvme/private.h +++ b/src/nvme/private.h @@ -75,7 +75,22 @@ struct nvme_ctrl { struct list_head paths; struct list_head namespaces; struct nvme_subsystem *s; + struct nvme_fabrics_config cfg; + + /* Immutable attributes */ + char *transport; + char *subsysnqn; + char *traddr; + char *trsvcid; + + /* persistent across reconfiguration */ + char *dhchap_key; + char *dhchap_ctrl_key; + char *keyring; + char *tls_key_identity; + char *tls_key; + /* reset during reconfiguration */ int fd; char *name; char *sysfs_dir; @@ -87,15 +102,6 @@ struct nvme_ctrl { char *queue_count; char *serial; char *sqsize; - char *transport; - char *subsysnqn; - char *traddr; - char *trsvcid; - char *dhchap_key; - char *dhchap_ctrl_key; - char *keyring; - char *tls_key_identity; - char *tls_key; char *cntrltype; char *cntlid; char *dctype; @@ -104,7 +110,6 @@ struct nvme_ctrl { bool unique_discovery_ctrl; bool discovered; bool persistent; - struct nvme_fabrics_config cfg; }; struct nvme_subsystem { From 069504c6deda8c1030b30bc572bd408d4416f303 Mon Sep 17 00:00:00 2001 From: Hannes Reinecke Date: Wed, 3 Sep 2025 11:17:41 +0200 Subject: [PATCH 4/4] nvme/{tree,fabrics}: do not crash when accessing disconnected controllers When a controller is disconnected it will just clear the volatile attributes, but remain part of the tree structure. So when traversing the tree one could access a disconnected controller, but then c->name is NULL. So fixup all places calling nvme_ctrl_get_name() to ensure that they don't crash on an empty name. Signed-off-by: Hannes Reinecke --- src/nvme/fabrics.c | 7 +++++++ src/nvme/tree.c | 5 +++++ 2 files changed, 12 insertions(+) diff --git a/src/nvme/fabrics.c b/src/nvme/fabrics.c index 6aa62eea5..6af757d01 100644 --- a/src/nvme/fabrics.c +++ b/src/nvme/fabrics.c @@ -1149,6 +1149,13 @@ static struct nvmf_discovery_log *nvme_discovery_log( .uuidx = NVME_UUID_NONE, }; + if (!name) { + nvme_msg(r, LOG_ERR, + "controller not connected\n"); + errno = ENOTCONN; + return NULL; + } + log = __nvme_alloc(sizeof(*log)); if (!log) { nvme_msg(r, LOG_ERR, diff --git a/src/nvme/tree.c b/src/nvme/tree.c index 21f91e687..98915b819 100644 --- a/src/nvme/tree.c +++ b/src/nvme/tree.c @@ -245,6 +245,9 @@ static void nvme_filter_ctrl(nvme_root_t r, nvme_ctrl_t c, if (f(NULL, c, NULL, f_args)) return; + if (!nvme_ctrl_get_name(c)) + return; + nvme_msg(r, LOG_DEBUG, "filter out controller %s\n", nvme_ctrl_get_name(c)); nvme_free_ctrl(c); @@ -1815,6 +1818,8 @@ nvme_ctrl_t __nvme_lookup_ctrl(nvme_subsystem_t s, const char *transport, c = p ? nvme_subsystem_next_ctrl(s, p) : nvme_subsystem_first_ctrl(s); for (; c != NULL; c = nvme_subsystem_next_ctrl(s, c)) { + if (!nvme_ctrl_get_name(c)) + continue; if (ctrl_match(c, &candidate)) { matching_c = c; break;