2022-05-30 11:33:23

by Ding Xiang

[permalink] [raw]
Subject: [PATCH] ext4: change variable "count" to signed integer

Since dx_make_map() may return -EFSCORRUPTED now,
so change "count" to signed integer.

Signed-off-by: Ding Xiang <[email protected]>
---
fs/ext4/namei.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/fs/ext4/namei.c b/fs/ext4/namei.c
index 47d0ca4c795b..db4ba99d1ceb 100644
--- a/fs/ext4/namei.c
+++ b/fs/ext4/namei.c
@@ -1929,7 +1929,8 @@ static struct ext4_dir_entry_2 *do_split(handle_t *handle, struct inode *dir,
struct dx_hash_info *hinfo)
{
unsigned blocksize = dir->i_sb->s_blocksize;
- unsigned count, continued;
+ unsigned continued;
+ int count;
struct buffer_head *bh2;
ext4_lblk_t newblock;
u32 hash2;
--
2.31.1





2022-06-18 03:03:59

by Theodore Ts'o

[permalink] [raw]
Subject: Re: [PATCH] ext4: change variable "count" to signed integer

On Mon, 30 May 2022 18:00:47 +0800, Ding Xiang wrote:
> Since dx_make_map() may return -EFSCORRUPTED now,
> so change "count" to signed integer.
>
>

Applied, thanks!

[1/1] ext4: change variable "count" to signed integer
commit: fefb759df063599ad483422eb07ef8e14c612cc2

Best regards,
--
Theodore Ts'o <[email protected]>

2022-06-18 09:52:40

by Dan Carpenter

[permalink] [raw]
Subject: Re: [PATCH] ext4: change variable "count" to signed integer

On Fri, Jun 17, 2022 at 10:59:01PM -0400, Theodore Ts'o wrote:
> On Mon, 30 May 2022 18:00:47 +0800, Ding Xiang wrote:
> > Since dx_make_map() may return -EFSCORRUPTED now,
> > so change "count" to signed integer.
> >
> >
>
> Applied, thanks!
>
> [1/1] ext4: change variable "count" to signed integer
> commit: fefb759df063599ad483422eb07ef8e14c612cc2
>

There was some kind of process error here...

1) That commit somehow never made it to linux-next.

2) No Fixes tag. Presumably Greg searches for Fixes tags before he back
ports patches. The original commit 46c116b920eb ("ext4: verify dir
block before splitting it") has been back ported to stable already.

3) I don't know why I didn't send a patch for this bug. I looked at it
on May 23 but never sent a patch. Sorry?

4) There are a dozen other static checkers which are supposed to have
warned about this. I search lore.kernel.org for 46c116b920eb
and 46c116b920ebec58031f0a78c5ea9599b0d2a371 but I couldn't find any
static checker warnings.

regards,
dan carpenter

2022-06-18 22:24:42

by Theodore Ts'o

[permalink] [raw]
Subject: Re: [PATCH] ext4: change variable "count" to signed integer

On Sat, Jun 18, 2022 at 12:33:01PM +0300, Dan Carpenter wrote:
> On Fri, Jun 17, 2022 at 10:59:01PM -0400, Theodore Ts'o wrote:
> > On Mon, 30 May 2022 18:00:47 +0800, Ding Xiang wrote:
> > > Since dx_make_map() may return -EFSCORRUPTED now,
> > > so change "count" to signed integer.
> > >
> > >
> >
> > Applied, thanks!
> >
> > [1/1] ext4: change variable "count" to signed integer
> > commit: fefb759df063599ad483422eb07ef8e14c612cc2
> >
>
> There was some kind of process error here...
>
> 1) That commit somehow never made it to linux-next.

That's I only pushed it out Friday night (US/Eastern), and Stephen
Rothwell creates new linux-next release based on snapshots taken
Monday through Friday in the Morning (AU/Canberra time).

Things have been crazy busy, so a last set of ext4 backports only
happened Friday starting around 10pm localtime. (Yes, I have no
life.)


> 2) No Fixes tag. Presumably Greg searches for Fixes tags before he back
> ports patches. The original commit 46c116b920eb ("ext4: verify dir
> block before splitting it") has been back ported to stable already.

I did add a Fixes tag in what is in the ext4 tree. I don't always
mention when I've rewritten since that requries manual editing of the
"b4 ty" generated acknowledgement.

In the ideal world when I rewrite the one-line snapshot, at the *very*
least it should show up in the Applied/thanks. Maybe something like
this:

[1/1] ext4: change variable "count" to signed integer
commit: fefb759df063599ad483422eb07ef8e14c612cc2
rewritten summary: ext4: make variable "count" signed

... and if the commit description is rewritten, maybe the "b4 ty"
e-mail should mention it. (Very often I end up rewriting commit
descriptions, especially when the original poster's first language is
not English.)

For the record, this is what is in the ext4 tree that I plan to push
to Linus is:

commit fefb759df063599ad483422eb07ef8e14c612cc2
Author: Ding Xiang <[email protected]>
Date: Mon May 30 18:00:47 2022 +0800

ext4: make variable "count" signed

Since dx_make_map() may return -EFSCORRUPTED now, so change "count" to
be a signed integer so we can correct check for an error code returned
by dx_make_map().

Fixes: 46c116b920eb ("ext4: verify dir block before splitting it")
Signed-off-by: Ding Xiang <[email protected]>
Link: https://lore.kernel.org/r/[email protected]
Signed-off-by: Theodore Ts'o <[email protected]>

Cheers,

- Ted

2022-06-20 05:54:25

by Dan Carpenter

[permalink] [raw]
Subject: Re: [PATCH] ext4: change variable "count" to signed integer

On Sat, Jun 18, 2022 at 06:22:00PM -0400, Theodore Ts'o wrote:
> On Sat, Jun 18, 2022 at 12:33:01PM +0300, Dan Carpenter wrote:
> > On Fri, Jun 17, 2022 at 10:59:01PM -0400, Theodore Ts'o wrote:
> > > On Mon, 30 May 2022 18:00:47 +0800, Ding Xiang wrote:
> > > > Since dx_make_map() may return -EFSCORRUPTED now,
> > > > so change "count" to signed integer.
> > > >
> > > >
> > >
> > > Applied, thanks!
> > >
> > > [1/1] ext4: change variable "count" to signed integer
> > > commit: fefb759df063599ad483422eb07ef8e14c612cc2
> > >
> >
> > There was some kind of process error here...
> >
> > 1) That commit somehow never made it to linux-next.
>
> That's I only pushed it out Friday night (US/Eastern), and Stephen
> Rothwell creates new linux-next release based on snapshots taken
> Monday through Friday in the Morning (AU/Canberra time).
>
> Things have been crazy busy, so a last set of ext4 backports only
> happened Friday starting around 10pm localtime. (Yes, I have no
> life.)

Ah, sorry. I didn't look at the dates carefully. Thanks for taking
care of this.

It's so weird that kbuild and I didn't send this bug report. I can't
figure that bit out at all. It's not super common though and other
people are looking at static analysis so it worked out in the end.

regards,
dan carpenter