]> www.infradead.org Git - users/hch/nvme-cli.git/commitdiff
fabrics: fix infinite loop is discovery recursion contains a loop
authorSagi Grimberg <sagi@grimberg.me>
Fri, 14 Aug 2020 20:42:39 +0000 (13:42 -0700)
committerKeith Busch <kbusch@kernel.org>
Wed, 19 Aug 2020 16:54:35 +0000 (09:54 -0700)
It's possible that different discovery controllers may refer to
each other. In this case, we will get into an infinite loop as
we don't track that we have already connected and seen this.

The kernel doesn't protect us from this for discovery controllers
because it always allows duplicate discovery controllers.

Signed-off-by: Sagi Grimberg <sagi@grimberg.me>
Signed-off-by: Keith Busch <kbusch@kernel.org>
fabrics.c

index 6d6b3b5a3ca701bab645dde3d1b1eb76428ac93e..73bb3752944892db58d173a4bded79309a1c8b56 100644 (file)
--- a/fabrics.c
+++ b/fabrics.c
@@ -93,8 +93,12 @@ struct connect_args {
        char *traddr;
        char *trsvcid;
        char *host_traddr;
+       struct connect_args *next;
+       struct connect_args *tail;
 };
 
+struct connect_args *tracked_ctrls;
+
 #define BUF_SIZE               4096
 #define PATH_NVME_FABRICS      "/dev/nvme-fabrics"
 #define PATH_NVMF_DISC         "/etc/nvme/discovery.conf"
@@ -385,6 +389,21 @@ static void free_connect_args(struct connect_args *cargs)
        free(cargs);
 }
 
+static void track_ctrl(char *argstr)
+{
+       struct connect_args *cargs;
+
+       cargs = extract_connect_args(argstr);
+       if (!cargs)
+               return;
+
+       if (!tracked_ctrls)
+               tracked_ctrls = cargs;
+       else
+               tracked_ctrls->tail->next = cargs;
+       tracked_ctrls->tail = cargs;
+}
+
 static int add_ctrl(const char *argstr)
 {
        substring_t args[MAX_OPT_ARGS];
@@ -428,6 +447,7 @@ static int add_ctrl(const char *argstr)
                        if (match_int(args, &token))
                                goto out_fail;
                        ret = token;
+                       track_ctrl((char *)argstr);
                        goto out_close;
                default:
                        /* ignore */
@@ -1148,10 +1168,39 @@ retry:
        return ret;
 }
 
+static bool cargs_match_found(struct nvmf_disc_rsp_page_entry *entry)
+{
+       struct connect_args cargs = {};
+       struct connect_args *c = tracked_ctrls;
+
+       cargs.traddr = strdup(entry->traddr);
+       cargs.transport = strdup(trtype_str(entry->trtype));
+       cargs.subsysnqn = strdup(entry->subnqn);
+       cargs.trsvcid = strdup(entry->trsvcid);
+       cargs.host_traddr = strdup(cfg.host_traddr ?: "\0");
+
+       /* check if we have a match in the discovery recursion */
+       while (c) {
+               if (!strcmp(cargs.subsysnqn, c->subsysnqn) &&
+                   !strcmp(cargs.transport, c->transport) &&
+                   !strcmp(cargs.traddr, c->traddr) &&
+                   !strcmp(cargs.trsvcid, c->trsvcid) &&
+                   !strcmp(cargs.host_traddr, c->host_traddr))
+                       return true;
+               c = c->next;
+       }
+
+       /* check if we have a matching existing controller */
+       return find_ctrl_with_connectargs(&cargs) != NULL;
+}
+
 static bool should_connect(struct nvmf_disc_rsp_page_entry *entry)
 {
        int len;
 
+       if (cargs_match_found(entry))
+               return false;
+
        if (!cfg.matching_only || !cfg.traddr)
                return true;