This fixes a wrong check for same cr_principal in same_creds

Introduced by 8fbba96e "nfsd4: stricter
cred comparison for setclientid/exchange_id".

Cc: stable@vger.kernel.org
Signed-off-by: NVivek Trivedi <vtrivedi018@gmail.com>
Signed-off-by: NNamjae Jeon <linkinjeon@gmail.com>
Signed-off-by: NJ. Bruce Fields <bfields@redhat.com>

We don't need to keep openowners around in the >=4.1 case, because they
aren't needed to handle CLOSE replays any more (that's a problem for
sessions).  And doing so causes unexpected failures on a subsequent
destroy_clientid to fail.

We probably also need something comparable for lock owners on last
unlock.
Signed-off-by: NJ. Bruce Fields <bfields@redhat.com>

Note we can simplify the error handling a little by doing the truncate
earlier.
Signed-off-by: NJ. Bruce Fields <bfields@redhat.com>

Share a little common logic.  And note the comments here are a little
out of date (e.g. we don't always create new state in the "new" case any
more.)
Signed-off-by: NJ. Bruce Fields <bfields@redhat.com>

According to RFC 5661, the TEST_STATEID operation is not allowed to
return NFS4ERR_STALE_STATEID.  In addition, RFC 5661 says:

15.1.16.5.  NFS4ERR_STALE_STATEID (Error Code 10023)

   A stateid generated by an earlier server instance was used.  This
   error is moot in NFSv4.1 because all operations that take a stateid
   MUST be preceded by the SEQUENCE operation, and the earlier server
   instance is detected by the session infrastructure that supports
   SEQUENCE.

I triggered NFS4ERR_STALE_STATEID while testing the Linux client's
NOGRACE recovery.  Bruce suggested an additional test that could be
useful to client developers.

Lastly, RFC 5661, section 18.48.3 has this:

 o  Special stateids are always considered invalid (they result in the
    error code NFS4ERR_BAD_STATEID).

An explicit check is made for those state IDs to avoid printk noise.
Signed-off-by: NChuck Lever <chuck.lever@oracle.com>
Signed-off-by: NJ. Bruce Fields <bfields@redhat.com>

Initiate a CB probe when a new connection with the correct direction is added
to a session (IFF backchannel is marked as down).  Without this a
BIND_CONN_TO_SESSION has no effect on the internal backchannel state, which
causes the server to reply to every SEQUENCE op with the
SEQ4_STATUS_CB_PATH_DOWN flag set until DESTROY_SESSION.
Signed-off-by: NWeston Andros Adamson <dros@netapp.com>
Signed-off-by: NJ. Bruce Fields <bfields@redhat.com>

Most frequent symptom was a BUG triggering in expire_client, with the
server locking up shortly thereafter.

Introduced by 508dc6e1 "nfsd41:
free_session/free_client must be called under the client_lock".

Cc: stable@kernel.org
Cc: Benny Halevy <bhalevy@tonian.com>
Signed-off-by: NJ. Bruce Fields <bfields@redhat.com>

Whoops: first, I reimplemented the already-existing has_resources
without noticing; second, I got the test backwards.  I did pick a better
name, though.  Combine the two....
Signed-off-by: NJ. Bruce Fields <bfields@redhat.com>

In the NFSv4.1 client-reboot case we're currently removing the client's
previous state in exchange_id.  That's wrong--we should be waiting till
the confirming create_session.
Signed-off-by: NJ. Bruce Fields <bfields@redhat.com>

The comment is redundant, and if we really want dprintk's here they'd
probably be better in the common (check-slot_seqid) code.
Signed-off-by: NJ. Bruce Fields <bfields@redhat.com>

The client should ignore the returned sequence_id in the case where the
CONFIRMED flag is set on an exchange_id reply--and in the unconfirmed
case "1" is always the right response.  So it shouldn't actually matter
what we return here.

We could continue returning 1 just to catch clients ignoring the spec
here, but I'd rather be generous.  Other things equal, returning the
existing sequence_id seems more informative.
Signed-off-by: NJ. Bruce Fields <bfields@redhat.com>

Otherwise nfsd4_set_ex_flags writes over the return flags.
Reported-by: NBryan Schumaker <bjschuma@netapp.com>
Signed-off-by: NJ. Bruce Fields <bfields@redhat.com>

This can't happen:
	- cl_time is zeroed only by unhash_client_locked, which is only
	  ever called under both the state lock and the client lock.
	- every caller of renew_client() should have looked up a
	  (non-expired) client and then called renew_client() all
	  without dropping the state lock.
	- the only other caller of renew_client_locked() is
	  release_session_client(), which first checks under the
	  client_lock that the cl_time is nonzero.

So make it clear that this is a bug, not something we handle.  I can't
quite bring myself to make this a BUG(), though, as there are a lot of
renew_client() callers, and returning here is probably safer than a
BUG().

We'll consider making it a BUG() after some more cleanup.
Signed-off-by: NJ. Bruce Fields <bfields@redhat.com>

The cases here divide into two main categories:

	- if there's an uncomfirmed record with a matching verifier,
	  then this is a "normal", succesful case: we're either creating
	  a new client, or updating an existing one.
	- otherwise, this is a weird case: a replay, or a server reboot.

Reordering to reflect that makes the code a bit more concise and the
logic a lot easier to understand.
Signed-off-by: NJ. Bruce Fields <bfields@redhat.com>

Signed-off-by: NJ. Bruce Fields <bfields@redhat.com>

Note CLID_INUSE is for the case where two clients are trying to use the
same client-provided long-form client identifiers.  But what we're
looking at here is the server-returned shorthand client id--if those
clash there's a bug somewhere.

Fix the error return, pull the check out into common code, and do the
check unconditionally in all cases.
Signed-off-by: NJ. Bruce Fields <bfields@redhat.com>

New clients are created only by nfsd4_setclientid(), which always gives
any new client a unique clientid.  The only exception is in the
"callback update" case, in which case it may create an unconfirmed
client with the same clientid as a confirmed client.  In that case it
also checks that the confirmed client has the same credential.

Therefore, it is pointless for setclientid_confirm to check whether a
confirmed and unconfirmed client with the same clientid have matching
credentials--they're guaranteed to.

Instead, it should be checking whether the credential on the
setclientid_confirm matches either of those.  Otherwise, it could be
anyone sending the setclientid_confirm.  Granted, I can't see why anyone
would, but still it's probalby safer to check.
Signed-off-by: NJ. Bruce Fields <bfields@redhat.com>

Boy, is this simpler.
Signed-off-by: NJ. Bruce Fields <bfields@redhat.com>

Signed-off-by: NJ. Bruce Fields <bfields@redhat.com>

The code here is mostly the same.
Signed-off-by: NJ. Bruce Fields <bfields@redhat.com>

Be a little more concise.
Signed-off-by: NJ. Bruce Fields <bfields@redhat.com>

Signed-off-by: NJ. Bruce Fields <bfields@redhat.com>

Move the rq_flavor into struct svc_cred, and use it in setclientid and
exchange_id comparisons as well.
Signed-off-by: NJ. Bruce Fields <bfields@redhat.com>

The typical setclientid or exchange_id will probably be performed with a
credential that maps to either root or nobody, so comparing just uid's
is unlikely to be useful.  So, use everything else we can get our hands
on.
Signed-off-by: NJ. Bruce Fields <bfields@redhat.com>

Instead of keeping the principal name associated with a request in a
structure that's private to auth_gss and using an accessor function,
move it to svc_cred.
Signed-off-by: NJ. Bruce Fields <bfields@redhat.com>

RFC 5661 actually says we should allow an exchange_id to remove a
matching client, even if the exchange_id comes from a different
principal, *if* the victim client lacks any state.
Signed-off-by: NJ. Bruce Fields <bfields@redhat.com>

Minor cleanup: it's simpler to have separate code paths for the update
and non-update cases.
Signed-off-by: NJ. Bruce Fields <bfields@redhat.com>

Make these comments a bit more concise and uniform.
Signed-off-by: NJ. Bruce Fields <bfields@redhat.com>

Signed-off-by: NJ. Bruce Fields <bfields@redhat.com>

Following rfc 5661 section 2.4.1, we can permit a 4.1 client to remove
an established 4.0 client's state.

(But we don't allow updates.)
Signed-off-by: NJ. Bruce Fields <bfields@redhat.com>

We mustn't allow a client to destroy another client with established
state unless it has the right credential.

And some minor cleanup.

(Note: our comparison of credentials is actually pretty bogus currently;
that will need to be fixed in another patch.)
Signed-off-by: NJ. Bruce Fields <bfields@redhat.com>

There's no point to the dprintk here as the main proc_compound loop
already does this.
Signed-off-by: NJ. Bruce Fields <bfields@redhat.com>

We just verified above that these two verifiers are already the same.
Signed-off-by: NJ. Bruce Fields <bfields@redhat.com>

Handle the st_deny_bmap in a similar fashion to the st_access_bmap. Add
accessor functions and use those instead of bare bitops.
Signed-off-by: NJeff Layton <jlayton@redhat.com>
Signed-off-by: NJ. Bruce Fields <bfields@redhat.com>

Currently, we do this for the most part with "bare" bitops, but
eventually we'll need to expand the share mode code to handle access
and deny modes on other nodes.

In order to facilitate that code in the future, move to some generic
accessor functions. For now, these are mostly static inlines, but
eventually we'll want to move these to "real" functions that are
able to handle multi-node configurations or have a way to "swap in"
new operations to be done in lieu of or in conjunction with these
atomic bitops.
Signed-off-by: NJeff Layton <jlayton@redhat.com>
Signed-off-by: NJ. Bruce Fields <bfields@redhat.com>

All of the callers treat the return that way already.
Signed-off-by: NJeff Layton <jlayton@redhat.com>
Signed-off-by: NJ. Bruce Fields <bfields@redhat.com>

These functions are identical. Also, rename them to bmap_to_share_mode
to better reflect what they do, and have them just return the result
instead of passing in a pointer to the storage location.
Signed-off-by: NJeff Layton <jlayton@redhat.com>
Signed-off-by: NJ. Bruce Fields <bfields@redhat.com>

According to RFC 3530bis, the only items SETCLIENTID_CONFIRM processing
should be concerned with is the clientid, clientid verifier, and
principal.  The client's IP address is not supposed to be interesting.

And, NFS4ERR_CLID_INUSE is meant only for principal mismatches.

I triggered this logic with a prototype UCS client -- one that
uses the same nfs_client_id4 string for all servers.  The client
mounted our server via its IPv4, then via its IPv6 address.
Signed-off-by: NChuck Lever <chuck.lever@oracle.com>
Signed-off-by: NJ. Bruce Fields <bfields@redhat.com>

In each of these cases there's a simple unambiguous correct choice, and
no actual bug.
Signed-off-by: NJ. Bruce Fields <bfields@redhat.com>

Signed-off-by: NJ. Bruce Fields <bfields@redhat.com>