linux-kernel - Re: [PATCH] fs/ntfs3: add checks for allocation failure

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Tue, 24 Aug 2021 19:02:44 +0300
From:   Kari Argillander <kari.argillander@...il.com>
To:     Dan Carpenter <dan.carpenter@...cle.com>
Cc:     Konstantin Komarov <almaz.alexandrovich@...agon-software.com>,
        ntfs3@...ts.linux.dev, linux-kernel@...r.kernel.org,
        kernel-janitors@...r.kernel.org
Subject: Re: [PATCH] fs/ntfs3: add checks for allocation failure

On Tue, Aug 24, 2021 at 02:52:36PM +0300, Dan Carpenter wrote:
> Add a check for when the kzalloc() in init_rsttbl() fails.  Some of
> the callers checked for NULL and some did not.  I went down the call
> tree and added NULL checks where ever they were missing.
> 
> Fixes: b46acd6a6a62 ("fs/ntfs3: Add NTFS journal")
> Signed-off-by: Dan Carpenter <dan.carpenter@...cle.com>

Seems ok. It is not easist file to follow. log_replay is monster and
it should be refactor in some point. I'm certain that many more bugs
will be founded there. Also at least community does not have very good
testing interface for this. 

Reviewed-by: Kari Argillander <kari.argillander@...il.com>

> ---
>  fs/ntfs3/fslog.c | 21 ++++++++++++++++++---
>  1 file changed, 18 insertions(+), 3 deletions(-)
> 
> diff --git a/fs/ntfs3/fslog.c b/fs/ntfs3/fslog.c
> index 397ba6a956e7..209fe6ddead0 100644
> --- a/fs/ntfs3/fslog.c
> +++ b/fs/ntfs3/fslog.c
> @@ -807,7 +807,11 @@ static inline struct RESTART_TABLE *init_rsttbl(u16 esize, u16 used)
>  	u32 off;
>  	u32 bytes = esize * used + sizeof(struct RESTART_TABLE);
>  	u32 lf = sizeof(struct RESTART_TABLE) + (used - 1) * esize;
> -	struct RESTART_TABLE *t = ntfs_zalloc(bytes);
> +	struct RESTART_TABLE *t;
> +
> +	t = ntfs_zalloc(bytes);
> +	if (!t)
> +		return NULL;
>  
>  	t->size = cpu_to_le16(esize);
>  	t->used = cpu_to_le16(used);
> @@ -831,7 +835,11 @@ static inline struct RESTART_TABLE *extend_rsttbl(struct RESTART_TABLE *tbl,
>  	u16 esize = le16_to_cpu(tbl->size);
>  	__le32 osize = cpu_to_le32(bytes_per_rt(tbl));
>  	u32 used = le16_to_cpu(tbl->used);
> -	struct RESTART_TABLE *rt = init_rsttbl(esize, used + add);
> +	struct RESTART_TABLE *rt;
> +
> +	rt = init_rsttbl(esize, used + add);
> +	if (!rt)
> +		return NULL;
>  
>  	memcpy(rt + 1, tbl + 1, esize * used);
>  
> @@ -864,8 +872,11 @@ static inline void *alloc_rsttbl_idx(struct RESTART_TABLE **tbl)
>  	__le32 *e;
>  	struct RESTART_TABLE *t = *tbl;
>  
> -	if (!t->first_free)
> +	if (!t->first_free) {
>  		*tbl = t = extend_rsttbl(t, 16, ~0u);
> +		if (!t)
> +			return NULL;
> +	}
>  
>  	off = le32_to_cpu(t->first_free);
>  
> @@ -4482,6 +4493,10 @@ int log_replay(struct ntfs_inode *ni, bool *initialized)
>  		}
>  
>  		dp = alloc_rsttbl_idx(&dptbl);
> +		if (!dp) {
> +			err = -ENOMEM;
> +			goto out;
> +		}
>  		dp->target_attr = cpu_to_le32(t16);
>  		dp->transfer_len = cpu_to_le32(t32 << sbi->cluster_bits);
>  		dp->lcns_follow = cpu_to_le32(t32);
> -- 
> 2.20.1
> 
>