[lustre-devel] [PATCH 15/24] lustre: llite: Refactor DIO/AIO free code
James Simmons
jsimmons at infradead.org
Mon Sep 5 18:55:28 PDT 2022
From: Patrick Farrell <pfarrell at whamcloud.com>
Refactor the DIO/AIO free code and add some asserts.
This removes a potential use-after-free in the freeing
code.
WC-bug-id: https://jira.whamcloud.com/browse/LU-15811
Lustre-commit: f1c8ac1156ebea2b8 ("LU-15811 llite: Refactor DIO/AIO free code")
Signed-off-by: Patrick Farrell <pfarrell at whamcloud.com>
Reviewed-on: https://review.whamcloud.com/48115
Reviewed-by: Andreas Dilger <adilger at whamcloud.com>
Reviewed-by: Yingjin Qian <qian at ddn.com>
Reviewed-by: Oleg Drokin <green at whamcloud.com>
Signed-off-by: James Simmons <jsimmons at infradead.org>
---
fs/lustre/include/cl_object.h | 11 +++++-----
fs/lustre/llite/file.c | 3 ++-
fs/lustre/llite/rw26.c | 9 ++++++---
fs/lustre/obdclass/cl_io.c | 47 ++++++++++++++++++++++++++-----------------
4 files changed, 41 insertions(+), 29 deletions(-)
diff --git a/fs/lustre/include/cl_object.h b/fs/lustre/include/cl_object.h
index 0f28cfe..3253f1c 100644
--- a/fs/lustre/include/cl_object.h
+++ b/fs/lustre/include/cl_object.h
@@ -2547,10 +2547,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)
{
@@ -2598,7 +2597,7 @@ struct cl_dio_aio {
struct kiocb *cda_iocb;
ssize_t cda_bytes;
unsigned int 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
@@ -2610,7 +2609,7 @@ struct cl_sub_dio {
ssize_t csd_bytes;
struct cl_dio_aio *csd_ll_aio;
struct ll_dio_pages csd_dio_pages;
- unsigned int csd_no_free:1;
+ unsigned int csd_creator_free:1;
};
void ll_release_user_pages(struct page **pages, int npages);
diff --git a/fs/lustre/llite/file.c b/fs/lustre/llite/file.c
index 8152821..115ee69 100644
--- a/fs/lustre/llite/file.c
+++ b/fs/lustre/llite/file.c
@@ -1856,7 +1856,8 @@ static void ll_heat_add(struct inode *inode, enum cl_io_type iot,
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/fs/lustre/llite/rw26.c b/fs/lustre/llite/rw26.c
index 0f9ab68..4f2e68e 100644
--- a/fs/lustre/llite/rw26.c
+++ b/fs/lustre/llite/rw26.c
@@ -391,8 +391,10 @@ static ssize_t ll_direct_IO(struct kiocb *iocb, struct iov_iter *iter)
&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;
}
@@ -412,7 +414,8 @@ static ssize_t ll_direct_IO(struct kiocb *iocb, struct iov_iter *iter)
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;
diff --git a/fs/lustre/obdclass/cl_io.c b/fs/lustre/obdclass/cl_io.c
index 06b9eb8..ee82260 100644
--- a/fs/lustre/obdclass/cl_io.c
+++ b/fs/lustre/obdclass/cl_io.c
@@ -1165,9 +1165,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;
@@ -1176,7 +1176,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;
@@ -1192,25 +1192,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);
kmem_cache_free(cl_dio_aio_kmem, aio);
}
}
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)
kmem_cache_free(cl_sub_dio_kmem, sdio);
}
EXPORT_SYMBOL(cl_sub_dio_free);
@@ -1247,7 +1246,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;
@@ -1260,18 +1262,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);
+ }
}
}
}
--
1.8.3.1
More information about the lustre-devel
mailing list