2012-05-02 11:45:43

by Suresh Jayaraman

[permalink] [raw]
Subject: Re: [PATCH] cifs - check S_AUTOMOUNT in revalidate

On 04/28/2012 09:03 AM, Steve French wrote:
> The fix makes sense, but it is fairly recent and I haven't had a
> chance to try it,
> so unless a new release is imminent, I would prefer to put in the next merge
> request (I have at least one more fix likely as well) next week.

We need this fix for -stable as well, right? Please include

Cc: [email protected]


Suresh


2012-05-02 17:51:08

by Jeff Layton

[permalink] [raw]
Subject: Re: [PATCH] cifs - check S_AUTOMOUNT in revalidate

On Wed, 02 May 2012 17:15:29 +0530
Suresh Jayaraman <[email protected]> wrote:

> On 04/28/2012 09:03 AM, Steve French wrote:
> > The fix makes sense, but it is fairly recent and I haven't had a
> > chance to try it,
> > so unless a new release is imminent, I would prefer to put in the next merge
> > request (I have at least one more fix likely as well) next week.
>
> We need this fix for -stable as well, right? Please include
>
> Cc: [email protected]
>
>
> Suresh
>
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-cifs" in
> the body of a message to [email protected]
> More majordomo info at http://vger.kernel.org/majordomo-info.html

The stable-kernel-rules.txt file says this:

- It must fix a real bug that bothers people (not a, "This could be a
problem..." type thing).

...and...

- No "theoretical race condition" issues, unless an explanation of how the
race can be exploited is also provided.

...and this seems to clearly violate both of those rules. I'd say we
just go with putting it in 3.4 for now, and keep it in mind if someone
comes back later and says that it's needed.

--
Jeff Layton <[email protected]>

2012-05-03 01:06:20

by Ian Kent

[permalink] [raw]
Subject: Re: [PATCH] cifs - check S_AUTOMOUNT in revalidate

On Wed, 2012-05-02 at 13:50 -0400, Jeff Layton wrote:
> On Wed, 02 May 2012 17:15:29 +0530
> Suresh Jayaraman <[email protected]> wrote:
>
> > On 04/28/2012 09:03 AM, Steve French wrote:
> > > The fix makes sense, but it is fairly recent and I haven't had a
> > > chance to try it,
> > > so unless a new release is imminent, I would prefer to put in the next merge
> > > request (I have at least one more fix likely as well) next week.
> >
> > We need this fix for -stable as well, right? Please include
> >
> > Cc: [email protected]
> >
> >
> > Suresh
> >
> >
> > --
> > To unsubscribe from this list: send the line "unsubscribe linux-cifs" in
> > the body of a message to [email protected]
> > More majordomo info at http://vger.kernel.org/majordomo-info.html
>
> The stable-kernel-rules.txt file says this:
>
> - It must fix a real bug that bothers people (not a, "This could be a
> problem..." type thing).
>
> ...and...
>
> - No "theoretical race condition" issues, unless an explanation of how the
> race can be exploited is also provided.
>
> ...and this seems to clearly violate both of those rules. I'd say we
> just go with putting it in 3.4 for now, and keep it in mind if someone
> comes back later and says that it's needed.
>

I have to agree with Jeff.

I just happened to notice it when trying to work out why DFS automounts
weren't happening.

I eventually found that the kernel in use didn't have the d_invalidate()
patch, then realized the potential problem. It would require fairly
heavy demand to trigger so it isn't urgent, especially since no-one has
actually reported it.

Another reason I should have sent this to Steve, not Linus, sorry for
the noise Linus.

Ian

2012-05-03 05:49:57

by Suresh Jayaraman

[permalink] [raw]
Subject: Re: [PATCH] cifs - check S_AUTOMOUNT in revalidate

On 05/02/2012 11:20 PM, Jeff Layton wrote:
> On Wed, 02 May 2012 17:15:29 +0530
> Suresh Jayaraman <[email protected]> wrote:
>
>> On 04/28/2012 09:03 AM, Steve French wrote:
>>> The fix makes sense, but it is fairly recent and I haven't had a
>>> chance to try it,
>>> so unless a new release is imminent, I would prefer to put in the next merge
>>> request (I have at least one more fix likely as well) next week.
>>
>> We need this fix for -stable as well, right? Please include
>>
>> Cc: [email protected]
>
> The stable-kernel-rules.txt file says this:
>
> - It must fix a real bug that bothers people (not a, "This could be a
> problem..." type thing).
>

Doh, looks like I had assumed (wrongly) that this fixes the problem
reported here

http://marc.info/?l=linux-cifs&m=133460168320472&w=2

(mapping to a dfsroot and cd to a reflink returning "Object is remote"
or "Not a directory" error). I didn't have a setup to verify it too.

I notice that Steve has already merged this patch adding Cc to stable
and the Cc has to be removed before sending the pull request.


Suresh