| From 53da6a53e1d414e05759fa59b7032ee08f4e22d7 Mon Sep 17 00:00:00 2001 |
| From: "J. Bruce Fields" <bfields@redhat.com> |
| Date: Tue, 17 Oct 2017 20:38:49 -0400 |
| Subject: nfsd4: catch some false session retries |
| |
| From: J. Bruce Fields <bfields@redhat.com> |
| |
| commit 53da6a53e1d414e05759fa59b7032ee08f4e22d7 upstream. |
| |
| The spec allows us to return NFS4ERR_SEQ_FALSE_RETRY if we notice that |
| the client is making a call that matches a previous (slot, seqid) pair |
| but that *isn't* actually a replay, because some detail of the call |
| doesn't actually match the previous one. |
| |
| Catching every such case is difficult, but we may as well catch a few |
| easy ones. This also handles the case described in the previous patch, |
| in a different way. |
| |
| The spec does however require us to catch the case where the difference |
| is in the rpc credentials. This prevents somebody from snooping another |
| user's replies by fabricating retries. |
| |
| (But the practical value of the attack is limited by the fact that the |
| replies with the most sensitive data are READ replies, which are not |
| normally cached.) |
| |
| Tested-by: Olga Kornievskaia <aglo@umich.edu> |
| Signed-off-by: J. Bruce Fields <bfields@redhat.com> |
| Signed-off-by: Donald Buczek <buczek@molgen.mpg.de> |
| Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org> |
| |
| --- |
| fs/nfsd/nfs4state.c | 37 ++++++++++++++++++++++++++++++++++++- |
| fs/nfsd/state.h | 1 + |
| 2 files changed, 37 insertions(+), 1 deletion(-) |
| |
| --- a/fs/nfsd/nfs4state.c |
| +++ b/fs/nfsd/nfs4state.c |
| @@ -1472,8 +1472,10 @@ free_session_slots(struct nfsd4_session |
| { |
| int i; |
| |
| - for (i = 0; i < ses->se_fchannel.maxreqs; i++) |
| + for (i = 0; i < ses->se_fchannel.maxreqs; i++) { |
| + free_svc_cred(&ses->se_slots[i]->sl_cred); |
| kfree(ses->se_slots[i]); |
| + } |
| } |
| |
| /* |
| @@ -2334,6 +2336,8 @@ nfsd4_store_cache_entry(struct nfsd4_com |
| slot->sl_flags |= NFSD4_SLOT_INITIALIZED; |
| slot->sl_opcnt = resp->opcnt; |
| slot->sl_status = resp->cstate.status; |
| + free_svc_cred(&slot->sl_cred); |
| + copy_cred(&slot->sl_cred, &resp->rqstp->rq_cred); |
| |
| if (!nfsd4_cache_this(resp)) { |
| slot->sl_flags &= ~NFSD4_SLOT_CACHED; |
| @@ -3040,6 +3044,34 @@ static bool nfsd4_request_too_big(struct |
| return xb->len > session->se_fchannel.maxreq_sz; |
| } |
| |
| +static bool replay_matches_cache(struct svc_rqst *rqstp, |
| + struct nfsd4_sequence *seq, struct nfsd4_slot *slot) |
| +{ |
| + struct nfsd4_compoundargs *argp = rqstp->rq_argp; |
| + |
| + if ((bool)(slot->sl_flags & NFSD4_SLOT_CACHETHIS) != |
| + (bool)seq->cachethis) |
| + return false; |
| + /* |
| + * If there's an error than the reply can have fewer ops than |
| + * the call. But if we cached a reply with *more* ops than the |
| + * call you're sending us now, then this new call is clearly not |
| + * really a replay of the old one: |
| + */ |
| + if (slot->sl_opcnt < argp->opcnt) |
| + return false; |
| + /* This is the only check explicitly called by spec: */ |
| + if (!same_creds(&rqstp->rq_cred, &slot->sl_cred)) |
| + return false; |
| + /* |
| + * There may be more comparisons we could actually do, but the |
| + * spec doesn't require us to catch every case where the calls |
| + * don't match (that would require caching the call as well as |
| + * the reply), so we don't bother. |
| + */ |
| + return true; |
| +} |
| + |
| __be32 |
| nfsd4_sequence(struct svc_rqst *rqstp, struct nfsd4_compound_state *cstate, |
| union nfsd4_op_u *u) |
| @@ -3099,6 +3131,9 @@ nfsd4_sequence(struct svc_rqst *rqstp, s |
| status = nfserr_seq_misordered; |
| if (!(slot->sl_flags & NFSD4_SLOT_INITIALIZED)) |
| goto out_put_session; |
| + status = nfserr_seq_false_retry; |
| + if (!replay_matches_cache(rqstp, seq, slot)) |
| + goto out_put_session; |
| cstate->slot = slot; |
| cstate->session = session; |
| cstate->clp = clp; |
| --- a/fs/nfsd/state.h |
| +++ b/fs/nfsd/state.h |
| @@ -169,6 +169,7 @@ static inline struct nfs4_delegation *de |
| struct nfsd4_slot { |
| u32 sl_seqid; |
| __be32 sl_status; |
| + struct svc_cred sl_cred; |
| u32 sl_datalen; |
| u16 sl_opcnt; |
| #define NFSD4_SLOT_INUSE (1 << 0) |