[PATCH] fs: ext4: Fix dereferencing the null pointer 'ptr'

[PATCH] fs: ext4: Fix dereferencing the null pointer 'ptr'

[Mikhail Ilin]

 If memory allocation fails on line 780, then 'fail' will be
 jumped to and 'ptr' will be null, causing it to be dereferenced it on line
 855. Thus, before using 'ptr[i]' one must make sure that the 'ptr' pointer
 is not NULL.

Fixes: 934b14f2bb30 ("ext4: free allocations by parse_path()")
Signed-off-by: Mikhail Ilin <ilin.mikhail.ol@gmail.com>
---
 fs/ext4/ext4_common.c | 10 ++++++----
 1 file changed, 6 insertions(+), 4 deletions(-)

diff --git a/fs/ext4/ext4_common.c b/fs/ext4/ext4_common.c
index 1185cb2c04..3cdd1a04a9 100644
--- a/fs/ext4/ext4_common.c
+++ b/fs/ext4/ext4_common.c
@@ -851,10 +851,12 @@ end:
 fail:
 	free(depth_dirname);
 	free(parse_dirname);
-	for (i = 0; i < depth; i++) {
-		if (!ptr[i])
-			break;
-		free(ptr[i]);
+	if (ptr) {
+		for (i = 0; i < depth; i++) {
+			if (!ptr[i])
+				break;
+			free(ptr[i]);
+		}
 	}
 	free(ptr);
 	free(parent_inode);
-- 
2.17.1

Re: [PATCH] fs: ext4: Fix dereferencing the null pointer 'ptr'

[Stefan Roese]

On 23.11.22 09:06, Mikhail Ilin wrote:
>   If memory allocation fails on line 780, then 'fail' will be
>   jumped to and 'ptr' will be null, causing it to be dereferenced it on line
>   855. Thus, before using 'ptr[i]' one must make sure that the 'ptr' pointer
>   is not NULL.

Nitpicking. You seem to have a leading space in this comment
block. Please remove next time.

> 
> Fixes: 934b14f2bb30 ("ext4: free allocations by parse_path()")
> Signed-off-by: Mikhail Ilin <ilin.mikhail.ol@gmail.com>
> ---
>   fs/ext4/ext4_common.c | 10 ++++++----
>   1 file changed, 6 insertions(+), 4 deletions(-)
> 
> diff --git a/fs/ext4/ext4_common.c b/fs/ext4/ext4_common.c
> index 1185cb2c04..3cdd1a04a9 100644
> --- a/fs/ext4/ext4_common.c
> +++ b/fs/ext4/ext4_common.c
> @@ -851,10 +851,12 @@ end:
>   fail:
>   	free(depth_dirname);
>   	free(parse_dirname);
> -	for (i = 0; i < depth; i++) {
> -		if (!ptr[i])
> -			break;
> -		free(ptr[i]);
> +	if (ptr) {
> +		for (i = 0; i < depth; i++) {
> +			if (!ptr[i])
> +				break;
> +			free(ptr[i]);
> +		}
>   	}
>   	free(ptr);

Won't this fail with ptr == NULL? Please also include the free(ptr) into
the if (ptr) { } part.

Thanks,
Stefan