linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
To: linux-kernel@vger.kernel.org
Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
	stable@vger.kernel.org, Olga Kornievskaia <aglo@umich.edu>,
	"J. Bruce Fields" <bfields@redhat.com>,
	Salvatore Bonaccorso <carnil@debian.org>
Subject: [PATCH 4.9 18/24] nfsd4: catch some false session retries
Date: Wed, 13 Feb 2019 19:38:15 +0100	[thread overview]
Message-ID: <20190213183648.924916825@linuxfoundation.org> (raw)
In-Reply-To: <20190213183647.333441569@linuxfoundation.org>

4.9-stable review patch.  If anyone has any objections, please let me know.

------------------

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>
Cc: Salvatore Bonaccorso <carnil@debian.org>
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]);
+	}
 }
 
 /*
@@ -2347,6 +2349,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;
@@ -3049,6 +3053,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,
@@ -3108,6 +3140,9 @@ nfsd4_sequence(struct svc_rqst *rqstp,
 		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)



  parent reply	other threads:[~2019-02-13 18:39 UTC|newest]

Thread overview: 30+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-02-13 18:37 [PATCH 4.9 00/24] 4.9.157-stable review Greg Kroah-Hartman
2019-02-13 18:37 ` [PATCH 4.9 01/24] mtd: rawnand: gpmi: fix MX28 bus master lockup problem Greg Kroah-Hartman
2019-02-13 18:37 ` [PATCH 4.9 02/24] iio: chemical: atlas-ph-sensor: correct IIO_TEMP values to millicelsius Greg Kroah-Hartman
2019-02-13 18:38 ` [PATCH 4.9 03/24] signal: Always notice exiting tasks Greg Kroah-Hartman
2019-02-13 18:38 ` [PATCH 4.9 04/24] signal: Better detection of synchronous signals Greg Kroah-Hartman
2019-02-13 18:38 ` [PATCH 4.9 05/24] misc: vexpress: Off by one in vexpress_syscfg_exec() Greg Kroah-Hartman
2019-02-13 18:38 ` [PATCH 4.9 06/24] samples: mei: use /dev/mei0 instead of /dev/mei Greg Kroah-Hartman
2019-02-13 18:38 ` [PATCH 4.9 07/24] debugfs: fix debugfs_rename parameter checking Greg Kroah-Hartman
2019-02-13 18:38 ` [PATCH 4.9 08/24] mips: cm: reprime error cause Greg Kroah-Hartman
2019-02-13 18:38 ` [PATCH 4.9 09/24] MIPS: OCTEON: dont set octeon_dma_bar_type if PCI is disabled Greg Kroah-Hartman
2019-02-13 18:38 ` [PATCH 4.9 10/24] MIPS: VDSO: Include $(ccflags-vdso) in o32,n32 .lds builds Greg Kroah-Hartman
2019-02-13 18:38 ` [PATCH 4.9 11/24] ARM: iop32x/n2100: fix PCI IRQ mapping Greg Kroah-Hartman
2019-02-13 18:38 ` [PATCH 4.9 12/24] ARM: tango: Improve ARCH_MULTIPLATFORM compatibility Greg Kroah-Hartman
2019-02-13 18:38 ` [PATCH 4.9 13/24] mac80211: ensure that mgmt tx skbs have tailroom for encryption Greg Kroah-Hartman
2019-02-13 18:38 ` [PATCH 4.9 14/24] drm/modes: Prevent division by zero htotal Greg Kroah-Hartman
2019-02-13 18:38 ` [PATCH 4.9 15/24] drm/vmwgfx: Fix setting of dma masks Greg Kroah-Hartman
2019-02-13 18:38 ` [PATCH 4.9 16/24] drm/vmwgfx: Return error code from vmw_execbuf_copy_fence_user Greg Kroah-Hartman
2019-02-13 18:38 ` [PATCH 4.9 17/24] nfsd4: fix cached replies to solo SEQUENCE compounds Greg Kroah-Hartman
2019-02-13 18:38 ` Greg Kroah-Hartman [this message]
2019-02-13 18:38 ` [PATCH 4.9 19/24] HID: debug: fix the ring buffer implementation Greg Kroah-Hartman
2019-02-13 18:38 ` [PATCH 4.9 20/24] Revert "cifs: In Kconfig CONFIG_CIFS_POSIX needs depends on legacy (insecure cifs)" Greg Kroah-Hartman
2019-02-13 18:38 ` [PATCH 4.9 21/24] libceph: avoid KEEPALIVE_PENDING races in ceph_con_keepalive() Greg Kroah-Hartman
2019-02-13 18:38 ` [PATCH 4.9 22/24] xfrm: refine validation of template and selector families Greg Kroah-Hartman
2019-02-13 18:38 ` [PATCH 4.9 23/24] batman-adv: Avoid WARN on net_device without parent in netns Greg Kroah-Hartman
2019-02-13 18:38 ` [PATCH 4.9 24/24] batman-adv: Force mac header to start of data on xmit Greg Kroah-Hartman
2019-02-14  3:39 ` [PATCH 4.9 00/24] 4.9.157-stable review kernelci.org bot
2019-02-14 10:04 ` Jon Hunter
2019-02-14 16:30 ` Dan Rue
2019-02-14 19:16 ` Guenter Roeck
2019-02-14 22:22 ` shuah

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=20190213183648.924916825@linuxfoundation.org \
    --to=gregkh@linuxfoundation.org \
    --cc=aglo@umich.edu \
    --cc=bfields@redhat.com \
    --cc=carnil@debian.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=stable@vger.kernel.org \
    /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;
as well as URLs for NNTP newsgroup(s).