linux-toradex.git/fs/nfsd, branch v4.4-rc2

nfsd: fix race with open / open upgrade stateids

2015-11-10T14:29:45+00:00

We observed multiple open stateids on the server for files that
seemingly should have been closed.

nfsd4_process_open2() tests for the existence of a preexisting
stateid. If one is not found, the locks are dropped and a new
one is created. The problem is that init_open_stateid(), which
is also responsible for hashing the newly initialized stateid,
doesn't check to see if another open has raced in and created
a matching stateid. This fix is to enable init_open_stateid() to
return the matching stateid and have nfsd4_process_open2()
swap to that stateid and switch to the open upgrade path.
In testing this patch, coverage to the newly created
path indicates that the race was indeed happening.

Signed-off-by: Andrew Elble 
Reviewed-by: Jeff Layton 
Signed-off-by: J. Bruce Fields

nfsd: eliminate sending duplicate and repeated delegations

2015-11-10T14:29:44+00:00

We've observed the nfsd server in a state where there are
multiple delegations on the same nfs4_file for the same client.
The nfs client does attempt to DELEGRETURN these when they are presented to
it - but apparently under some (unknown) circumstances the client does not
manage to return all of them. This leads to the eventual
attempt to CB_RECALL more than one delegation with the same nfs
filehandle to the same client. The first recall will succeed, but the
next recall will fail with NFS4ERR_BADHANDLE. This leads to the server
having delegations on cl_revoked that the client has no way to FREE
or DELEGRETURN, with resulting inability to recover. The state manager
on the server will continually assert SEQ4_STATUS_RECALLABLE_STATE_REVOKED,
and the state manager on the client will be looping unable to satisfy
the server.

List discussion also reports a race between OPEN and DELEGRETURN that
will be avoided by only sending the delegation once to the
client. This is also logically in accordance with RFC5561 9.1.1 and 10.2.

So, let's:

1.) Not hand out duplicate delegations.
2.) Only send them to the client once.

RFC 5561:

9.1.1:
"Delegations and layouts, on the other hand, are not associated with a
specific owner but are associated with the client as a whole
(identified by a client ID)."

10.2:
"...the stateid for a delegation is associated with a client ID and may be
used on behalf of all the open-owners for the given client.  A
delegation is made to the client as a whole and not to any specific
process or thread of control within it."

Reported-by: Eric Meddaugh 
Cc: Trond Myklebust 
Cc: Olga Kornievskaia 
Signed-off-by: Andrew Elble 
Cc: stable@vger.kernel.org
Signed-off-by: J. Bruce Fields

nfsd: remove recurring workqueue job to clean DRC

2015-11-10T14:25:51+00:00

We have a shrinker, we clean out the cache when nfsd is shut down, and
prune the chains on each request. A recurring workqueue job seems like
unnecessary overhead. Just remove it.

Signed-off-by: Jeff Layton 
Signed-off-by: J. Bruce Fields

nfsd: ensure that seqid morphing operations are atomic wrt to copies

2015-10-23T19:57:33+00:00

Bruce points out that the increment of the seqid in stateids is not
serialized in any way, so it's possible for racing calls to bump it
twice and end up sending the same stateid. While we don't have any
reports of this problem it _is_ theoretically possible, and could lead
to spurious state recovery by the client.

In the current code, update_stateid is always followed by a memcpy of
that stateid, so we can combine the two operations. For better
atomicity, we add a spinlock to the nfs4_stid and hold that when bumping
the seqid and copying the stateid.

Signed-off-by: Jeff Layton 
Signed-off-by: J. Bruce Fields

nfsd: serialize layout stateid morphing operations

2015-10-23T19:57:32+00:00

In order to allow the client to make a sane determination of what
happened with racing LAYOUTGET/LAYOUTRETURN/CB_LAYOUTRECALL calls, we
must ensure that the seqids return accurately represent the order of
operations. The simplest way to do that is to ensure that operations on
a single stateid are serialized.

This patch adds a mutex to the layout stateid, and locks it when
checking the layout stateid's seqid. The mutex is held over the entire
operation and released after the seqid is bumped.

Note that in the case of CB_LAYOUTRECALL we must move the increment of
the seqid and setting into a new cb "prepare" operation. The lease
infrastructure will call the lm_break callback with a spinlock held, so
and we can't take the mutex in that codepath.

Cc: Christoph Hellwig 
Signed-off-by: Jeff Layton 
Signed-off-by: J. Bruce Fields

nfsd: improve client_has_state to check for unused openowners

2015-10-23T19:57:31+00:00

At least in the v4.0 case openowners can hang around for a while after
last close, but they shouldn't really block (for example), a new mount
with a different principal.

Signed-off-by: J. Bruce Fields

nfsd: fix clid_inuse on mount with security change

2015-10-23T19:57:30+00:00

In bakeathon testing Solaris client was getting CLID_INUSE error when
doing a krb5 mount soon after an auth_sys mount, or vice versa.

That's not really necessary since in this case the old client doesn't
have any state any more:

	http://tools.ietf.org/html/rfc7530#page-103

	"when the server gets a SETCLIENTID for a client ID that
	currently has no state, or it has state but the lease has
	expired, rather than returning NFS4ERR_CLID_INUSE, the server
	MUST allow the SETCLIENTID and confirm the new client ID if
	followed by the appropriate SETCLIENTID_CONFIRM."

This doesn't fix the problem completely since our client_has_state()
check counts openowners left around to handle close replays, which we
should probably just remove in this case.

Signed-off-by: J. Bruce Fields

nfsd: move include of state.h from trace.c to trace.h

2015-10-23T19:57:29+00:00

Any file which includes trace.h will need to include state.h, even if
they aren't using any state tracepoints. Ensure that we include any
headers that might be needed in trace.h instead of relying on the
*.c files to have the right ones.

Signed-off-by: Jeff Layton 
Reviewed-by: Christoph Hellwig 
Signed-off-by: J. Bruce Fields

nfsd: switch unsigned char flags in svc_fh to bools

2015-10-12T21:31:04+00:00

...just for clarity.

Signed-off-by: Jeff Layton 
Signed-off-by: J. Bruce Fields

nfsd: move svc_fh->fh_maxsize to just after fh_handle

2015-10-12T21:31:04+00:00

This moves the hole in the struct down below the flags fields, which
allows us to potentially add a new flag without growing the struct.

Signed-off-by: Jeff Layton 
Signed-off-by: J. Bruce Fields