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 <[email protected]>
---
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
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 <[email protected]>
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 <[email protected]>
> ---
> 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
>
>
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 <[email protected]>
>
> 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
> From: Dan Carpenter <[email protected]>
> Sent: Tuesday, August 24, 2021 2:53 PM
> To: Konstantin Komarov <[email protected]>
> Cc: [email protected]; [email protected]; [email protected]
> 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 <[email protected]>
> ---
> 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