All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH resend] jfs: fix use-after-free in lbmIODone
@ 2021-03-22 16:11 ` Anirudh Rayabharam
  0 siblings, 0 replies; 4+ messages in thread
From: Anirudh Rayabharam @ 2021-03-22 16:11 UTC (permalink / raw)
  To: shaggy
  Cc: jfs-discussion, linux-kernel, linux-kernel-mentees, gregkh,
	Anirudh Rayabharam, syzbot+5d2008bd1f1b722ba94e, Hillf Danton

Fix use-after-free by waiting for ongoing IO to complete before freeing
lbufs in lbmLogShutdown. Add a counter in struct jfs_log to keep track
of the number of in-flight IO operations and a wait queue to wait on for
the IO operations to complete.

Reported-by: syzbot+5d2008bd1f1b722ba94e@syzkaller.appspotmail.com
Suggested-by: Hillf Danton <hdanton@sina.com>
Signed-off-by: Anirudh Rayabharam <mail@anirudhrb.com>
---
 fs/jfs/jfs_logmgr.c | 17 ++++++++++++++---
 fs/jfs/jfs_logmgr.h |  2 ++
 2 files changed, 16 insertions(+), 3 deletions(-)

diff --git a/fs/jfs/jfs_logmgr.c b/fs/jfs/jfs_logmgr.c
index 9330eff210e0..82d20c4687aa 100644
--- a/fs/jfs/jfs_logmgr.c
+++ b/fs/jfs/jfs_logmgr.c
@@ -1815,6 +1815,8 @@ static int lbmLogInit(struct jfs_log * log)
 	 */
 	init_waitqueue_head(&log->free_wait);
 
+	init_waitqueue_head(&log->io_waitq);
+
 	log->lbuf_free = NULL;
 
 	for (i = 0; i < LOGPAGES;) {
@@ -1864,6 +1866,7 @@ static void lbmLogShutdown(struct jfs_log * log)
 	struct lbuf *lbuf;
 
 	jfs_info("lbmLogShutdown: log:0x%p", log);
+	wait_event(log->io_waitq, !atomic_read(&log->io_inflight));
 
 	lbuf = log->lbuf_free;
 	while (lbuf) {
@@ -1990,6 +1993,8 @@ static int lbmRead(struct jfs_log * log, int pn, struct lbuf ** bpp)
 	bio->bi_end_io = lbmIODone;
 	bio->bi_private = bp;
 	bio->bi_opf = REQ_OP_READ;
+
+	atomic_inc(&log->io_inflight);
 	/*check if journaling to disk has been disabled*/
 	if (log->no_integrity) {
 		bio->bi_iter.bi_size = 0;
@@ -2135,6 +2140,7 @@ static void lbmStartIO(struct lbuf * bp)
 	bio->bi_private = bp;
 	bio->bi_opf = REQ_OP_WRITE | REQ_SYNC;
 
+	atomic_inc(&log->io_inflight);
 	/* check if journaling to disk has been disabled */
 	if (log->no_integrity) {
 		bio->bi_iter.bi_size = 0;
@@ -2200,6 +2206,8 @@ static void lbmIODone(struct bio *bio)
 
 	bio_put(bio);
 
+	log = bp->l_log;
+
 	/*
 	 *	pagein completion
 	 */
@@ -2211,7 +2219,7 @@ static void lbmIODone(struct bio *bio)
 		/* wakeup I/O initiator */
 		LCACHE_WAKEUP(&bp->l_ioevent);
 
-		return;
+		goto out;
 	}
 
 	/*
@@ -2230,13 +2238,12 @@ static void lbmIODone(struct bio *bio)
 	INCREMENT(lmStat.pagedone);
 
 	/* update committed lsn */
-	log = bp->l_log;
 	log->clsn = (bp->l_pn << L2LOGPSIZE) + bp->l_ceor;
 
 	if (bp->l_flag & lbmDIRECT) {
 		LCACHE_WAKEUP(&bp->l_ioevent);
 		LCACHE_UNLOCK(flags);
-		return;
+		goto out;
 	}
 
 	tail = log->wqueue;
@@ -2315,6 +2322,10 @@ static void lbmIODone(struct bio *bio)
 
 		LCACHE_UNLOCK(flags);	/* unlock+enable */
 	}
+
+out:
+	if (atomic_dec_and_test(&log->io_inflight))
+		wake_up(&log->io_waitq);
 }
 
 int jfsIOWait(void *arg)
diff --git a/fs/jfs/jfs_logmgr.h b/fs/jfs/jfs_logmgr.h
index 805877ce5020..3e92fe251f28 100644
--- a/fs/jfs/jfs_logmgr.h
+++ b/fs/jfs/jfs_logmgr.h
@@ -400,6 +400,8 @@ struct jfs_log {
 	uuid_t uuid;		/* 16: 128-bit uuid of log device */
 
 	int no_integrity;	/* 3: flag to disable journaling to disk */
+	atomic_t io_inflight;
+	wait_queue_head_t io_waitq;
 };
 
 /*
-- 
2.26.2



^ permalink raw reply related	[flat|nested] 4+ messages in thread

* [PATCH resend] jfs: fix use-after-free in lbmIODone
@ 2021-03-22 16:11 ` Anirudh Rayabharam
  0 siblings, 0 replies; 4+ messages in thread
From: Anirudh Rayabharam @ 2021-03-22 16:11 UTC (permalink / raw)
  To: shaggy
  Cc: jfs-discussion, syzbot+5d2008bd1f1b722ba94e, linux-kernel,
	Hillf Danton, linux-kernel-mentees

Fix use-after-free by waiting for ongoing IO to complete before freeing
lbufs in lbmLogShutdown. Add a counter in struct jfs_log to keep track
of the number of in-flight IO operations and a wait queue to wait on for
the IO operations to complete.

Reported-by: syzbot+5d2008bd1f1b722ba94e@syzkaller.appspotmail.com
Suggested-by: Hillf Danton <hdanton@sina.com>
Signed-off-by: Anirudh Rayabharam <mail@anirudhrb.com>
---
 fs/jfs/jfs_logmgr.c | 17 ++++++++++++++---
 fs/jfs/jfs_logmgr.h |  2 ++
 2 files changed, 16 insertions(+), 3 deletions(-)

diff --git a/fs/jfs/jfs_logmgr.c b/fs/jfs/jfs_logmgr.c
index 9330eff210e0..82d20c4687aa 100644
--- a/fs/jfs/jfs_logmgr.c
+++ b/fs/jfs/jfs_logmgr.c
@@ -1815,6 +1815,8 @@ static int lbmLogInit(struct jfs_log * log)
 	 */
 	init_waitqueue_head(&log->free_wait);
 
+	init_waitqueue_head(&log->io_waitq);
+
 	log->lbuf_free = NULL;
 
 	for (i = 0; i < LOGPAGES;) {
@@ -1864,6 +1866,7 @@ static void lbmLogShutdown(struct jfs_log * log)
 	struct lbuf *lbuf;
 
 	jfs_info("lbmLogShutdown: log:0x%p", log);
+	wait_event(log->io_waitq, !atomic_read(&log->io_inflight));
 
 	lbuf = log->lbuf_free;
 	while (lbuf) {
@@ -1990,6 +1993,8 @@ static int lbmRead(struct jfs_log * log, int pn, struct lbuf ** bpp)
 	bio->bi_end_io = lbmIODone;
 	bio->bi_private = bp;
 	bio->bi_opf = REQ_OP_READ;
+
+	atomic_inc(&log->io_inflight);
 	/*check if journaling to disk has been disabled*/
 	if (log->no_integrity) {
 		bio->bi_iter.bi_size = 0;
@@ -2135,6 +2140,7 @@ static void lbmStartIO(struct lbuf * bp)
 	bio->bi_private = bp;
 	bio->bi_opf = REQ_OP_WRITE | REQ_SYNC;
 
+	atomic_inc(&log->io_inflight);
 	/* check if journaling to disk has been disabled */
 	if (log->no_integrity) {
 		bio->bi_iter.bi_size = 0;
@@ -2200,6 +2206,8 @@ static void lbmIODone(struct bio *bio)
 
 	bio_put(bio);
 
+	log = bp->l_log;
+
 	/*
 	 *	pagein completion
 	 */
@@ -2211,7 +2219,7 @@ static void lbmIODone(struct bio *bio)
 		/* wakeup I/O initiator */
 		LCACHE_WAKEUP(&bp->l_ioevent);
 
-		return;
+		goto out;
 	}
 
 	/*
@@ -2230,13 +2238,12 @@ static void lbmIODone(struct bio *bio)
 	INCREMENT(lmStat.pagedone);
 
 	/* update committed lsn */
-	log = bp->l_log;
 	log->clsn = (bp->l_pn << L2LOGPSIZE) + bp->l_ceor;
 
 	if (bp->l_flag & lbmDIRECT) {
 		LCACHE_WAKEUP(&bp->l_ioevent);
 		LCACHE_UNLOCK(flags);
-		return;
+		goto out;
 	}
 
 	tail = log->wqueue;
@@ -2315,6 +2322,10 @@ static void lbmIODone(struct bio *bio)
 
 		LCACHE_UNLOCK(flags);	/* unlock+enable */
 	}
+
+out:
+	if (atomic_dec_and_test(&log->io_inflight))
+		wake_up(&log->io_waitq);
 }
 
 int jfsIOWait(void *arg)
diff --git a/fs/jfs/jfs_logmgr.h b/fs/jfs/jfs_logmgr.h
index 805877ce5020..3e92fe251f28 100644
--- a/fs/jfs/jfs_logmgr.h
+++ b/fs/jfs/jfs_logmgr.h
@@ -400,6 +400,8 @@ struct jfs_log {
 	uuid_t uuid;		/* 16: 128-bit uuid of log device */
 
 	int no_integrity;	/* 3: flag to disable journaling to disk */
+	atomic_t io_inflight;
+	wait_queue_head_t io_waitq;
 };
 
 /*
-- 
2.26.2


_______________________________________________
Linux-kernel-mentees mailing list
Linux-kernel-mentees@lists.linuxfoundation.org
https://lists.linuxfoundation.org/mailman/listinfo/linux-kernel-mentees

^ permalink raw reply related	[flat|nested] 4+ messages in thread

* Re: [PATCH resend] jfs: fix use-after-free in lbmIODone
  2021-03-22 16:11 ` Anirudh Rayabharam
@ 2021-03-26 13:56   ` Greg KH
  -1 siblings, 0 replies; 4+ messages in thread
From: Greg KH @ 2021-03-26 13:56 UTC (permalink / raw)
  To: Anirudh Rayabharam
  Cc: shaggy, jfs-discussion, linux-kernel, linux-kernel-mentees,
	syzbot+5d2008bd1f1b722ba94e, Hillf Danton

On Mon, Mar 22, 2021 at 09:41:47PM +0530, Anirudh Rayabharam wrote:
> Fix use-after-free by waiting for ongoing IO to complete before freeing
> lbufs in lbmLogShutdown. Add a counter in struct jfs_log to keep track
> of the number of in-flight IO operations and a wait queue to wait on for
> the IO operations to complete.
> 
> Reported-by: syzbot+5d2008bd1f1b722ba94e@syzkaller.appspotmail.com
> Suggested-by: Hillf Danton <hdanton@sina.com>
> Signed-off-by: Anirudh Rayabharam <mail@anirudhrb.com>
> ---
>  fs/jfs/jfs_logmgr.c | 17 ++++++++++++++---
>  fs/jfs/jfs_logmgr.h |  2 ++
>  2 files changed, 16 insertions(+), 3 deletions(-)
> 
> diff --git a/fs/jfs/jfs_logmgr.c b/fs/jfs/jfs_logmgr.c
> index 9330eff210e0..82d20c4687aa 100644
> --- a/fs/jfs/jfs_logmgr.c
> +++ b/fs/jfs/jfs_logmgr.c
> @@ -1815,6 +1815,8 @@ static int lbmLogInit(struct jfs_log * log)
>  	 */
>  	init_waitqueue_head(&log->free_wait);
>  
> +	init_waitqueue_head(&log->io_waitq);
> +
>  	log->lbuf_free = NULL;
>  
>  	for (i = 0; i < LOGPAGES;) {
> @@ -1864,6 +1866,7 @@ static void lbmLogShutdown(struct jfs_log * log)
>  	struct lbuf *lbuf;
>  
>  	jfs_info("lbmLogShutdown: log:0x%p", log);
> +	wait_event(log->io_waitq, !atomic_read(&log->io_inflight));
>  
>  	lbuf = log->lbuf_free;
>  	while (lbuf) {
> @@ -1990,6 +1993,8 @@ static int lbmRead(struct jfs_log * log, int pn, struct lbuf ** bpp)
>  	bio->bi_end_io = lbmIODone;
>  	bio->bi_private = bp;
>  	bio->bi_opf = REQ_OP_READ;
> +
> +	atomic_inc(&log->io_inflight);
>  	/*check if journaling to disk has been disabled*/
>  	if (log->no_integrity) {
>  		bio->bi_iter.bi_size = 0;
> @@ -2135,6 +2140,7 @@ static void lbmStartIO(struct lbuf * bp)
>  	bio->bi_private = bp;
>  	bio->bi_opf = REQ_OP_WRITE | REQ_SYNC;
>  
> +	atomic_inc(&log->io_inflight);

Why use an atomic for this?  The value can change after you test for it,
as there's no lock involved.

Do you really need to keep track of all of these "inflight"?  That feels
very "heavy" to me.

jfs developers, any ideas?

thanks,

greg k-h

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH resend] jfs: fix use-after-free in lbmIODone
@ 2021-03-26 13:56   ` Greg KH
  0 siblings, 0 replies; 4+ messages in thread
From: Greg KH @ 2021-03-26 13:56 UTC (permalink / raw)
  To: Anirudh Rayabharam
  Cc: shaggy, jfs-discussion, syzbot+5d2008bd1f1b722ba94e,
	linux-kernel, Hillf Danton, linux-kernel-mentees

On Mon, Mar 22, 2021 at 09:41:47PM +0530, Anirudh Rayabharam wrote:
> Fix use-after-free by waiting for ongoing IO to complete before freeing
> lbufs in lbmLogShutdown. Add a counter in struct jfs_log to keep track
> of the number of in-flight IO operations and a wait queue to wait on for
> the IO operations to complete.
> 
> Reported-by: syzbot+5d2008bd1f1b722ba94e@syzkaller.appspotmail.com
> Suggested-by: Hillf Danton <hdanton@sina.com>
> Signed-off-by: Anirudh Rayabharam <mail@anirudhrb.com>
> ---
>  fs/jfs/jfs_logmgr.c | 17 ++++++++++++++---
>  fs/jfs/jfs_logmgr.h |  2 ++
>  2 files changed, 16 insertions(+), 3 deletions(-)
> 
> diff --git a/fs/jfs/jfs_logmgr.c b/fs/jfs/jfs_logmgr.c
> index 9330eff210e0..82d20c4687aa 100644
> --- a/fs/jfs/jfs_logmgr.c
> +++ b/fs/jfs/jfs_logmgr.c
> @@ -1815,6 +1815,8 @@ static int lbmLogInit(struct jfs_log * log)
>  	 */
>  	init_waitqueue_head(&log->free_wait);
>  
> +	init_waitqueue_head(&log->io_waitq);
> +
>  	log->lbuf_free = NULL;
>  
>  	for (i = 0; i < LOGPAGES;) {
> @@ -1864,6 +1866,7 @@ static void lbmLogShutdown(struct jfs_log * log)
>  	struct lbuf *lbuf;
>  
>  	jfs_info("lbmLogShutdown: log:0x%p", log);
> +	wait_event(log->io_waitq, !atomic_read(&log->io_inflight));
>  
>  	lbuf = log->lbuf_free;
>  	while (lbuf) {
> @@ -1990,6 +1993,8 @@ static int lbmRead(struct jfs_log * log, int pn, struct lbuf ** bpp)
>  	bio->bi_end_io = lbmIODone;
>  	bio->bi_private = bp;
>  	bio->bi_opf = REQ_OP_READ;
> +
> +	atomic_inc(&log->io_inflight);
>  	/*check if journaling to disk has been disabled*/
>  	if (log->no_integrity) {
>  		bio->bi_iter.bi_size = 0;
> @@ -2135,6 +2140,7 @@ static void lbmStartIO(struct lbuf * bp)
>  	bio->bi_private = bp;
>  	bio->bi_opf = REQ_OP_WRITE | REQ_SYNC;
>  
> +	atomic_inc(&log->io_inflight);

Why use an atomic for this?  The value can change after you test for it,
as there's no lock involved.

Do you really need to keep track of all of these "inflight"?  That feels
very "heavy" to me.

jfs developers, any ideas?

thanks,

greg k-h
_______________________________________________
Linux-kernel-mentees mailing list
Linux-kernel-mentees@lists.linuxfoundation.org
https://lists.linuxfoundation.org/mailman/listinfo/linux-kernel-mentees

^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2021-03-26 13:57 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-03-22 16:11 [PATCH resend] jfs: fix use-after-free in lbmIODone Anirudh Rayabharam
2021-03-22 16:11 ` Anirudh Rayabharam
2021-03-26 13:56 ` Greg KH
2021-03-26 13:56   ` Greg KH

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.