[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