nvme_fcloop: disassocate local port structs
authorJames Smart <jsmart2021@gmail.com>
Thu, 30 Nov 2017 00:47:31 +0000 (16:47 -0800)
committerChristoph Hellwig <hch@lst.de>
Mon, 8 Jan 2018 10:01:54 +0000 (11:01 +0100)
The current fcloop driver gets its lport structure from the private
area co-allocated with the fc_localport. All is fine except the
teardown path, which wants to wait on the completion, which is marked
complete by the delete_localport callback performed after
unregister_localport.  The issue is, the nvme_fc transport frees the
localport structure immediately after delete_localport is called,
meaning the original routine is trying to wait on a complete that
was just freed.

Change such that a lport struct is allocated coincident with the
addition and registration of a localport. The private area of the
localport now contains just a backpointer to the real lport struct.
Now, the completion can be waited for, and after completing, the
new structure can be kfree'd.

Signed-off-by: James Smart <james.smart@broadcom.com>
Signed-off-by: Christoph Hellwig <hch@lst.de>
drivers/nvme/target/fcloop.c

index 3eb2a0733f46de5fb00c69e3e1744c6d5423b7d3..c0080f6ab2f5b209eb8aeae003df6659f9124f11 100644 (file)
@@ -204,6 +204,10 @@ struct fcloop_lport {
        struct completion unreg_done;
 };
 
+struct fcloop_lport_priv {
+       struct fcloop_lport *lport;
+};
+
 struct fcloop_rport {
        struct nvme_fc_remote_port *remoteport;
        struct nvmet_fc_target_port *targetport;
@@ -659,7 +663,8 @@ fcloop_nport_get(struct fcloop_nport *nport)
 static void
 fcloop_localport_delete(struct nvme_fc_local_port *localport)
 {
-       struct fcloop_lport *lport = localport->private;
+       struct fcloop_lport_priv *lport_priv = localport->private;
+       struct fcloop_lport *lport = lport_priv->lport;
 
        /* release any threads waiting for the unreg to complete */
        complete(&lport->unreg_done);
@@ -699,7 +704,7 @@ static struct nvme_fc_port_template fctemplate = {
        .max_dif_sgl_segments   = FCLOOP_SGL_SEGS,
        .dma_boundary           = FCLOOP_DMABOUND_4G,
        /* sizes of additional private data for data structures */
-       .local_priv_sz          = sizeof(struct fcloop_lport),
+       .local_priv_sz          = sizeof(struct fcloop_lport_priv),
        .remote_priv_sz         = sizeof(struct fcloop_rport),
        .lsrqst_priv_sz         = sizeof(struct fcloop_lsreq),
        .fcprqst_priv_sz        = sizeof(struct fcloop_ini_fcpreq),
@@ -730,11 +735,17 @@ fcloop_create_local_port(struct device *dev, struct device_attribute *attr,
        struct fcloop_ctrl_options *opts;
        struct nvme_fc_local_port *localport;
        struct fcloop_lport *lport;
-       int ret;
+       struct fcloop_lport_priv *lport_priv;
+       unsigned long flags;
+       int ret = -ENOMEM;
+
+       lport = kzalloc(sizeof(*lport), GFP_KERNEL);
+       if (!lport)
+               return -ENOMEM;
 
        opts = kzalloc(sizeof(*opts), GFP_KERNEL);
        if (!opts)
-               return -ENOMEM;
+               goto out_free_lport;
 
        ret = fcloop_parse_options(opts, buf);
        if (ret)
@@ -754,23 +765,25 @@ fcloop_create_local_port(struct device *dev, struct device_attribute *attr,
 
        ret = nvme_fc_register_localport(&pinfo, &fctemplate, NULL, &localport);
        if (!ret) {
-               unsigned long flags;
-
                /* success */
-               lport = localport->private;
+               lport_priv = localport->private;
+               lport_priv->lport = lport;
+
                lport->localport = localport;
                INIT_LIST_HEAD(&lport->lport_list);
 
                spin_lock_irqsave(&fcloop_lock, flags);
                list_add_tail(&lport->lport_list, &fcloop_lports);
                spin_unlock_irqrestore(&fcloop_lock, flags);
-
-               /* mark all of the input buffer consumed */
-               ret = count;
        }
 
 out_free_opts:
        kfree(opts);
+out_free_lport:
+       /* free only if we're going to fail */
+       if (ret)
+               kfree(lport);
+
        return ret ? ret : count;
 }
 
@@ -792,6 +805,8 @@ __wait_localport_unreg(struct fcloop_lport *lport)
 
        wait_for_completion(&lport->unreg_done);
 
+       kfree(lport);
+
        return ret;
 }