fs: xfs: add NULL pointer check

[PATCH] fs: xfs: add NULL pointer check

Posted by cgel.zte@gmail.com 4 years, 2 months ago

From: Lv Ruyi <lv.ruyi@zte.com.cn>

kmem_zalloc() is a memory allocation function which can return NULL when
some internal memory errors happen. It is safer to check NULL pointer.

Reported-by: Zeal Robot <zealci@zte.com.cn>
Signed-off-by: Lv Ruyi <lv.ruyi@zte.com.cn>
---
 fs/xfs/libxfs/xfs_attr_leaf.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/fs/xfs/libxfs/xfs_attr_leaf.c b/fs/xfs/libxfs/xfs_attr_leaf.c
index 014daa8c542d..e6694f49f563 100644
--- a/fs/xfs/libxfs/xfs_attr_leaf.c
+++ b/fs/xfs/libxfs/xfs_attr_leaf.c
@@ -1571,6 +1571,8 @@ xfs_attr3_leaf_compact(
 	trace_xfs_attr_leaf_compact(args);
 
 	tmpbuffer = kmem_alloc(args->geo->blksize, 0);
+	if (!tmpbuffer)
+		return;
 	memcpy(tmpbuffer, bp->b_addr, args->geo->blksize);
 	memset(bp->b_addr, 0, args->geo->blksize);
 	leaf_src = (xfs_attr_leafblock_t *)tmpbuffer;
@@ -2290,6 +2292,8 @@ xfs_attr3_leaf_unbalance(
 		struct xfs_attr3_icleaf_hdr tmphdr;
 
 		tmp_leaf = kmem_zalloc(state->args->geo->blksize, 0);
+		if (!tmp_leaf)
+			return;
 
 		/*
 		 * Copy the header into the temp leaf so that all the stuff
-- 
2.25.1

Re: [PATCH] fs: xfs: add NULL pointer check

Posted by Damien Le Moal 4 years, 2 months ago

On 3/28/22 12:26, cgel.zte@gmail.com wrote:
> From: Lv Ruyi <lv.ruyi@zte.com.cn>
> 
> kmem_zalloc() is a memory allocation function which can return NULL when
> some internal memory errors happen. It is safer to check NULL pointer.
> 
> Reported-by: Zeal Robot <zealci@zte.com.cn>
> Signed-off-by: Lv Ruyi <lv.ruyi@zte.com.cn>
> ---
>  fs/xfs/libxfs/xfs_attr_leaf.c | 4 ++++
>  1 file changed, 4 insertions(+)
> 
> diff --git a/fs/xfs/libxfs/xfs_attr_leaf.c b/fs/xfs/libxfs/xfs_attr_leaf.c
> index 014daa8c542d..e6694f49f563 100644
> --- a/fs/xfs/libxfs/xfs_attr_leaf.c
> +++ b/fs/xfs/libxfs/xfs_attr_leaf.c
> @@ -1571,6 +1571,8 @@ xfs_attr3_leaf_compact(
>  	trace_xfs_attr_leaf_compact(args);
>  
>  	tmpbuffer = kmem_alloc(args->geo->blksize, 0);

See kmem_alloc() code: this function cannot fail (it retries the
allocation until success). So checking for NULL pointer does not make sense.

> +	if (!tmpbuffer)
> +		return;
>  	memcpy(tmpbuffer, bp->b_addr, args->geo->blksize);
>  	memset(bp->b_addr, 0, args->geo->blksize);
>  	leaf_src = (xfs_attr_leafblock_t *)tmpbuffer;
> @@ -2290,6 +2292,8 @@ xfs_attr3_leaf_unbalance(
>  		struct xfs_attr3_icleaf_hdr tmphdr;
>  
>  		tmp_leaf = kmem_zalloc(state->args->geo->blksize, 0);
> +		if (!tmp_leaf)
> +			return;
>  
>  		/*
>  		 * Copy the header into the temp leaf so that all the stuff


-- 
Damien Le Moal
Western Digital Research

Re: [PATCH] fs: xfs: add NULL pointer check

Posted by Lv Ruyi 4 years, 2 months ago

Sorry, please ignore the noise.

thanks

Re: [PATCH] fs: xfs: add NULL pointer check

Posted by Damien Le Moal 4 years, 2 months ago

On 3/28/22 12:40, Damien Le Moal wrote:
> On 3/28/22 12:26, cgel.zte@gmail.com wrote:
>> From: Lv Ruyi <lv.ruyi@zte.com.cn>
>>
>> kmem_zalloc() is a memory allocation function which can return NULL when
>> some internal memory errors happen. It is safer to check NULL pointer.
>>
>> Reported-by: Zeal Robot <zealci@zte.com.cn>
>> Signed-off-by: Lv Ruyi <lv.ruyi@zte.com.cn>
>> ---
>>  fs/xfs/libxfs/xfs_attr_leaf.c | 4 ++++
>>  1 file changed, 4 insertions(+)
>>
>> diff --git a/fs/xfs/libxfs/xfs_attr_leaf.c b/fs/xfs/libxfs/xfs_attr_leaf.c
>> index 014daa8c542d..e6694f49f563 100644
>> --- a/fs/xfs/libxfs/xfs_attr_leaf.c
>> +++ b/fs/xfs/libxfs/xfs_attr_leaf.c
>> @@ -1571,6 +1571,8 @@ xfs_attr3_leaf_compact(
>>  	trace_xfs_attr_leaf_compact(args);
>>  
>>  	tmpbuffer = kmem_alloc(args->geo->blksize, 0);
> 
> See kmem_alloc() code: this function cannot fail (it retries the
> allocation until success). So checking for NULL pointer does not make sense.

Note: this comment is not 100% correct. kmem_alloc() can fail if
KM_MAYFAIL is specified as a flag. But that is not the case here.

> 
>> +	if (!tmpbuffer)
>> +		return;
>>  	memcpy(tmpbuffer, bp->b_addr, args->geo->blksize);
>>  	memset(bp->b_addr, 0, args->geo->blksize);
>>  	leaf_src = (xfs_attr_leafblock_t *)tmpbuffer;
>> @@ -2290,6 +2292,8 @@ xfs_attr3_leaf_unbalance(
>>  		struct xfs_attr3_icleaf_hdr tmphdr;
>>  
>>  		tmp_leaf = kmem_zalloc(state->args->geo->blksize, 0);
>> +		if (!tmp_leaf)
>> +			return;
>>  
>>  		/*
>>  		 * Copy the header into the temp leaf so that all the stuff
> 
> 


-- 
Damien Le Moal
Western Digital Research