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
Attachment:
signature.asc
Description: PGP signature
