kernel-janitors.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] fs/ntfs3: add checks for allocation failure
@ 2021-08-24 11:52 Dan Carpenter
  2021-08-24 16:02 ` Kari Argillander
  2021-08-27 17:16 ` Konstantin Komarov
  0 siblings, 2 replies; 4+ messages in thread
From: Dan Carpenter @ 2021-08-24 11:52 UTC (permalink / raw)
  To: Konstantin Komarov; +Cc: ntfs3, linux-kernel, kernel-janitors

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@oracle.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


^ permalink raw reply related	[flat|nested] 4+ messages in thread

* Re: [PATCH] fs/ntfs3: add checks for allocation failure
  2021-08-24 11:52 [PATCH] fs/ntfs3: add checks for allocation failure Dan Carpenter
@ 2021-08-24 16:02 ` Kari Argillander
  2021-08-24 16:57   ` Dan Carpenter
  2021-08-27 17:16 ` Konstantin Komarov
  1 sibling, 1 reply; 4+ messages in thread
From: Kari Argillander @ 2021-08-24 16:02 UTC (permalink / raw)
  To: Dan Carpenter; +Cc: Konstantin Komarov, ntfs3, linux-kernel, kernel-janitors

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@oracle.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@gmail.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
> 
> 

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH] fs/ntfs3: add checks for allocation failure
  2021-08-24 16:02 ` Kari Argillander
@ 2021-08-24 16:57   ` Dan Carpenter
  0 siblings, 0 replies; 4+ messages in thread
From: Dan Carpenter @ 2021-08-24 16:57 UTC (permalink / raw)
  To: Kari Argillander; +Cc: Konstantin Komarov, ntfs3, linux-kernel, kernel-janitors

On Tue, Aug 24, 2021 at 07:02:44PM +0300, Kari Argillander wrote:
> 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@oracle.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. 

There is a way to do allocation fault injection, but I haven't messed
with it.

My guess is that syzbot will soon start reporting a ton of bugs.  ;)

regards,
dan carpenter


^ permalink raw reply	[flat|nested] 4+ messages in thread

* RE: [PATCH] fs/ntfs3: add checks for allocation failure
  2021-08-24 11:52 [PATCH] fs/ntfs3: add checks for allocation failure Dan Carpenter
  2021-08-24 16:02 ` Kari Argillander
@ 2021-08-27 17:16 ` Konstantin Komarov
  1 sibling, 0 replies; 4+ messages in thread
From: Konstantin Komarov @ 2021-08-27 17:16 UTC (permalink / raw)
  To: Dan Carpenter; +Cc: ntfs3, linux-kernel, kernel-janitors

> From: Dan Carpenter <dan.carpenter@oracle.com>
> Sent: Tuesday, August 24, 2021 2:53 PM
> To: Konstantin Komarov <almaz.alexandrovich@paragon-software.com>
> Cc: ntfs3@lists.linux.dev; linux-kernel@vger.kernel.org; kernel-janitors@vger.kernel.org
> Subject: [PATCH] fs/ntfs3: add checks for allocation failure
> 
> 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@oracle.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

Hi Dan! Thanks, applied :)

Best regards

^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2021-08-27 17:17 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-08-24 11:52 [PATCH] fs/ntfs3: add checks for allocation failure Dan Carpenter
2021-08-24 16:02 ` Kari Argillander
2021-08-24 16:57   ` Dan Carpenter
2021-08-27 17:16 ` Konstantin Komarov

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).