[PATCH] ext4: fix potential use after free in __ext4_journal_stop

[PATCH] ext4: fix potential use after free in __ext4_journal_stop

[Lukas Czerner]

There is a use-after-free possibility in __ext4_journal_stop() in the
case that we free the handle in the first jbd2_journal_stop() because
we're referencing handle->h_err afterwards. This was introduced in
9705acd63b125dee8b15c705216d7186daea4625 and it is wrong. Fix it by
storing the handle->h_err value beforehand and avoid referencing
potentially freed handle.

Signed-off-by: Lukas Czerner <lczerner@redhat.com>
---
 fs/ext4/ext4_jbd2.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/fs/ext4/ext4_jbd2.c b/fs/ext4/ext4_jbd2.c
index d418431..e770c1ee 100644
--- a/fs/ext4/ext4_jbd2.c
+++ b/fs/ext4/ext4_jbd2.c
@@ -88,13 +88,13 @@ int __ext4_journal_stop(const char *where, unsigned int line, handle_t *handle)
 		return 0;
 	}
 
+	err = handle->h_err;
 	if (!handle->h_transaction) {
-		err = jbd2_journal_stop(handle);
-		return handle->h_err ? handle->h_err : err;
+		rc = jbd2_journal_stop(handle);
+		return err ? err : rc;
 	}
 
 	sb = handle->h_transaction->t_journal->j_private;
-	err = handle->h_err;
 	rc = jbd2_journal_stop(handle);
 
 	if (!err)
-- 
1.8.3.1

Re: [PATCH] ext4: fix potential use after free in __ext4_journal_stop

[Lukáš Czerner]

On Wed, 2 Sep 2015, Lukas Czerner wrote:

> Date: Wed,  2 Sep 2015 16:45:54 +0200
> From: Lukas Czerner <lczerner@redhat.com>
> To: linux-ext4@vger.kernel.org
> Cc: Lukas Czerner <lczerner@redhat.com>
> Subject: [PATCH] ext4: fix potential use after free in __ext4_journal_stop
> 
> There is a use-after-free possibility in __ext4_journal_stop() in the
> case that we free the handle in the first jbd2_journal_stop() because
> we're referencing handle->h_err afterwards. This was introduced in
> 9705acd63b125dee8b15c705216d7186daea4625 and it is wrong. Fix it by
> storing the handle->h_err value beforehand and avoid referencing
> potentially freed handle.

ping

-Lukas

> 
> Signed-off-by: Lukas Czerner <lczerner@redhat.com>
> ---
>  fs/ext4/ext4_jbd2.c | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/fs/ext4/ext4_jbd2.c b/fs/ext4/ext4_jbd2.c
> index d418431..e770c1ee 100644
> --- a/fs/ext4/ext4_jbd2.c
> +++ b/fs/ext4/ext4_jbd2.c
> @@ -88,13 +88,13 @@ int __ext4_journal_stop(const char *where, unsigned int line, handle_t *handle)
>  		return 0;
>  	}
>  
> +	err = handle->h_err;
>  	if (!handle->h_transaction) {
> -		err = jbd2_journal_stop(handle);
> -		return handle->h_err ? handle->h_err : err;
> +		rc = jbd2_journal_stop(handle);
> +		return err ? err : rc;
>  	}
>  
>  	sb = handle->h_transaction->t_journal->j_private;
> -	err = handle->h_err;
>  	rc = jbd2_journal_stop(handle);
>  
>  	if (!err)
> 

Re: [PATCH] ext4: fix potential use after free in __ext4_journal_stop

[Andreas Dilger]

[-- Attachment #1: Type: text/plain, Size: 1897 bytes --]


> On Oct 5, 2015, at 8:18 AM, Lukáš Czerner <lczerner@redhat.com> wrote:
> 
> On Wed, 2 Sep 2015, Lukas Czerner wrote:
> 
>> Date: Wed,  2 Sep 2015 16:45:54 +0200
>> From: Lukas Czerner <lczerner@redhat.com>
>> To: linux-ext4@vger.kernel.org
>> Cc: Lukas Czerner <lczerner@redhat.com>
>> Subject: [PATCH] ext4: fix potential use after free in __ext4_journal_stop
>> 
>> There is a use-after-free possibility in __ext4_journal_stop() in the
>> case that we free the handle in the first jbd2_journal_stop() because
>> we're referencing handle->h_err afterwards. This was introduced in
>> 9705acd63b125dee8b15c705216d7186daea4625 and it is wrong. Fix it by
>> storing the handle->h_err value beforehand and avoid referencing
>> potentially freed handle.
> 
> ping
>> 
>> Signed-off-by: Lukas Czerner <lczerner@redhat.com>

Reviewed-by: Andreas Dilger <adilger@dilger.ca>

>> ---
>> fs/ext4/ext4_jbd2.c | 6 +++---
>> 1 file changed, 3 insertions(+), 3 deletions(-)
>> 
>> diff --git a/fs/ext4/ext4_jbd2.c b/fs/ext4/ext4_jbd2.c
>> index d418431..e770c1ee 100644
>> --- a/fs/ext4/ext4_jbd2.c
>> +++ b/fs/ext4/ext4_jbd2.c
>> @@ -88,13 +88,13 @@ int __ext4_journal_stop(const char *where, unsigned int line, handle_t *handle)
>> 		return 0;
>> 	}
>> 
>> +	err = handle->h_err;
>> 	if (!handle->h_transaction) {
>> -		err = jbd2_journal_stop(handle);
>> -		return handle->h_err ? handle->h_err : err;
>> +		rc = jbd2_journal_stop(handle);
>> +		return err ? err : rc;
>> 	}
>> 
>> 	sb = handle->h_transaction->t_journal->j_private;
>> -	err = handle->h_err;
>> 	rc = jbd2_journal_stop(handle);
>> 
>> 	if (!err)
>> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-ext4" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html


Cheers, Andreas






[-- Attachment #2: Message signed with OpenPGP using GPGMail --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [PATCH] ext4: fix potential use after free in __ext4_journal_stop

[Theodore Ts'o]

On Mon, Oct 05, 2015 at 01:20:52PM -0600, Andreas Dilger wrote:
> > On Oct 5, 2015, at 8:18 AM, Lukáš Czerner <lczerner@redhat.com> wrote:
> > On Wed, 2 Sep 2015, Lukas Czerner wrote:
> > 
> >> Date: Wed,  2 Sep 2015 16:45:54 +0200
> >> From: Lukas Czerner <lczerner@redhat.com>
> >> To: linux-ext4@vger.kernel.org
> >> Cc: Lukas Czerner <lczerner@redhat.com>
> >> Subject: [PATCH] ext4: fix potential use after free in __ext4_journal_stop
> >> 
> >> There is a use-after-free possibility in __ext4_journal_stop() in the
> >> case that we free the handle in the first jbd2_journal_stop() because
> >> we're referencing handle->h_err afterwards. This was introduced in
> >> 9705acd63b125dee8b15c705216d7186daea4625 and it is wrong. Fix it by
> >> storing the handle->h_err value beforehand and avoid referencing
> >> potentially freed handle.
> > 
> > ping
> >> 
> >> Signed-off-by: Lukas Czerner <lczerner@redhat.com>
> 
> Reviewed-by: Andreas Dilger <adilger@dilger.ca>

Applied, thanks.

						- Ted
--
To unsubscribe from this list: send the line "unsubscribe linux-ext4" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html