LU-15811 llite: Refactor DIO/AIO free code

Refactor the DIO/AIO free code and add some asserts.

This removes a potential use-after-free in the freeing
code.

Signed-off-by: Patrick Farrell <pfarrell@whamcloud.com>
Change-Id: I335b18fc7a28fc426a25675e2449d3d192cba596
Reviewed-on: https://review.whamcloud.com/48115
Tested-by: jenkins <devops@whamcloud.com>
Tested-by: Maloo <maloo@whamcloud.com>
Reviewed-by: Andreas Dilger <adilger@whamcloud.com>
Reviewed-by: Yingjin Qian <qian@ddn.com>
Reviewed-by: Oleg Drokin <green@whamcloud.com>

diff --git a/lustre/include/cl_object.h b/lustre/include/cl_object.h
index 1a76b18..2ef9c84 100644 (file)
--- a/lustre/include/cl_object.h
+++ b/lustre/include/cl_object.h
@@ -2531,10 +2531,9 @@ int cl_sync_io_wait_recycle(const struct lu_env *env, struct cl_sync_io *anchor,
                            long timeout, int ioret);
 struct cl_dio_aio *cl_dio_aio_alloc(struct kiocb *iocb, struct cl_object *obj,
                                    bool is_aio);
-struct cl_sub_dio *cl_sub_dio_alloc(struct cl_dio_aio *ll_aio, bool nofree);
-void cl_dio_aio_free(const struct lu_env *env, struct cl_dio_aio *aio,
-                    bool always_free);
-void cl_sub_dio_free(struct cl_sub_dio *sdio, bool nofree);
+struct cl_sub_dio *cl_sub_dio_alloc(struct cl_dio_aio *ll_aio, bool sync);
+void cl_dio_aio_free(const struct lu_env *env, struct cl_dio_aio *aio);
+void cl_sub_dio_free(struct cl_sub_dio *sdio);
 static inline void cl_sync_io_init(struct cl_sync_io *anchor, int nr)
 {
        cl_sync_io_init_notify(anchor, nr, NULL, NULL);
@@ -2579,7 +2578,7 @@ struct cl_dio_aio {
        struct kiocb            *cda_iocb;
        ssize_t                 cda_bytes;
        unsigned                cda_no_aio_complete:1,
-                               cda_no_sub_free:1;
+                               cda_creator_free:1;
 };
 
 /* Sub-dio used for splitting DIO (and AIO, because AIO is DIO) according to
@@ -2591,7 +2590,7 @@ struct cl_sub_dio {
        ssize_t                 csd_bytes;
        struct cl_dio_aio       *csd_ll_aio;
        struct ll_dio_pages     csd_dio_pages;
-       unsigned                csd_no_free:1;
+       unsigned                csd_creator_free:1;
 };
 #if defined(HAVE_DIRECTIO_ITER) || defined(HAVE_IOV_ITER_RW) || \
        defined(HAVE_DIRECTIO_2ARGS)

diff --git a/lustre/llite/file.c b/lustre/llite/file.c
index cd440c3..f451a38 100644 (file)
--- a/lustre/llite/file.c
+++ b/lustre/llite/file.c
@@ -1841,7 +1841,8 @@ out:
                cl_sync_io_note(env, &io->ci_dio_aio->cda_sync,
                                rc == -EIOCBQUEUED ? 0 : rc);
                if (!is_aio) {
-                       cl_dio_aio_free(env, io->ci_dio_aio, true);
+                       LASSERT(io->ci_dio_aio->cda_creator_free);
+                       cl_dio_aio_free(env, io->ci_dio_aio);
                        io->ci_dio_aio = NULL;
                }
        }

diff --git a/lustre/llite/rw26.c b/lustre/llite/rw26.c
index f558984..28d6a30 100644 (file)
--- a/lustre/llite/rw26.c
+++ b/lustre/llite/rw26.c
@@ -487,8 +487,10 @@ ll_direct_IO_impl(struct kiocb *iocb, struct iov_iter *iter, int rw)
                                           &pvec->ldp_count, count);
                if (unlikely(result <= 0)) {
                        cl_sync_io_note(env, &ldp_aio->csd_sync, result);
-                       if (sync_submit)
-                               cl_sub_dio_free(ldp_aio, true);
+                       if (sync_submit) {
+                               LASSERT(ldp_aio->csd_creator_free);
+                               cl_sub_dio_free(ldp_aio);
+                       }
                        GOTO(out, result);
                }
 
@@ -508,7 +510,8 @@ ll_direct_IO_impl(struct kiocb *iocb, struct iov_iter *iter, int rw)
                                             0);
                        if (result == 0 && rc2)
                                result = rc2;
-                       cl_sub_dio_free(ldp_aio, true);
+                       LASSERT(ldp_aio->csd_creator_free);
+                       cl_sub_dio_free(ldp_aio);
                }
                if (unlikely(result < 0))
                        GOTO(out, result);

diff --git a/lustre/obdclass/cl_io.c b/lustre/obdclass/cl_io.c
index 60aabf2..40e17de 100644 (file)
--- a/lustre/obdclass/cl_io.c
+++ b/lustre/obdclass/cl_io.c
@@ -1257,9 +1257,9 @@ struct cl_dio_aio *cl_dio_aio_alloc(struct kiocb *iocb, struct cl_object *obj,
                 * no one is waiting (in the kernel) for this to complete
                 *
                 * in other cases, the last user is cl_sync_io_wait, and in
-                * that case, the caller frees the struct after that call
+                * that case, the creator frees the struct after that call
                 */
-               aio->cda_no_sub_free = !is_aio;
+               aio->cda_creator_free = !is_aio;
 
                cl_object_get(obj);
                aio->cda_obj = obj;
@@ -1268,7 +1268,7 @@ struct cl_dio_aio *cl_dio_aio_alloc(struct kiocb *iocb, struct cl_object *obj,
 }
 EXPORT_SYMBOL(cl_dio_aio_alloc);
 
-struct cl_sub_dio *cl_sub_dio_alloc(struct cl_dio_aio *ll_aio, bool nofree)
+struct cl_sub_dio *cl_sub_dio_alloc(struct cl_dio_aio *ll_aio, bool sync)
 {
        struct cl_sub_dio *sdio;
 
@@ -1284,25 +1284,24 @@ struct cl_sub_dio *cl_sub_dio_alloc(struct cl_dio_aio *ll_aio, bool nofree)
 
                sdio->csd_ll_aio = ll_aio;
                atomic_add(1,  &ll_aio->cda_sync.csi_sync_nr);
-               sdio->csd_no_free = nofree;
+               sdio->csd_creator_free = sync;
        }
        return sdio;
 }
 EXPORT_SYMBOL(cl_sub_dio_alloc);
 
-void cl_dio_aio_free(const struct lu_env *env, struct cl_dio_aio *aio,
-                    bool always_free)
+void cl_dio_aio_free(const struct lu_env *env, struct cl_dio_aio *aio)
 {
-       if (aio && (!aio->cda_no_sub_free || always_free)) {
+       if (aio) {
                cl_object_put(env, aio->cda_obj);
                OBD_SLAB_FREE_PTR(aio, cl_dio_aio_kmem);
        }
 }
 EXPORT_SYMBOL(cl_dio_aio_free);
 
-void cl_sub_dio_free(struct cl_sub_dio *sdio, bool always_free)
+void cl_sub_dio_free(struct cl_sub_dio *sdio)
 {
-       if (sdio && (!sdio->csd_no_free || always_free))
+       if (sdio)
                OBD_SLAB_FREE_PTR(sdio, cl_sub_dio_kmem);
 }
 EXPORT_SYMBOL(cl_sub_dio_free);
@@ -1351,7 +1350,10 @@ void cl_sync_io_note(const struct lu_env *env, struct cl_sync_io *anchor,
        LASSERT(atomic_read(&anchor->csi_sync_nr) > 0);
        if (atomic_dec_and_lock(&anchor->csi_sync_nr,
                                &anchor->csi_waitq.lock)) {
-               void *dio_aio = NULL;
+               struct cl_sub_dio *sub_dio_aio = NULL;
+               struct cl_dio_aio *dio_aio = NULL;
+               void *csi_dio_aio = NULL;
+               bool creator_free = true;
 
                cl_sync_io_end_t *end_io = anchor->csi_end_io;
 
@@ -1367,18 +1369,25 @@ void cl_sync_io_note(const struct lu_env *env, struct cl_sync_io *anchor,
                if (end_io)
                        end_io(env, anchor);
 
-               dio_aio = anchor->csi_dio_aio;
+               csi_dio_aio = anchor->csi_dio_aio;
+               sub_dio_aio = csi_dio_aio;
+               dio_aio = csi_dio_aio;
+
+               if (csi_dio_aio && end_io == cl_dio_aio_end)
+                       creator_free = dio_aio->cda_creator_free;
+               else if (csi_dio_aio && end_io == cl_sub_dio_end)
+                       creator_free = sub_dio_aio->csd_creator_free;
 
                spin_unlock(&anchor->csi_waitq.lock);
 
-               if (dio_aio) {
-                       if (end_io == cl_dio_aio_end)
-                               cl_dio_aio_free(env,
-                                               (struct cl_dio_aio *) dio_aio,
-                                               false);
-                       else if (end_io == cl_sub_dio_end)
-                               cl_sub_dio_free((struct cl_sub_dio *) dio_aio,
-                                               false);
+               if (csi_dio_aio) {
+                       if (end_io == cl_dio_aio_end) {
+                               if (!creator_free)
+                                       cl_dio_aio_free(env, dio_aio);
+                       } else if (end_io == cl_sub_dio_end) {
+                               if (!creator_free)
+                                       cl_sub_dio_free(sub_dio_aio);
+                       }
                }
        }
        EXIT;