Linux NFS development
 help / color / mirror / Atom feed
From: Mike Snitzer <snitzer@kernel.org>
To: NeilBrown <neilb@suse.de>
Cc: cel@kernel.org, linux-nfs@vger.kernel.org
Subject: Re: [RFC PATCH 1/6] NFSD: Handle @rqstp == NULL in check_nfsd_access()
Date: Wed, 28 Aug 2024 20:27:00 -0400	[thread overview]
Message-ID: <Zs_AVOZ3n8CXu9NA@kernel.org> (raw)
In-Reply-To: <172487914501.4433.5035812857573545333@noble.neil.brown.name>

On Thu, Aug 29, 2024 at 07:05:45AM +1000, NeilBrown wrote:
> On Wed, 28 Aug 2024, Mike Snitzer wrote:
> > 
> > So I honestly feel like Chuck's latest revision is perfectly fine.
> > I disagree that "The behavior for LOCALIO is therefore the same as
> > the AUTH_UNIX check below." is inaccurate.  The precondition from the
> > client (used to establish localio and cause rqstp to be NULL in
> > check_nfsd_access) is effectively comparable no?
> > 
> 
> I don't think the correctness of the code is at all related to
> AUTH_UNIX.
> Suppose we did add support for krb5 somehow - would we really need a
> different test?  I think not.
> 
> I think that the reason the code is correct and safe is that we trust
> the client.  We *know* the client will only present an filehandle which
> it has received over the wire and which it verified - with the
> accompanying credential - it was allowed to access.
> 
> Maybe that is what you meant by "The precondition from the client".  I
> agree that does give us sufficient safety.  I don't think AUTH_UNIX is
> relevant.
> 
> /*
>  * If rqstp is NULL, this is a LOCALIO request which will only ever use
>  * filehandle/credential pair for which access has been affirmed (by
>  * ACCESS or OPEN NFS requests) over the wire.  So there is no need for
>  * further checks here.
>  */

Makes sense, and thanks!

> (It wasn't quite this clear to me when I wrote previously - but I always
>  seems to think more clearly in the mornings!)

I haven't been sleeping enough.. tonight, tonight I will!!! ;)

Mike

  reply	other threads:[~2024-08-29  0:27 UTC|newest]

Thread overview: 19+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-08-28  0:44 [RFC PATCH 0/6] Split up refactoring of fh_verify() cel
2024-08-28  0:44 ` [RFC PATCH 1/6] NFSD: Handle @rqstp == NULL in check_nfsd_access() cel
2024-08-28  1:12   ` NeilBrown
2024-08-28  3:00     ` Mike Snitzer
2024-08-28  6:30       ` NeilBrown
2024-08-28 13:26         ` Chuck Lever III
2024-08-28 13:45         ` Mike Snitzer
2024-08-28 21:05           ` NeilBrown
2024-08-29  0:27             ` Mike Snitzer [this message]
2024-08-28  0:44 ` [RFC PATCH 2/6] NFSD: Refactor nfsd_setuser_and_check_port() cel
2024-08-28  0:44 ` [RFC PATCH 3/6] NFSD: Avoid using rqstp->rq_vers in nfsd_set_fh_dentry() cel
2024-08-28  5:02   ` NeilBrown
2024-08-28 13:45     ` Chuck Lever
2024-08-28 14:19       ` Mike Snitzer
2024-08-28  0:44 ` [RFC PATCH 4/6] NFSD: Short-circuit fh_verify tracepoints for LOCALIO cel
2024-08-28  0:44 ` [RFC PATCH 5/6] nfsd: factor out __fh_verify to allow NULL rqstp to be passed cel
2024-08-28  0:44 ` [RFC PATCH 6/6] nfsd: add nfsd_file_acquire_local() cel
2024-08-28  1:08 ` [RFC PATCH 0/6] Split up refactoring of fh_verify() Mike Snitzer
2024-08-28  2:32   ` Mike Snitzer

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=Zs_AVOZ3n8CXu9NA@kernel.org \
    --to=snitzer@kernel.org \
    --cc=cel@kernel.org \
    --cc=linux-nfs@vger.kernel.org \
    --cc=neilb@suse.de \
    /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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox