[prev in list] [next in list] [prev in thread] [next in thread] 

List:       linux-nfs
Subject:    Re: [PATCH] nfsd: check for oversized NFSv2/v3 arguments
From:       NeilBrown <neilb () suse ! com>
Date:       2017-04-18 0:25:20
Message-ID: 87h91mtvdb.fsf () notabene ! neil ! brown ! name
[Download RAW message or body]

On Fri, Apr 14 2017, J. Bruce Fields wrote:

> (Cc'd you, Neil, partly on the off chance you might have a better idea
> where this came from.  Looks to me like it may have been there forever,
> but, I haven't looked too hard yet.)
>

Hi Bruce,
 I can't say that I like this patch at all.

The problem is that:

	pages = size / PAGE_SIZE + 1; /* extra page as we hold both request and reply.
				       * We assume one is at most one page
				       */

this assumption is never verified.
To my mind, the "obvious" way to verify this assumption is that an
attempt to generate a multi-page reply should fail if there was a
multi-page request.
Failing if there was a little bit of extra noise at the end of the
request seems harsher than necessary, and could result in a regression.

We already know how big replies can get, so we can perform a complete
sanity check quite early:

diff --git a/net/sunrpc/svc.c b/net/sunrpc/svc.c
index a08aeb56b8e4..14f4d425cf8c 100644
--- a/net/sunrpc/svc.c
+++ b/net/sunrpc/svc.c
@@ -1196,6 +1196,12 @@ svc_process_common(struct svc_rqst *rqstp, struct kvec *argv, struct kvec *resv)
 		goto err_bad_proc;
 	rqstp->rq_procinfo = procp;
 
+	if ((procp->pc_xdrressize == 0 ||
+	     procp->pc_xdrressize > XDR_QUADLEN(PAGE_SIZE)) &&
+	    rqstp->rq_arg.len > PAGE_SIZE)
+		/* The assumption about request/reply sizes in svc_init_buffer() is violated! */
+		goto err_garbage;
+
 	/* Syntactic check complete */
 	serv->sv_stats->rpccnt++;
 

I haven't tested this at all and haven't even convinced myself that
it covers every case (though I cannot immediately think of any likely
corners).

Does it address your test case?

Thanks,
NeilBrown

["signature.asc" (application/pgp-signature)]
--
To unsubscribe from this list: send the line "unsubscribe linux-nfs" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[prev in list] [next in list] [prev in thread] [next in thread] 

Configure | About | News | Add a list | Sponsored by KoreLogic