[PATCH -next 0/2] Fix two issue in jbd2_fc_wait_bufs

[PATCH -next 0/2] Fix two issue in jbd2_fc_wait_bufs

[Ye Bin]

Ye Bin (2):
  jbd2: fix potential buffer head reference count leak
  jbd2: fix potential use-after-free in jbd2_fc_wait_bufs

 fs/jbd2/journal.c | 10 ++++++++--
 1 file changed, 8 insertions(+), 2 deletions(-)

-- 
2.31.1

[PATCH -next 1/2] jbd2: fix potential buffer head reference count leak

[Ye Bin]

As in 'jbd2_fc_wait_bufs' if buffer isn't uptodate, will return -EIO without
update 'journal->j_fc_off'. But 'jbd2_fc_release_bufs' will release buffer head
from ‘j_fc_off - 1’ if 'bh' is NULL will terminal release which will lead to
buffer head buffer head reference count leak.
To solve above issue, update 'journal->j_fc_off' before return -EIO.

Signed-off-by: Ye Bin <yebin10@huawei.com>
---
 fs/jbd2/journal.c | 8 +++++++-
 1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/fs/jbd2/journal.c b/fs/jbd2/journal.c
index 140b070471c0..1c833d8cb0fe 100644
--- a/fs/jbd2/journal.c
+++ b/fs/jbd2/journal.c
@@ -925,8 +925,14 @@ int jbd2_fc_wait_bufs(journal_t *journal, int num_blks)
 		wait_on_buffer(bh);
 		put_bh(bh);
 		journal->j_fc_wbuf[i] = NULL;
-		if (unlikely(!buffer_uptodate(bh)))
+		/*
+		 * Update j_fc_off so jbd2_fc_release_bufs can release remain
+		 * buffer head.
+		 */
+		if (unlikely(!buffer_uptodate(bh))) {
+			journal->j_fc_off = i;
 			return -EIO;
+		}
 	}
 
 	return 0;
-- 
2.31.1

[PATCH -next 2/2] jbd2: fix potential use-after-free in jbd2_fc_wait_bufs

[Ye Bin]

In 'jbd2_fc_wait_bufs' use 'bh' after put buffer head reference count
which may lead to use-after-free.
So judge buffer if uptodate before put buffer head reference count.

Signed-off-by: Ye Bin <yebin10@huawei.com>
---
 fs/jbd2/journal.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/fs/jbd2/journal.c b/fs/jbd2/journal.c
index 1c833d8cb0fe..4d49f629d06b 100644
--- a/fs/jbd2/journal.c
+++ b/fs/jbd2/journal.c
@@ -923,16 +923,16 @@ int jbd2_fc_wait_bufs(journal_t *journal, int num_blks)
 	for (i = j_fc_off - 1; i >= j_fc_off - num_blks; i--) {
 		bh = journal->j_fc_wbuf[i];
 		wait_on_buffer(bh);
-		put_bh(bh);
-		journal->j_fc_wbuf[i] = NULL;
 		/*
 		 * Update j_fc_off so jbd2_fc_release_bufs can release remain
 		 * buffer head.
 		 */
 		if (unlikely(!buffer_uptodate(bh))) {
-			journal->j_fc_off = i;
+			journal->j_fc_off = i + 1;
 			return -EIO;
 		}
+		put_bh(bh);
+		journal->j_fc_wbuf[i] = NULL;
 	}
 
 	return 0;
-- 
2.31.1

Re: [PATCH -next 1/2] jbd2: fix potential buffer head reference count leak

[Jan Kara]

On Wed 14-09-22 18:08:11, Ye Bin wrote:
> As in 'jbd2_fc_wait_bufs' if buffer isn't uptodate, will return -EIO without
> update 'journal->j_fc_off'. But 'jbd2_fc_release_bufs' will release buffer head
> from ‘j_fc_off - 1’ if 'bh' is NULL will terminal release which will lead to
> buffer head buffer head reference count leak.
> To solve above issue, update 'journal->j_fc_off' before return -EIO.
> 
> Signed-off-by: Ye Bin <yebin10@huawei.com>

Looks good. Feel free to add:

Reviewed-by: Jan Kara <jack@suse.cz>

								Honza

> ---
>  fs/jbd2/journal.c | 8 +++++++-
>  1 file changed, 7 insertions(+), 1 deletion(-)
> 
> diff --git a/fs/jbd2/journal.c b/fs/jbd2/journal.c
> index 140b070471c0..1c833d8cb0fe 100644
> --- a/fs/jbd2/journal.c
> +++ b/fs/jbd2/journal.c
> @@ -925,8 +925,14 @@ int jbd2_fc_wait_bufs(journal_t *journal, int num_blks)
>  		wait_on_buffer(bh);
>  		put_bh(bh);
>  		journal->j_fc_wbuf[i] = NULL;
> -		if (unlikely(!buffer_uptodate(bh)))
> +		/*
> +		 * Update j_fc_off so jbd2_fc_release_bufs can release remain
> +		 * buffer head.
> +		 */
> +		if (unlikely(!buffer_uptodate(bh))) {
> +			journal->j_fc_off = i;
>  			return -EIO;
> +		}
>  	}
>  
>  	return 0;
> -- 
> 2.31.1
> 
-- 
Jan Kara <jack@suse.com>
SUSE Labs, CR

Re: [PATCH -next 2/2] jbd2: fix potential use-after-free in jbd2_fc_wait_bufs

[Jan Kara]

On Wed 14-09-22 18:08:12, Ye Bin wrote:
> In 'jbd2_fc_wait_bufs' use 'bh' after put buffer head reference count
> which may lead to use-after-free.
> So judge buffer if uptodate before put buffer head reference count.
> 
> Signed-off-by: Ye Bin <yebin10@huawei.com>

Looks good. Feel free to add:

Reviewed-by: Jan Kara <jack@suse.cz>

								Honza

> ---
>  fs/jbd2/journal.c | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/fs/jbd2/journal.c b/fs/jbd2/journal.c
> index 1c833d8cb0fe..4d49f629d06b 100644
> --- a/fs/jbd2/journal.c
> +++ b/fs/jbd2/journal.c
> @@ -923,16 +923,16 @@ int jbd2_fc_wait_bufs(journal_t *journal, int num_blks)
>  	for (i = j_fc_off - 1; i >= j_fc_off - num_blks; i--) {
>  		bh = journal->j_fc_wbuf[i];
>  		wait_on_buffer(bh);
> -		put_bh(bh);
> -		journal->j_fc_wbuf[i] = NULL;
>  		/*
>  		 * Update j_fc_off so jbd2_fc_release_bufs can release remain
>  		 * buffer head.
>  		 */
>  		if (unlikely(!buffer_uptodate(bh))) {
> -			journal->j_fc_off = i;
> +			journal->j_fc_off = i + 1;
>  			return -EIO;
>  		}
> +		put_bh(bh);
> +		journal->j_fc_wbuf[i] = NULL;
>  	}
>  
>  	return 0;
> -- 
> 2.31.1
> 
-- 
Jan Kara <jack@suse.com>
SUSE Labs, CR

Re: [PATCH -next 0/2] Fix two issue in jbd2_fc_wait_bufs

[Theodore Ts'o]

On Wed, 14 Sep 2022 18:08:10 +0800, Ye Bin wrote:
> Ye Bin (2):
>   jbd2: fix potential buffer head reference count leak
>   jbd2: fix potential use-after-free in jbd2_fc_wait_bufs
> 
> fs/jbd2/journal.c | 10 ++++++++--
>  1 file changed, 8 insertions(+), 2 deletions(-)
> 
> [...]

Applied, thanks!

[1/2] jbd2: fix potential buffer head reference count leak
      commit: 53dee029ec341f5c985b880de34f64de60072688
[2/2] jbd2: fix potential use-after-free in jbd2_fc_wait_bufs
      commit: 6e16a2d9ff8f3ea4a53e10ae4607feb446cf5c90

Best regards,
-- 
Theodore Ts'o <tytso@mit.edu>