[PATCH] reiserfs: journal: Increase jl pointer check

[PATCH] reiserfs: journal: Increase jl pointer check

[Li zeming]

If kzalloc fails to allocate the jl pointer, NULL is returned directly.

Signed-off-by: Li zeming <zeming@nfschina.com>
---
 fs/reiserfs/journal.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/fs/reiserfs/journal.c b/fs/reiserfs/journal.c
index 94addfcefede..d64e9de126c1 100644
--- a/fs/reiserfs/journal.c
+++ b/fs/reiserfs/journal.c
@@ -2569,6 +2569,9 @@ static struct reiserfs_journal_list *alloc_journal_list(struct super_block *s)
 	struct reiserfs_journal_list *jl;
 	jl = kzalloc(sizeof(struct reiserfs_journal_list),
 		     GFP_NOFS | __GFP_NOFAIL);
+	if (!jl)
+		return NULL;
+
 	INIT_LIST_HEAD(&jl->j_list);
 	INIT_LIST_HEAD(&jl->j_working_list);
 	INIT_LIST_HEAD(&jl->j_tail_bh_list);
-- 
2.18.2

Re: [PATCH] reiserfs: journal: Increase jl pointer check

[NeilBrown]

On Tue, 25 Oct 2022, Li zeming wrote:
> If kzalloc fails to allocate the jl pointer, NULL is returned directly.
> 
> Signed-off-by: Li zeming <zeming@nfschina.com>
> ---
>  fs/reiserfs/journal.c | 3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/fs/reiserfs/journal.c b/fs/reiserfs/journal.c
> index 94addfcefede..d64e9de126c1 100644
> --- a/fs/reiserfs/journal.c
> +++ b/fs/reiserfs/journal.c
> @@ -2569,6 +2569,9 @@ static struct reiserfs_journal_list *alloc_journal_list(struct super_block *s)
>  	struct reiserfs_journal_list *jl;
>  	jl = kzalloc(sizeof(struct reiserfs_journal_list),
>  		     GFP_NOFS | __GFP_NOFAIL);
> +	if (!jl)
> +		return NULL;
> +

What do you think the __GFP_NOFAIL flag might mean?

NeilBrown



>  	INIT_LIST_HEAD(&jl->j_list);
>  	INIT_LIST_HEAD(&jl->j_working_list);
>  	INIT_LIST_HEAD(&jl->j_tail_bh_list);
> -- 
> 2.18.2
> 
> 

Re: [PATCH] reiserfs: journal: Increase jl pointer check

[Li zeming]

I'm sorry, I think __GFP_NOFAIL should be like this:

The __GFP_NOFAIL flag will cause memory to be allocated an infinite
 number of times until the allocation is successful, but it is best to
 use it only for very necessary code, and try not to use it.

I'm not sure alloc_journal_list function is a very important function
 here. If it is, this patch ignores it and does not consider it anymore
 __GFP_NOFAIL allocatiIon problem.

Re: [PATCH] reiserfs: journal: Increase jl pointer check

[NeilBrown]

On Wed, 26 Oct 2022, Li zeming wrote:
> I'm sorry, I think __GFP_NOFAIL should be like this:
> 
> The __GFP_NOFAIL flag will cause memory to be allocated an infinite
>  number of times until the allocation is successful, but it is best to
>  use it only for very necessary code, and try not to use it.

I agree with that.  But you didn't try not to use it - you left it there
in the code.
Had you removed the flag as well, then your patch would have made a bit
more sense.  However you would then need to explain why it is safe to
return NULL from alloc_journal_list().  Are you sure callers don't try
to dereference the pointer?

> 
> I'm not sure alloc_journal_list function is a very important function
>  here. If it is, this patch ignores it and does not consider it anymore
>  __GFP_NOFAIL allocatiIon problem.
> 
> 

You are correct that the function isn't very important, but that is
because reiserfs itself isn't very important.
You might not have noticed, but in the reiserfs Kconfig file it says:

	  Reiserfs is deprecated and scheduled to be removed from the kernel
	  in 2025. If you are still using it, please migrate to another
	  filesystem or tell us your usecase for reiserfs.

So it will still be around for a few years, but there isn't much point
with small clean-ups like this as hardly anyone uses the code, and in a
few years it won't be in mainline Linux at all.

Thanks,
NeilBrown

Re: [PATCH] reiserfs: journal: Increase jl pointer check

[Li zeming]

many thanks for your reply. I think I should do something more meaningful
.