On Thu, May 08, 2008 at 01:03:09PM +1000, Neil Brown wrote:
> On Tuesday May 6, bfields@xxxxxxxxxxxx wrote:
> > On Tue, May 06, 2008 at 10:35:46AM +1000, Neil Brown wrote:
> > >
> > > To fix the current bug properly, reconnect_path still needs to bypass
> > > normal permission checks even when subtree_check is in effect, so it
> > > can be sure of getting read permission on the parent directory.
> >
> > OK, but why not just forget the subtree_check case? It would be just
> > another item on the "reasons not to use subtree_check" list.
>
> I guess so.
>
> >
> > If a fix for the subtree checking case were easy (or if someone else had
> > the time to do a very careful job of it), then fine, but maybe we should
> > just fix the easy case and leave the subtree checking as is for now.
>
> So is this the proposed fix? A bit ugly, but I guess it's OK.
Something like that, yep. (Frank, can you confirm that this does the
job for you?)
It'd be nice if we could find a way to incorporate a little cleanup at
the same time, but I'm not sure exactly what to suggest.
--b.
>
> NeilBrown
>
> Signed-off-by: Neil Brown <neilb@xxxxxxx>
>
> ### Diffstat output
> ./fs/nfsd/nfsfh.c | 29 ++++++++++++++++++++++++++---
> 1 file changed, 26 insertions(+), 3 deletions(-)
>
> diff .prev/fs/nfsd/nfsfh.c ./fs/nfsd/nfsfh.c
> --- .prev/fs/nfsd/nfsfh.c 2008-05-06 10:06:59.000000000 +1000
> +++ ./fs/nfsd/nfsfh.c 2008-05-08 13:01:06.000000000 +1000
> @@ -176,9 +176,24 @@ static __be32 nfsd_set_fh_dentry(struct
> if (IS_ERR(exp))
> return nfserrno(PTR_ERR(exp));
>
> - error = nfsd_setuser_and_check_port(rqstp, exp);
> - if (error)
> - goto out;
> + if (exp->ex_flags & NFSEXP_NOSUBTREECHECK) {
> + /* Elevate privileges so that the lack of 'r' or 'x'
> + * permission on some parent directory will
> + * not stop exportfs_decode_fh from being able
> + * to reconnect a directory into the dentry cache.
> + * The same problem can affect "SUBTREECHECK" exports,
> + * but as nfsd_acceptable depends on correct
> + * access control settings being in effect, we cannot
> + * fix that case easily - so though.
> + */
> + current->cap_effective =
> + cap_raise_nfsd_set(current->cap_effective,
> + current->cap_permitted);
> + } else {
> + error = nfsd_setuser_and_check_port(rqstp, exp);
> + if (error)
> + goto out;
> + }
>
> /*
> * Look up the dentry using the NFS file handle.
> @@ -215,6 +230,14 @@ static __be32 nfsd_set_fh_dentry(struct
> goto out;
> }
>
> + if (exp->ex_flags & NFSEXP_NOSUBTREECHECK) {
> + error = nfsd_setuser_and_check_port(rqstp, exp);
> + if (error) {
> + dput(dentry);
> + goto out;
> + }
> + }
> +
> if (S_ISDIR(dentry->d_inode->i_mode) &&
> (dentry->d_flags & DCACHE_DISCONNECTED)) {
> printk("nfsd: find_fh_dentry returned a DISCONNECTED directory: %s/%s\n",
--
To unsubscribe from this list: send the line "unsubscribe linux-nfs" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
