All of lore.kernel.org
 help / color / mirror / Atom feed
From: Jeff Layton <jlayton@redhat.com>
To: ebiederm@xmission.com (Eric W. Biederman)
Cc: Jamie Lokier <jamie@shareable.org>,
	linux-kernel@vger.kernel.org, linux-fsdevel@vger.kernel.org,
	linux-nfs@vger.kernel.org, adobriyan@gmail.com,
	viro@ZenIV.linux.org.uk
Subject: Re: [PATCH] proc: revalidate dentry returned by proc_pid_follow_link
Date: Mon, 9 Nov 2009 08:11:31 -0500	[thread overview]
Message-ID: <20091109081131.7ebfde76@tlielax.poochiereds.net> (raw)
In-Reply-To: <m1hbt49xxs.fsf@fess.ebiederm.org>

On Sun, 08 Nov 2009 19:30:55 -0800
ebiederm@xmission.com (Eric W. Biederman) wrote:

> Jeff Layton <jlayton@redhat.com> writes:
> 
> >> Hmm.  Looking at the code I get the impression that a file bind mount
> >> will have exactly the same problem.
> >> 
> >> Can you confirm.
> >> 
> >> If file bind mounts also have this problem a bugfix to to just
> >> proc seems questionable.
> >> 
> >
> > I'm not sure I understand what you mean by "file bind mount". Is that
> > something like mounting with "-o loop" ?
> 
> # cd /tmp
> # echo foo > foo
> # echo bar > bar
> # mount --bind foo bar
> # cat bar
> foo
> #
> 
> > I'm not at all opposed to fixing this in a more broad fashion, but as
> > best I can tell, the only place that LAST_BIND is used is in procfs.
> 
> proc does appear to be the only user of LAST_BIND.  With a file bind
> mount we can get to the same ok: label without a revalidate.  The
> difference is that we came from __follow_mount instead of follow_link.
> 
> At least that is how I read the code.
> 

Thanks.

Yes, you're correct. That scenario does seem to have a similar problem.
I'm not quite sure yet how to fix it there yet.

It's easy enough to force a d_revalidate at a higher level. The tricky
bit is what to do when that returns 0 or an error. When I spoke to Al
about this, he suggested returning -ESTALE from the follow_link routine
if that occurs. That would force LOOKUP_REVAL to get set and cause the
whole chain of dentries to be revalidated back up to the root (if
necessary).

That's a little more difficult in the file bind mount case as I don't
see where it calls link_path_walk at all and hence can't really deal
with an ESTALE on a reval properly. I'll need to study this code some
more to see what the right fix is. If you (or anyone) has a suggestion,
I'd love to hear it.

-- 
Jeff Layton <jlayton@redhat.com>

WARNING: multiple messages have this Message-ID (diff)
From: Jeff Layton <jlayton-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
To: ebiederm-aS9lmoZGLiVWk0Htik3J/w@public.gmane.org (Eric W. Biederman)
Cc: Jamie Lokier <jamie-yetKDKU6eevNLxjTenLetw@public.gmane.org>,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
	linux-fsdevel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
	linux-nfs-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
	adobriyan-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org,
	viro-3bDd1+5oDREiFSDQTTA3OLVCufUGDwFn@public.gmane.org
Subject: Re: [PATCH] proc: revalidate dentry returned by proc_pid_follow_link
Date: Mon, 9 Nov 2009 08:11:31 -0500	[thread overview]
Message-ID: <20091109081131.7ebfde76@tlielax.poochiereds.net> (raw)
In-Reply-To: <m1hbt49xxs.fsf-+imSwln9KH6u2/kzUuoCbdi2O/JbrIOy@public.gmane.org>

On Sun, 08 Nov 2009 19:30:55 -0800
ebiederm-aS9lmoZGLiVWk0Htik3J/w@public.gmane.org (Eric W. Biederman) wrote:

> Jeff Layton <jlayton-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org> writes:
> 
> >> Hmm.  Looking at the code I get the impression that a file bind mount
> >> will have exactly the same problem.
> >> 
> >> Can you confirm.
> >> 
> >> If file bind mounts also have this problem a bugfix to to just
> >> proc seems questionable.
> >> 
> >
> > I'm not sure I understand what you mean by "file bind mount". Is that
> > something like mounting with "-o loop" ?
> 
> # cd /tmp
> # echo foo > foo
> # echo bar > bar
> # mount --bind foo bar
> # cat bar
> foo
> #
> 
> > I'm not at all opposed to fixing this in a more broad fashion, but as
> > best I can tell, the only place that LAST_BIND is used is in procfs.
> 
> proc does appear to be the only user of LAST_BIND.  With a file bind
> mount we can get to the same ok: label without a revalidate.  The
> difference is that we came from __follow_mount instead of follow_link.
> 
> At least that is how I read the code.
> 

Thanks.

Yes, you're correct. That scenario does seem to have a similar problem.
I'm not quite sure yet how to fix it there yet.

It's easy enough to force a d_revalidate at a higher level. The tricky
bit is what to do when that returns 0 or an error. When I spoke to Al
about this, he suggested returning -ESTALE from the follow_link routine
if that occurs. That would force LOOKUP_REVAL to get set and cause the
whole chain of dentries to be revalidated back up to the root (if
necessary).

That's a little more difficult in the file bind mount case as I don't
see where it calls link_path_walk at all and hence can't really deal
with an ESTALE on a reval properly. I'll need to study this code some
more to see what the right fix is. If you (or anyone) has a suggestion,
I'd love to hear it.

-- 
Jeff Layton <jlayton-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
--
To unsubscribe from this list: send the line "unsubscribe linux-nfs" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

WARNING: multiple messages have this Message-ID (diff)
From: Jeff Layton <jlayton@redhat.com>
To: ebiederm@xmission.com (Eric W. Biederman)
Cc: Jamie Lokier <jamie@shareable.org>,
	linux-kernel@vger.kernel.org, linux-fsdevel@vger.kernel.org,
	linux-nfs@vger.kernel.org, adobriyan@gmail.com,
	viro@ZenIV.linux.org.uk
Subject: Re: [PATCH] proc: revalidate dentry returned by proc_pid_follow_link
Date: Mon, 9 Nov 2009 08:11:31 -0500	[thread overview]
Message-ID: <20091109081131.7ebfde76@tlielax.poochiereds.net> (raw)
In-Reply-To: <m1hbt49xxs.fsf-+imSwln9KH6u2/kzUuoCbdi2O/JbrIOy@public.gmane.org>

On Sun, 08 Nov 2009 19:30:55 -0800
ebiederm@xmission.com (Eric W. Biederman) wrote:

> Jeff Layton <jlayton@redhat.com> writes:
> 
> >> Hmm.  Looking at the code I get the impression that a file bind mount
> >> will have exactly the same problem.
> >> 
> >> Can you confirm.
> >> 
> >> If file bind mounts also have this problem a bugfix to to just
> >> proc seems questionable.
> >> 
> >
> > I'm not sure I understand what you mean by "file bind mount". Is that
> > something like mounting with "-o loop" ?
> 
> # cd /tmp
> # echo foo > foo
> # echo bar > bar
> # mount --bind foo bar
> # cat bar
> foo
> #
> 
> > I'm not at all opposed to fixing this in a more broad fashion, but as
> > best I can tell, the only place that LAST_BIND is used is in procfs.
> 
> proc does appear to be the only user of LAST_BIND.  With a file bind
> mount we can get to the same ok: label without a revalidate.  The
> difference is that we came from __follow_mount instead of follow_link.
> 
> At least that is how I read the code.
> 

Thanks.

Yes, you're correct. That scenario does seem to have a similar problem.
I'm not quite sure yet how to fix it there yet.

It's easy enough to force a d_revalidate at a higher level. The tricky
bit is what to do when that returns 0 or an error. When I spoke to Al
about this, he suggested returning -ESTALE from the follow_link routine
if that occurs. That would force LOOKUP_REVAL to get set and cause the
whole chain of dentries to be revalidated back up to the root (if
necessary).

That's a little more difficult in the file bind mount case as I don't
see where it calls link_path_walk at all and hence can't really deal
with an ESTALE on a reval properly. I'll need to study this code some
more to see what the right fix is. If you (or anyone) has a suggestion,
I'd love to hear it.

-- 
Jeff Layton <jlayton@redhat.com>

  reply	other threads:[~2009-11-09 13:11 UTC|newest]

Thread overview: 15+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2009-11-06 13:19 [PATCH] proc: revalidate dentry returned by proc_pid_follow_link Jeff Layton
2009-11-06 13:19 ` Jeff Layton
2009-11-06 20:36 ` Jamie Lokier
2009-11-06 21:06   ` Jeff Layton
2009-11-06 21:06     ` Jeff Layton
2009-11-08 10:15     ` Eric W. Biederman
2009-11-09  1:12       ` Jeff Layton
2009-11-09  1:12         ` Jeff Layton
2009-11-09  1:12         ` Jeff Layton
2009-11-09  3:30         ` Eric W. Biederman
2009-11-09  3:30           ` Eric W. Biederman
2009-11-09  3:30           ` Eric W. Biederman
2009-11-09 13:11           ` Jeff Layton [this message]
2009-11-09 13:11             ` Jeff Layton
2009-11-09 13:11             ` Jeff Layton

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20091109081131.7ebfde76@tlielax.poochiereds.net \
    --to=jlayton@redhat.com \
    --cc=adobriyan@gmail.com \
    --cc=ebiederm@xmission.com \
    --cc=jamie@shareable.org \
    --cc=linux-fsdevel@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-nfs@vger.kernel.org \
    --cc=viro@ZenIV.linux.org.uk \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.