Re: [PATCH 3.10 099/180] fix d_walk()/non-delayed __d_free() race
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
Re: [PATCH 3.10 099/180] fix d_walk()/non-delayed __d_free() race
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
Re: [PATCH 3.10 099/180] fix d_walk()/non-delayed __d_free() race
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(>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=147117565612275=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=147117433412006=2
> >
> >
> > Also mentioned by Sasha Levin on 3.18 and 4.1 commits:
> > > > Cc: sta...@vger.kernel.org # v3.2+ (and watch out for
> > > > __d_materialise_dentry())
> >
> > http://marc.info/?l=linux-stable-commits=146648034410827=2
> > http://marc.info/?l=linux-stable-commits=146647471009771=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
signature.asc
Description: This is a digitally signed message part
Re: [PATCH 3.10 099/180] fix d_walk()/non-delayed __d_free() race
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(>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=147117565612275=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=147117433412006=2
> >
> >
> > Also mentioned by Sasha Levin on 3.18 and 4.1 commits:
> > > > Cc: sta...@vger.kernel.org # v3.2+ (and watch out for
> > > > __d_materialise_dentry())
> >
> > http://marc.info/?l=linux-stable-commits=146648034410827=2
> > http://marc.info/?l=linux-stable-commits=146647471009771=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
signature.asc
Description: This is a digitally signed message part
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(>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=147117565612275=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=147117433412006=2
>
>
> Also mentioned by Sasha Levin on 3.18 and 4.1 commits:
> Cc: sta...@vger.kernel.org # v3.2+ (and watch out for
> __d_materialise_dentry())
>
> http://marc.info/?l=linux-stable-commits=146648034410827=2
> http://marc.info/?l=linux-stable-commits=146647471009771=2
>
> --
> Jari Ruusu 4096R/8132F189 12D6 4C3A DCDA 0AA4 27BD ACDF F073 3C80 8132 F189
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(>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=147117565612275=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=147117433412006=2
>
>
> Also mentioned by Sasha Levin on 3.18 and 4.1 commits:
> Cc: sta...@vger.kernel.org # v3.2+ (and watch out for
> __d_materialise_dentry())
>
> http://marc.info/?l=linux-stable-commits=146648034410827=2
> http://marc.info/?l=linux-stable-commits=146647471009771=2
>
> --
> Jari Ruusu 4096R/8132F189 12D6 4C3A DCDA 0AA4 27BD ACDF F073 3C80 8132 F189
Re: [PATCH 3.10 099/180] fix d_walk()/non-delayed __d_free() race
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. (...) Many thanks Jari, I'll use Ben's backport then. Cheers, Willy
Re: [PATCH 3.10 099/180] fix d_walk()/non-delayed __d_free() race
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. (...) Many thanks Jari, I'll use Ben's backport then. Cheers, Willy
Re: [PATCH 3.10 099/180] fix d_walk()/non-delayed __d_free() race
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=147117565612275=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=147117433412006=2 Also mentioned by Sasha Levin on 3.18 and 4.1 commits: Cc: sta...@vger.kernel.org # v3.2+ (and watch out for __d_materialise_dentry()) http://marc.info/?l=linux-stable-commits=146648034410827=2 http://marc.info/?l=linux-stable-commits=146647471009771=2 -- Jari Ruusu 4096R/8132F189 12D6 4C3A DCDA 0AA4 27BD ACDF F073 3C80 8132 F189
Re: [PATCH 3.10 099/180] fix d_walk()/non-delayed __d_free() race
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=147117565612275=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=147117433412006=2 Also mentioned by Sasha Levin on 3.18 and 4.1 commits: Cc: sta...@vger.kernel.org # v3.2+ (and watch out for __d_materialise_dentry()) http://marc.info/?l=linux-stable-commits=146648034410827=2 http://marc.info/?l=linux-stable-commits=146647471009771=2 -- Jari Ruusu 4096R/8132F189 12D6 4C3A DCDA 0AA4 27BD ACDF F073 3C80 8132 F189
