Re: [PATCH] Adding the nfs4_secure_mounts bool

From: NeilBrown <neilb@suse.de>
To: "Myklebust, Trond" <Trond.Myklebust@netapp.com>
Cc: "J. Bruce Fields" <bfields@fieldses.org>,
	Charles Edward Lever <chuck.lever@oracle.com>,
	Steve Dickson <SteveD@redhat.com>,
	"Linux NFS Mailing List" <linux-nfs@vger.kernel.org>
Subject: Re: [PATCH] Adding the nfs4_secure_mounts bool
Date: Thu, 14 Nov 2013 12:05:33 +1100	[thread overview]
Message-ID: <20131114120533.1df06f5c@notabene.brown> (raw)
In-Reply-To: <1384306012.15992.9.camel@leira.trondhjem.org>

[-- Attachment #1: Type: text/plain, Size: 1769 bytes --]

On Wed, 13 Nov 2013 01:26:56 +0000 "Myklebust, Trond"
<Trond.Myklebust@netapp.com> wrote:

> On Wed, 2013-11-13 at 12:13 +1100, NeilBrown wrote:
> > On Wed, 13 Nov 2013 00:30:53 +0000 "Myklebust, Trond"
> > <Trond.Myklebust@netapp.com> wrote:
> > 
> > > On Wed, 2013-11-13 at 11:23 +1100, NeilBrown wrote:
> > > > But back to my problem:  Following Trond's suggestion I've come up with the
> > > > following patch.  Does it look right?
> > > > 
> > > > The "fd = -1" is just to stop us trying to close a non-open fd in an error
> > > > path.
> > > > 
> > > > The change from testing ->servicename to ->prog stops us from repeating the
> > > > failed DNS lookup on every request, not that the failure isn't fatal.
> > > > 
> > > > The last stanza makes sure we always reply to an upcall, with EINVAL if
> > > > nothing else seems appropriate.
> > > 
> > > Wouldn't EACCES be more appropriate as a default?
> > > 
> > 
> > Maybe.  And that is what you suggested before and I mis-remembered - sorry.
> > 
> > However EACCES is "Permission denied" which doesn't quite seem right to me.
> > It isn't really "you aren't allowed to do that", but "your question doesn't
> > make sense".
> > 
> > However I'm not fussed.  If you prefer EACCES, then I'll make it EACCES.
> 
> If you look at gss_pipe_downcall(), then you'll note that it treats
> EINVAL as a temporary error, and converts it to EAGAIN. That again
> causes call_refreshresult to retry the upcall 2 more times before
> failing with EACCES anyway...
> 

Yes, I see now, thanks.

I also see a 'BUG()' in there if the error code returned from user-space
isn't in the known list.  I suspect that should at most be a WARN, and
probably removed altogether.

Thanks,
NeilBrown

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 828 bytes --]