2019-08-13 04:06:42

by Jari Ruusu

[permalink] [raw]
Subject: Re: [PATCH 3.16 043/157] ext4: brelse all indirect buffer inext4_ind_remove_space()

Ben Hutchings wrote:
> From: "zhangyi (F)" <[email protected]>
>
> commit 674a2b27234d1b7afcb0a9162e81b2e53aeef217 upstream.

[snip]

> --- a/fs/ext4/indirect.c
> +++ b/fs/ext4/indirect.c
> @@ -1481,10 +1481,14 @@ end_range:
> partial->p + 1,
> partial2->p,
> (chain+n-1) - partial);
> - BUFFER_TRACE(partial->bh, "call brelse");
> - brelse(partial->bh);
> - BUFFER_TRACE(partial2->bh, "call brelse");
> - brelse(partial2->bh);
> + while (partial > chain) {
> + BUFFER_TRACE(partial->bh, "call brelse");
> + brelse(partial->bh);
> + }
> + while (partial2 > chain2) {
> + BUFFER_TRACE(partial2->bh, "call brelse");
> + brelse(partial2->bh);
> + }
> return 0;
> }
>

Above patch is really messed up. Alone that patch is livelocking
and file system corrupting. Look at those new while loops. Once the
while condition is true once, it is ALWAYS true, so it livelocks.

It absolutely needs follow-up patch from <[email protected]>
"ext4: cleanup bh release code in ext4_ind_remove_space()"
upstream commit 5e86bdda41534e17621d5a071b294943cae4376e.

For more info about how to trigger that bug, see this earlier email

https://marc.info/?l=linux-kernel&m=155419973129522&w=2

For 3.16 kernels you may need to set CONFIG_EXT4_USE_FOR_EXT23=y
so that ext4 code handles ext3 file systems.

--
Jari Ruusu 4096R/8132F189 12D6 4C3A DCDA 0AA4 27BD ACDF F073 3C80 8132 F189


2019-08-13 11:40:02

by Ben Hutchings

[permalink] [raw]
Subject: Re: [PATCH 3.16 043/157] ext4: brelse all indirect buffer inext4_ind_remove_space()

On Tue, 2019-08-13 at 07:06 +0300, Jari Ruusu wrote:
> Ben Hutchings wrote:
> > From: "zhangyi (F)" <[email protected]>
> >
> > commit 674a2b27234d1b7afcb0a9162e81b2e53aeef217 upstream.
>
> [snip]
>
> > --- a/fs/ext4/indirect.c
> > +++ b/fs/ext4/indirect.c
> > @@ -1481,10 +1481,14 @@ end_range:
> > partial->p + 1,
> > partial2->p,
> > (chain+n-1) - partial);
> > - BUFFER_TRACE(partial->bh, "call brelse");
> > - brelse(partial->bh);
> > - BUFFER_TRACE(partial2->bh, "call brelse");
> > - brelse(partial2->bh);
> > + while (partial > chain) {
> > + BUFFER_TRACE(partial->bh, "call brelse");
> > + brelse(partial->bh);
> > + }
> > + while (partial2 > chain2) {
> > + BUFFER_TRACE(partial2->bh, "call brelse");
> > + brelse(partial2->bh);
> > + }
> > return 0;
> > }
> >
>
> Above patch is really messed up. Alone that patch is livelocking
> and file system corrupting. Look at those new while loops. Once the
> while condition is true once, it is ALWAYS true, so it livelocks.

Thank you very much for this information.

> It absolutely needs follow-up patch from <[email protected]>
> "ext4: cleanup bh release code in ext4_ind_remove_space()"
> upstream commit 5e86bdda41534e17621d5a071b294943cae4376e.
>
> For more info about how to trigger that bug, see this earlier email
>
> https://marc.info/?l=linux-kernel&m=155419973129522&w=2
>
> For 3.16 kernels you may need to set CONFIG_EXT4_USE_FOR_EXT23=y
> so that ext4 code handles ext3 file systems.

As I want to release the update right now, I'll defer both of these.

Ben.

--
Ben Hutchings
When in doubt, use brute force. - Ken Thompson



Attachments:
signature.asc (849.00 B)
This is a digitally signed message part