Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1471141 > unrolled thread

Re: [PATCH 3.10 099/180] fix d_walk()/non-delayed __d_free() race

Back to article view | Back to linux.kernel

This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by below is the oldest one visible, not the original post.

Contents

  Re: [PATCH 3.10 099/180] fix d_walk()/non-delayed __d_free() race Willy Tarreau <w@1wt.eu> - 2016-08-27 11:40 +0200
    Re: [PATCH 3.10 099/180] fix d_walk()/non-delayed __d_free() race Ben Hutchings <ben@decadent.org.uk> - 2016-08-27 13:40 +0200
      Re: [PATCH 3.10 099/180] fix d_walk()/non-delayed __d_free() race Willy Tarreau <w@1wt.eu> - 2016-08-27 23:20 +0200

#1471141 — Re: [PATCH 3.10 099/180] fix d_walk()/non-delayed __d_free() race

Greg, Jiri,

I checked Jari's explanation below and found that v3.14.77 and v3.12.62
are missing the same fix as 3.10. In fact Al's original commit 3d56c25
("fix d_walk()/non-delayed __d_free() race") used to mention to check 
this __d_materialise_dentry() function in the Cc: stable line, but this
got lost during the backports.

Normally all of our 3 kernels need to apply the following patch that
Ben correctly put in 3.16 and 3.2. I'm fixing the backport in 3.10.103
right now.

Cheers,
Willy

diff --git a/fs/dcache.c b/fs/dcache.c
index 2a808fb..2d0b9d2 100644
--- a/fs/dcache.c
+++ b/fs/dcache.c
@@ -2401,6 +2401,7 @@ static void __d_materialise_dentry(struct dentry *dentry, struct dentry *anon)
 	switch_names(dentry, anon);
 	swap(dentry->d_name.hash, anon->d_name.hash);
 
+	dentry->d_flags |= DCACHE_RCUACCESS;
 	dentry->d_parent = dentry;
 	list_del_init(&dentry->d_child);
 	anon->d_parent = dparent;


On Mon, Aug 22, 2016 at 04:56:57PM +0300, Jari Ruusu wrote:
> This patch for 3.10 branch appears to be missing one important
> 
> +       dentry->d_flags |= DCACHE_RCUACCESS;
> 
> in fs/dcache.c __d_materialise_dentry() function. When Ben Hutchings
> backported Al Viro's original fix to stable branches that he maintains,
> he added that one additional line to both 3.2 and 3.16 branches. Please
> consider including that additional one line fix for 3.10 stable branch
> also.
> 
> 
> Ben Hutchings said this on his 3.2.82-rc1 patch:
> [bwh: Backported to 3.2:
>  - Adjust context
>  - Also set the flag in __d_materialise_dentry())]
> 
> http://marc.info/?l=linux-kernel&m=147117565612275&w=2
> 
> 
> Ben Hutchings said this on his 3.16.37-rc1 patch:
> [bwh: Backported to 3.16:
>  - Adjust context
>  - Also set the flag in __d_materialise_dentry())]
> 
> http://marc.info/?l=linux-kernel&m=147117433412006&w=2
> 
> 
> Also mentioned by Sasha Levin on 3.18 and 4.1 commits:
> Cc: stable@vger.kernel.org # v3.2+ (and watch out for __d_materialise_dentry())
> 
> http://marc.info/?l=linux-stable-commits&m=146648034410827&w=2
> http://marc.info/?l=linux-stable-commits&m=146647471009771&w=2
> 
> -- 
> Jari Ruusu  4096R/8132F189 12D6 4C3A DCDA 0AA4 27BD  ACDF F073 3C80 8132 F189

[toc] | [next] | [standalone]

#1471152

[Multipart message — attachments visible in raw view] — view raw

On Sat, 2016-08-27 at 11:31 +0200, Willy Tarreau wrote:
> Greg, Jiri,
> 
> I checked Jari's explanation below and found that v3.14.77 and v3.12.62
> are missing the same fix as 3.10. In fact Al's original commit 3d56c25
> ("fix d_walk()/non-delayed __d_free() race") used to mention to check 
> this __d_materialise_dentry() function in the Cc: stable line, but this
> got lost during the backports.
> 
> Normally all of our 3 kernels need to apply the following patch that
> Ben correctly put in 3.16 and 3.2. I'm fixing the backport in 3.10.103
> right now.

I never did get positive confirmation that this is the right change in
__d_materialise_dentry().  Al, could you please comment?

Ben.

> > Cheers,
> Willy
> 
> diff --git a/fs/dcache.c b/fs/dcache.c
> index 2a808fb..2d0b9d2 100644
> --- a/fs/dcache.c
> +++ b/fs/dcache.c
> @@ -2401,6 +2401,7 @@ static void __d_materialise_dentry(struct dentry *dentry, struct dentry *anon)
> >  	switch_names(dentry, anon);
> >  	swap(dentry->d_name.hash, anon->d_name.hash);
>  
> > +	dentry->d_flags |= DCACHE_RCUACCESS;
> >  	dentry->d_parent = dentry;
> >  	list_del_init(&dentry->d_child);
> >  	anon->d_parent = dparent;
> 
> 
> On Mon, Aug 22, 2016 at 04:56:57PM +0300, Jari Ruusu wrote:
> > 
> > This patch for 3.10 branch appears to be missing one important
> > 
> > +       dentry->d_flags |= DCACHE_RCUACCESS;
> > 
> > in fs/dcache.c __d_materialise_dentry() function. When Ben Hutchings
> > backported Al Viro's original fix to stable branches that he maintains,
> > he added that one additional line to both 3.2 and 3.16 branches. Please
> > consider including that additional one line fix for 3.10 stable branch
> > also.
> > 
> > 
> > Ben Hutchings said this on his 3.2.82-rc1 patch:
> > [bwh: Backported to 3.2:
> >  - Adjust context
> >  - Also set the flag in __d_materialise_dentry())]
> > 
> > http://marc.info/?l=linux-kernel&m=147117565612275&w=2
> > 
> > 
> > Ben Hutchings said this on his 3.16.37-rc1 patch:
> > [bwh: Backported to 3.16:
> >  - Adjust context
> >  - Also set the flag in __d_materialise_dentry())]
> > 
> > http://marc.info/?l=linux-kernel&m=147117433412006&w=2
> > 
> > 
> > Also mentioned by Sasha Levin on 3.18 and 4.1 commits:
> > > > Cc: stable@vger.kernel.org # v3.2+ (and watch out for __d_materialise_dentry())
> > 
> > http://marc.info/?l=linux-stable-commits&m=146648034410827&w=2
> > http://marc.info/?l=linux-stable-commits&m=146647471009771&w=2
> > 
> > -- 
> > Jari Ruusu  4096R/8132F189 12D6 4C3A DCDA 0AA4 27BD  ACDF F073 3C80 8132 F189
-- 
Ben Hutchings
[W]e found...that it wasn't as easy to get programs right as we had
thought.
... I realized that a large part of my life from then on was going to
be spent
in finding mistakes in my own programs. - Maurice Wilkes, 1949

[toc] | [prev] | [next] | [standalone]

#1471250

On Sat, Aug 27, 2016 at 12:38:38PM +0100, Ben Hutchings wrote:
> On Sat, 2016-08-27 at 11:31 +0200, Willy Tarreau wrote:
> > Greg, Jiri,
> > 
> > I checked Jari's explanation below and found that v3.14.77 and v3.12.62
> > are missing the same fix as 3.10. In fact Al's original commit 3d56c25
> > ("fix d_walk()/non-delayed __d_free() race") used to mention to check 
> > this __d_materialise_dentry() function in the Cc: stable line, but this
> > got lost during the backports.
> > 
> > Normally all of our 3 kernels need to apply the following patch that
> > Ben correctly put in 3.16 and 3.2. I'm fixing the backport in 3.10.103
> > right now.
> 
> I never did get positive confirmation that this is the right change in
> __d_materialise_dentry().  Al, could you please comment?

Well in my experience Al checks our reviews and steps in when there's
a mistake. Also your patch seems to reproduce the fix for the code
that was later killed by commit 63cf427 ("kill __d_materialise_dentry()")
which factors it out into __d_move() so I'm inclined to think that what
you did makes sense.

Cheers,
Willy

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web