Happy new year, campers.
The following series has been percolating for quite a while, thanks
to the extended 6.1-rc cycle. I'd like to get this work reviewed
for possible inclusion in v6.3, so I'm starting early.
The purpose of this series is to replace the svc_get* macros in the
Linux kernel server's RPC call header parsing code with xdr_stream
helpers. I've measured no change in CPU utilization after the
overhaul; svc_recv() and friends remain the highest CPU consumers by
an order of magnitude.
Memory safety: Buffer bounds checking after decoding each XDR item
is more memory-safe than the current decoding mechanism. Subsequent
memory safety improvements to the xdr_stream helpers will benefit
all who use them.
Audit friendliness: The new code has lots of comments and other
clean-up to help align it with the relevant RPC specifications. The
use of common helpers also makes the decoders easier to audit.
I've split the full series in half to make it easier to review. The
patches posted here handle RPC call header decoding. Remaining
patches, to be posted later, deal with RPC reply header encoding.
Yes, there are a lot of patches, but they are each small, easily
chewed mechanical changes.
---
Chuck Lever (25):
SUNRPC: Push svcxdr_init_decode() into svc_process_common()
SUNRPC: Move svcxdr_init_decode() into ->accept methods
SUNRPC: Add an XDR decoding helper for struct opaque_auth
SUNRPC: Convert svcauth_null_accept() to use xdr_stream
SUNRPC: Convert svcauth_unix_accept() to use xdr_stream
SUNRPC: Convert svcauth_tls_accept() to use xdr_stream
SUNRPC: Move the server-side GSS upcall to a noinline function
SUNRPC: Hoist common verifier decoding code into svcauth_gss_proc_init()
SUNRPC: Remove gss_read_common_verf()
SUNRPC: Remove gss_read_verf()
SUNRPC: Convert server-side GSS upcall helpers to use xdr_stream
SUNRPC: Replace read_u32_from_xdr_buf() with existing XDR helper
SUNRPC: Rename automatic variables in unwrap_integ_data()
SUNRPC: Convert unwrap_integ_data() to use xdr_stream
SUNRPC: Rename automatic variables in unwrap_priv_data()
SUNRPC: Convert unwrap_priv_data() to use xdr_stream
SUNRPC: Convert gss_verify_header() to use xdr_stream
SUNRPC: Clean up svcauth_gss_accept's NULL procedure check
SUNRPC: Convert the svcauth_gss_accept() pre-amble to use xdr_stream
SUNRPC: Hoist init_decode out of svc_authenticate()
SUNRPC: Re-order construction of the first reply fields
SUNRPC: Eliminate unneeded variable
SUNRPC: Decode most of RPC header with xdr_stream
SUNRPC: Remove svc_process_common's argv parameter
SUNRPC: Hoist svcxdr_init_decode() into svc_process()
fs/lockd/svc.c | 1 -
fs/nfs/callback_xdr.c | 1 -
fs/nfsd/nfssvc.c | 1 -
include/linux/sunrpc/msg_prot.h | 5 +
include/linux/sunrpc/xdr.h | 5 +-
net/sunrpc/auth_gss/svcauth_gss.c | 512 ++++++++++++++++--------------
net/sunrpc/svc.c | 69 ++--
net/sunrpc/svc_xprt.c | 1 -
net/sunrpc/svcauth.c | 13 +-
net/sunrpc/svcauth_unix.c | 132 +++++---
net/sunrpc/xdr.c | 50 ++-
11 files changed, 468 insertions(+), 322 deletions(-)
--
Chuck Lever
From: Chuck Lever <[email protected]>
Done as part of hardening the server-side RPC header decoding path.
Signed-off-by: Chuck Lever <[email protected]>
---
net/sunrpc/auth_gss/svcauth_gss.c | 125 ++++++++++++++++++++++---------------
1 file changed, 75 insertions(+), 50 deletions(-)
diff --git a/net/sunrpc/auth_gss/svcauth_gss.c b/net/sunrpc/auth_gss/svcauth_gss.c
index 363c25198547..557de28127fe 100644
--- a/net/sunrpc/auth_gss/svcauth_gss.c
+++ b/net/sunrpc/auth_gss/svcauth_gss.c
@@ -697,23 +697,6 @@ static inline u32 round_up_to_quad(u32 i)
return (i + 3 ) & ~3;
}
-static inline int
-svc_safe_getnetobj(struct kvec *argv, struct xdr_netobj *o)
-{
- int l;
-
- if (argv->iov_len < 4)
- return -1;
- o->len = svc_getnl(argv);
- l = round_up_to_quad(o->len);
- if (argv->iov_len < l)
- return -1;
- o->data = argv->iov_base;
- argv->iov_base += l;
- argv->iov_len -= l;
- return 0;
-}
-
static inline int
svc_safe_putnetobj(struct kvec *resv, struct xdr_netobj *o)
{
@@ -1553,27 +1536,91 @@ static void destroy_use_gss_proxy_proc_entry(struct net *net) {}
#endif /* CONFIG_PROC_FS */
/*
- * Accept an rpcsec packet.
- * If context establishment, punt to user space
- * If data exchange, verify/decrypt
- * If context destruction, handle here
- * In the context establishment and destruction case we encode
- * response here and return SVC_COMPLETE.
+ * The Call's credential body should contain a struct rpc_gss_cred_t.
+ *
+ * RFC 2203 Section 5
+ *
+ * struct rpc_gss_cred_t {
+ * union switch (unsigned int version) {
+ * case RPCSEC_GSS_VERS_1:
+ * struct {
+ * rpc_gss_proc_t gss_proc;
+ * unsigned int seq_num;
+ * rpc_gss_service_t service;
+ * opaque handle<>;
+ * } rpc_gss_cred_vers_1_t;
+ * }
+ * };
+ */
+static bool
+svcauth_gss_decode_credbody(struct xdr_stream *xdr,
+ struct rpc_gss_wire_cred *gc,
+ __be32 **rpcstart)
+{
+ ssize_t handle_len;
+ u32 body_len;
+ __be32 *p;
+
+ p = xdr_inline_decode(xdr, XDR_UNIT);
+ if (!p)
+ return false;
+ /*
+ * start of rpc packet is 7 u32's back from here:
+ * xid direction rpcversion prog vers proc flavour
+ */
+ *rpcstart = p - 7;
+ body_len = be32_to_cpup(p);
+ if (body_len > RPC_MAX_AUTH_SIZE)
+ return false;
+
+ /* struct rpc_gss_cred_t */
+ if (xdr_stream_decode_u32(xdr, &gc->gc_v) < 0)
+ return false;
+ if (xdr_stream_decode_u32(xdr, &gc->gc_proc) < 0)
+ return false;
+ if (xdr_stream_decode_u32(xdr, &gc->gc_seq) < 0)
+ return false;
+ if (xdr_stream_decode_u32(xdr, &gc->gc_svc) < 0)
+ return false;
+ handle_len = xdr_stream_decode_opaque_inline(xdr,
+ (void **)&gc->gc_ctx.data,
+ body_len);
+ if (handle_len < 0)
+ return false;
+ if (body_len != XDR_UNIT * 5 + xdr_align_size(handle_len))
+ return false;
+
+ gc->gc_ctx.len = handle_len;
+ return true;
+}
+
+/**
+ * svcauth_gss_accept - Decode and validate incoming RPC_AUTH_GSS credential
+ * @rqstp: RPC transaction
+ *
+ * Return values:
+ * %SVC_OK: Success
+ * %SVC_COMPLETE: GSS context lifetime event
+ * %SVC_DENIED: Credential or verifier is not valid
+ * %SVC_GARBAGE: Failed to decode credential or verifier
+ * %SVC_CLOSE: Temporary failure
+ *
+ * The rqstp->rq_auth_stat field is also set (see RFCs 2203 and 5531).
*/
static int
svcauth_gss_accept(struct svc_rqst *rqstp)
{
- struct kvec *argv = &rqstp->rq_arg.head[0];
struct kvec *resv = &rqstp->rq_res.head[0];
- u32 crlen;
struct gss_svc_data *svcdata = rqstp->rq_auth_data;
+ __be32 *rpcstart;
struct rpc_gss_wire_cred *gc;
struct rsc *rsci = NULL;
- __be32 *rpcstart;
__be32 *reject_stat = resv->iov_base + resv->iov_len;
int ret;
struct sunrpc_net *sn = net_generic(SVC_NET(rqstp), sunrpc_net_id);
+ svcxdr_init_decode(rqstp);
+
rqstp->rq_auth_stat = rpc_autherr_badcred;
if (!svcdata)
svcdata = kmalloc(sizeof(*svcdata), GFP_KERNEL);
@@ -1584,31 +1631,10 @@ svcauth_gss_accept(struct svc_rqst *rqstp)
svcdata->rsci = NULL;
gc = &svcdata->clcred;
- /* start of rpc packet is 7 u32's back from here:
- * xid direction rpcversion prog vers proc flavour
- */
- rpcstart = argv->iov_base;
- rpcstart -= 7;
-
- /* credential is:
- * version(==1), proc(0,1,2,3), seq, service (1,2,3), handle
- * at least 5 u32s, and is preceded by length, so that makes 6.
- */
-
- if (argv->iov_len < 5 * 4)
+ if (!svcauth_gss_decode_credbody(&rqstp->rq_arg_stream, gc, &rpcstart))
goto auth_err;
- crlen = svc_getnl(argv);
- if (svc_getnl(argv) != RPC_GSS_VERSION)
+ if (gc->gc_v != RPC_GSS_VERSION)
goto auth_err;
- gc->gc_proc = svc_getnl(argv);
- gc->gc_seq = svc_getnl(argv);
- gc->gc_svc = svc_getnl(argv);
- if (svc_safe_getnetobj(argv, &gc->gc_ctx))
- goto auth_err;
- if (crlen != round_up_to_quad(gc->gc_ctx.len) + 5 * 4)
- goto auth_err;
-
- svcxdr_init_decode(rqstp);
switch (gc->gc_proc) {
case RPC_GSS_PROC_INIT:
@@ -1621,7 +1647,6 @@ svcauth_gss_accept(struct svc_rqst *rqstp)
goto auth_err;
fallthrough;
case RPC_GSS_PROC_DATA:
- /* Look up the context, and check the verifier: */
rqstp->rq_auth_stat = rpcsec_gsserr_credproblem;
rsci = gss_svc_searchbyctx(sn->rsc_cache, &gc->gc_ctx);
if (!rsci)
From: Chuck Lever <[email protected]>
Now that each ->accept method has been converted to use xdr_stream,
the svcxdr_init_decode() calls can be hoisted back up into the
generic RPC server code.
The dprintk in svc_authenticate() is removed, since
trace_svc_authenticate() reports the same information.
Signed-off-by: Chuck Lever <[email protected]>
---
net/sunrpc/auth_gss/svcauth_gss.c | 2 --
net/sunrpc/svc.c | 1 +
net/sunrpc/svcauth.c | 13 ++++++++-----
net/sunrpc/svcauth_unix.c | 6 ------
4 files changed, 9 insertions(+), 13 deletions(-)
diff --git a/net/sunrpc/auth_gss/svcauth_gss.c b/net/sunrpc/auth_gss/svcauth_gss.c
index 557de28127fe..2e603358fae1 100644
--- a/net/sunrpc/auth_gss/svcauth_gss.c
+++ b/net/sunrpc/auth_gss/svcauth_gss.c
@@ -1619,8 +1619,6 @@ svcauth_gss_accept(struct svc_rqst *rqstp)
int ret;
struct sunrpc_net *sn = net_generic(SVC_NET(rqstp), sunrpc_net_id);
- svcxdr_init_decode(rqstp);
-
rqstp->rq_auth_stat = rpc_autherr_badcred;
if (!svcdata)
svcdata = kmalloc(sizeof(*svcdata), GFP_KERNEL);
diff --git a/net/sunrpc/svc.c b/net/sunrpc/svc.c
index 85f0c3cfc877..e4f8a177763e 100644
--- a/net/sunrpc/svc.c
+++ b/net/sunrpc/svc.c
@@ -1276,6 +1276,7 @@ svc_process_common(struct svc_rqst *rqstp, struct kvec *argv, struct kvec *resv)
* We do this before anything else in order to get a decent
* auth verifier.
*/
+ svcxdr_init_decode(rqstp);
auth_res = svc_authenticate(rqstp);
/* Also give the program a chance to reject this call: */
if (auth_res == SVC_OK && progp)
diff --git a/net/sunrpc/svcauth.c b/net/sunrpc/svcauth.c
index e72ba2f13f6c..67d8245a08af 100644
--- a/net/sunrpc/svcauth.c
+++ b/net/sunrpc/svcauth.c
@@ -63,14 +63,17 @@ svc_put_auth_ops(struct auth_ops *aops)
int
svc_authenticate(struct svc_rqst *rqstp)
{
- rpc_authflavor_t flavor;
- struct auth_ops *aops;
+ struct auth_ops *aops;
+ u32 flavor;
rqstp->rq_auth_stat = rpc_auth_ok;
- flavor = svc_getnl(&rqstp->rq_arg.head[0]);
-
- dprintk("svc: svc_authenticate (%d)\n", flavor);
+ /*
+ * Decode the Call credential's flavor field. The credential's
+ * body field is decoded in the chosen ->accept method below.
+ */
+ if (xdr_stream_decode_u32(&rqstp->rq_arg_stream, &flavor) < 0)
+ return SVC_GARBAGE;
aops = svc_get_auth_ops(flavor);
if (aops == NULL) {
diff --git a/net/sunrpc/svcauth_unix.c b/net/sunrpc/svcauth_unix.c
index 168e12137754..f09a148aa0c1 100644
--- a/net/sunrpc/svcauth_unix.c
+++ b/net/sunrpc/svcauth_unix.c
@@ -750,8 +750,6 @@ svcauth_null_accept(struct svc_rqst *rqstp)
u32 flavor, len;
void *body;
- svcxdr_init_decode(rqstp);
-
/* Length of Call's credential body field: */
if (xdr_stream_decode_u32(xdr, &len) < 0)
return SVC_GARBAGE;
@@ -828,8 +826,6 @@ svcauth_tls_accept(struct svc_rqst *rqstp)
u32 flavor, len;
void *body;
- svcxdr_init_decode(rqstp);
-
/* Length of Call's credential body field: */
if (xdr_stream_decode_u32(xdr, &len) < 0)
return SVC_GARBAGE;
@@ -905,8 +901,6 @@ svcauth_unix_accept(struct svc_rqst *rqstp)
void *body;
__be32 *p;
- svcxdr_init_decode(rqstp);
-
/*
* This implementation ignores the length of the Call's
* credential body field and the timestamp and machinename
From: Chuck Lever <[email protected]>
Clean up: Saving the RPC program number in two places is
unnecessary.
Signed-off-by: Chuck Lever <[email protected]>
---
net/sunrpc/svc.c | 8 ++++----
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/net/sunrpc/svc.c b/net/sunrpc/svc.c
index e473c19b2f8d..3c1574f362fe 100644
--- a/net/sunrpc/svc.c
+++ b/net/sunrpc/svc.c
@@ -1232,7 +1232,7 @@ svc_process_common(struct svc_rqst *rqstp, struct kvec *argv, struct kvec *resv)
struct svc_serv *serv = rqstp->rq_server;
struct svc_process_info process;
__be32 *statp;
- u32 prog, vers;
+ u32 vers;
__be32 rpc_stat;
int auth_res, rc;
__be32 *reply_statp;
@@ -1259,12 +1259,12 @@ svc_process_common(struct svc_rqst *rqstp, struct kvec *argv, struct kvec *resv)
svc_putnl(resv, 0); /* ACCEPT */
- rqstp->rq_prog = prog = svc_getnl(argv); /* program number */
+ rqstp->rq_prog = svc_getnl(argv); /* program number */
rqstp->rq_vers = svc_getnl(argv); /* version number */
rqstp->rq_proc = svc_getnl(argv); /* procedure number */
for (progp = serv->sv_program; progp; progp = progp->pg_next)
- if (prog == progp->pg_prog)
+ if (rqstp->rq_prog == progp->pg_prog)
break;
/*
@@ -1389,7 +1389,7 @@ svc_process_common(struct svc_rqst *rqstp, struct kvec *argv, struct kvec *resv)
goto sendit;
err_bad_prog:
- dprintk("svc: unknown program %d\n", prog);
+ dprintk("svc: unknown program %d\n", rqstp->rq_prog);
serv->sv_stats->rpcbadfmt++;
svc_putnl(resv, RPC_PROG_UNAVAIL);
goto sendit;
From: Chuck Lever <[email protected]>
gss_read_verf() is already short. Fold it into its only caller.
Signed-off-by: Chuck Lever <[email protected]>
---
net/sunrpc/auth_gss/svcauth_gss.c | 37 +++++++++++--------------------------
1 file changed, 11 insertions(+), 26 deletions(-)
diff --git a/net/sunrpc/auth_gss/svcauth_gss.c b/net/sunrpc/auth_gss/svcauth_gss.c
index 5da4a3d48a7d..5b03a97e32b7 100644
--- a/net/sunrpc/auth_gss/svcauth_gss.c
+++ b/net/sunrpc/auth_gss/svcauth_gss.c
@@ -1090,28 +1090,6 @@ gss_write_init_verf(struct cache_detail *cd, struct svc_rqst *rqstp,
return rc;
}
-static inline int
-gss_read_verf(struct rpc_gss_wire_cred *gc,
- struct kvec *argv, __be32 *authp,
- struct xdr_netobj *in_handle,
- struct xdr_netobj *in_token)
-{
- struct xdr_netobj tmpobj;
-
- if (dup_netobj(in_handle, &gc->gc_ctx))
- return SVC_CLOSE;
- if (svc_safe_getnetobj(argv, &tmpobj)) {
- kfree(in_handle->data);
- return SVC_DENIED;
- }
- if (dup_netobj(in_token, &tmpobj)) {
- kfree(in_handle->data);
- return SVC_CLOSE;
- }
-
- return 0;
-}
-
static void gss_free_in_token_pages(struct gssp_in_token *in_token)
{
u32 inlen;
@@ -1224,14 +1202,21 @@ static int svcauth_gss_legacy_init(struct svc_rqst *rqstp,
struct kvec *argv = &rqstp->rq_arg.head[0];
struct kvec *resv = &rqstp->rq_res.head[0];
struct rsi *rsip, rsikey;
+ struct xdr_netobj tmpobj;
int ret;
struct sunrpc_net *sn = net_generic(SVC_NET(rqstp), sunrpc_net_id);
memset(&rsikey, 0, sizeof(rsikey));
- ret = gss_read_verf(gc, argv, &rqstp->rq_auth_stat,
- &rsikey.in_handle, &rsikey.in_token);
- if (ret)
- return ret;
+ if (dup_netobj(&rsikey.in_handle, &gc->gc_ctx))
+ return SVC_CLOSE;
+ if (svc_safe_getnetobj(argv, &tmpobj)) {
+ kfree(rsikey.in_handle.data);
+ return SVC_DENIED;
+ }
+ if (dup_netobj(&rsikey.in_token, &tmpobj)) {
+ kfree(rsikey.in_handle.data);
+ return SVC_CLOSE;
+ }
/* Perform upcall, or find upcall result: */
rsip = rsi_lookup(sn->rsi_cache, &rsikey);
From: Chuck Lever <[email protected]>
Clean up: With xdr_stream decoding, the @argv parameter is no longer
used.
Signed-off-by: Chuck Lever <[email protected]>
---
net/sunrpc/svc.c | 10 +++++-----
1 file changed, 5 insertions(+), 5 deletions(-)
diff --git a/net/sunrpc/svc.c b/net/sunrpc/svc.c
index f292b898f200..571151a17e87 100644
--- a/net/sunrpc/svc.c
+++ b/net/sunrpc/svc.c
@@ -1225,7 +1225,7 @@ EXPORT_SYMBOL_GPL(svc_generic_init_request);
* Common routine for processing the RPC request.
*/
static int
-svc_process_common(struct svc_rqst *rqstp, struct kvec *argv, struct kvec *resv)
+svc_process_common(struct svc_rqst *rqstp, struct kvec *resv)
{
struct svc_program *progp;
const struct svc_procedure *procp = NULL;
@@ -1363,8 +1363,8 @@ svc_process_common(struct svc_rqst *rqstp, struct kvec *argv, struct kvec *resv)
return 0;
err_short_len:
- svc_printk(rqstp, "short len %zd, dropping request\n",
- argv->iov_len);
+ svc_printk(rqstp, "short len %u, dropping request\n",
+ rqstp->rq_arg.len);
goto close_xprt;
err_bad_rpc:
@@ -1453,7 +1453,7 @@ svc_process(struct svc_rqst *rqstp)
dir = svc_getu32(argv);
if (dir != rpc_call)
goto out_baddir;
- if (!svc_process_common(rqstp, argv, resv))
+ if (!svc_process_common(rqstp, resv))
goto out_drop;
return svc_send(rqstp);
@@ -1519,7 +1519,7 @@ bc_svc_process(struct svc_serv *serv, struct rpc_rqst *req,
svc_getnl(argv); /* CALLDIR */
/* Parse and execute the bc call */
- proc_error = svc_process_common(rqstp, argv, resv);
+ proc_error = svc_process_common(rqstp, resv);
atomic_dec(&req->rq_xprt->bc_slot_count);
if (!proc_error) {
From: Chuck Lever <[email protected]>
Since upcalls are infrequent, ensure the compiler places the upcall
mechanism out-of-line from the I/O path.
Signed-off-by: Chuck Lever <[email protected]>
---
net/sunrpc/auth_gss/svcauth_gss.c | 13 +++++++++----
1 file changed, 9 insertions(+), 4 deletions(-)
diff --git a/net/sunrpc/auth_gss/svcauth_gss.c b/net/sunrpc/auth_gss/svcauth_gss.c
index 8ebc06bebbec..2a11721dd5c4 100644
--- a/net/sunrpc/auth_gss/svcauth_gss.c
+++ b/net/sunrpc/auth_gss/svcauth_gss.c
@@ -1444,6 +1444,14 @@ static bool use_gss_proxy(struct net *net)
return sn->use_gss_proxy;
}
+static noinline_for_stack int
+svcauth_gss_proc_init(struct svc_rqst *rqstp, struct rpc_gss_wire_cred *gc)
+{
+ if (!use_gss_proxy(SVC_NET(rqstp)))
+ return svcauth_gss_legacy_init(rqstp, gc);
+ return svcauth_gss_proxy_init(rqstp, gc);
+}
+
#ifdef CONFIG_PROC_FS
static ssize_t write_gssp(struct file *file, const char __user *buf,
@@ -1600,10 +1608,7 @@ svcauth_gss_accept(struct svc_rqst *rqstp)
switch (gc->gc_proc) {
case RPC_GSS_PROC_INIT:
case RPC_GSS_PROC_CONTINUE_INIT:
- if (use_gss_proxy(SVC_NET(rqstp)))
- return svcauth_gss_proxy_init(rqstp, gc);
- else
- return svcauth_gss_legacy_init(rqstp, gc);
+ return svcauth_gss_proc_init(rqstp, gc);
case RPC_GSS_PROC_DATA:
case RPC_GSS_PROC_DESTROY:
/* Look up the context, and check the verifier: */
From: Chuck Lever <[email protected]>
Done as part of hardening the server-side RPC header decoding path.
Signed-off-by: Chuck Lever <[email protected]>
---
include/linux/sunrpc/xdr.h | 1 -
net/sunrpc/auth_gss/svcauth_gss.c | 65 ++++++++++++-------------------------
net/sunrpc/xdr.c | 7 ----
3 files changed, 21 insertions(+), 52 deletions(-)
diff --git a/include/linux/sunrpc/xdr.h b/include/linux/sunrpc/xdr.h
index accfe8d6e283..884df67009f4 100644
--- a/include/linux/sunrpc/xdr.h
+++ b/include/linux/sunrpc/xdr.h
@@ -188,7 +188,6 @@ xdr_adjust_iovec(struct kvec *iov, __be32 *p)
/*
* XDR buffer helper functions
*/
-extern void xdr_shift_buf(struct xdr_buf *, size_t);
extern void xdr_buf_from_iov(const struct kvec *, struct xdr_buf *);
extern int xdr_buf_subsegment(const struct xdr_buf *, struct xdr_buf *, unsigned int, unsigned int);
extern void xdr_buf_trim(struct xdr_buf *, unsigned int);
diff --git a/net/sunrpc/auth_gss/svcauth_gss.c b/net/sunrpc/auth_gss/svcauth_gss.c
index 8f91768d0be0..074bfe9fb838 100644
--- a/net/sunrpc/auth_gss/svcauth_gss.c
+++ b/net/sunrpc/auth_gss/svcauth_gss.c
@@ -982,17 +982,6 @@ total_buf_len(struct xdr_buf *buf)
return buf->head[0].iov_len + buf->page_len + buf->tail[0].iov_len;
}
-static void
-fix_priv_head(struct xdr_buf *buf, int pad)
-{
- if (buf->page_len == 0) {
- /* We need to adjust head and buf->len in tandem in this
- * case to make svc_defer() work--it finds the original
- * buffer start using buf->len - buf->head[0].iov_len. */
- buf->head[0].iov_len -= pad;
- }
-}
-
/*
* RFC 2203, Section 5.3.2.3
*
@@ -1005,49 +994,37 @@ fix_priv_head(struct xdr_buf *buf, int pad)
* proc_req_arg_t arg;
* };
*/
-static int
-svcauth_gss_unwrap_priv(struct svc_rqst *rqstp, struct xdr_buf *buf, u32 seq,
- struct gss_ctx *ctx)
+static noinline_for_stack int
+svcauth_gss_unwrap_priv(struct svc_rqst *rqstp, u32 seq, struct gss_ctx *ctx)
{
- u32 len, seq_num, maj_stat;
- int pad, remaining_len, offset;
+ struct xdr_stream *xdr = &rqstp->rq_arg_stream;
+ u32 len, maj_stat, seq_num, offset;
+ struct xdr_buf *buf = xdr->buf;
+ unsigned int saved_len;
__clear_bit(RQ_SPLICE_OK, &rqstp->rq_flags);
- len = svc_getnl(&buf->head[0]);
+ if (xdr_stream_decode_u32(xdr, &len) < 0)
+ goto unwrap_failed;
if (rqstp->rq_deferred) {
/* Already decrypted last time through! The sequence number
* check at out_seq is unnecessary but harmless: */
goto out_seq;
}
- /* buf->len is the number of bytes from the original start of the
- * request to the end, where head[0].iov_len is just the bytes
- * not yet read from the head, so these two values are different: */
- remaining_len = total_buf_len(buf);
- if (len > remaining_len)
+ if (len > xdr_stream_remaining(xdr))
goto unwrap_failed;
- pad = remaining_len - len;
- buf->len -= pad;
- fix_priv_head(buf, pad);
-
- maj_stat = gss_unwrap(ctx, 0, len, buf);
- pad = len - buf->len;
- /* The upper layers assume the buffer is aligned on 4-byte boundaries.
- * In the krb5p case, at least, the data ends up offset, so we need to
- * move it around. */
- /* XXX: This is very inefficient. It would be better to either do
- * this while we encrypt, or maybe in the receive code, if we can peak
- * ahead and work out the service and mechanism there. */
- offset = xdr_pad_size(buf->head[0].iov_len);
- if (offset) {
- buf->buflen = RPCSVC_MAXPAYLOAD;
- xdr_shift_buf(buf, offset);
- fix_priv_head(buf, pad);
- }
+ offset = xdr_stream_pos(xdr);
+
+ saved_len = buf->len;
+ maj_stat = gss_unwrap(ctx, offset, offset + len, buf);
if (maj_stat != GSS_S_COMPLETE)
goto bad_unwrap;
+ xdr->nwords -= XDR_QUADLEN(saved_len - buf->len);
+
out_seq:
- seq_num = svc_getnl(&buf->head[0]);
+ /* gss_unwrap() decrypted the sequence number. */
+ if (xdr_stream_decode_u32(xdr, &seq_num) < 0)
+ goto unwrap_failed;
if (seq_num != seq)
goto bad_seqno;
return 0;
@@ -1689,11 +1666,11 @@ svcauth_gss_accept(struct svc_rqst *rqstp)
/* placeholders for length and seq. number: */
svc_putnl(resv, 0);
svc_putnl(resv, 0);
- if (svcauth_gss_unwrap_priv(rqstp, &rqstp->rq_arg,
- gc->gc_seq, rsci->mechctx))
+ svcxdr_init_decode(rqstp);
+ if (svcauth_gss_unwrap_priv(rqstp, gc->gc_seq,
+ rsci->mechctx))
goto garbage_args;
rqstp->rq_auth_slack = RPC_MAX_AUTH_SIZE * 2;
- svcxdr_init_decode(rqstp);
break;
default:
goto auth_err;
diff --git a/net/sunrpc/xdr.c b/net/sunrpc/xdr.c
index c7e89921d511..56d87c784c9e 100644
--- a/net/sunrpc/xdr.c
+++ b/net/sunrpc/xdr.c
@@ -863,13 +863,6 @@ static unsigned int xdr_shrink_pagelen(struct xdr_buf *buf, unsigned int len)
return shift;
}
-void
-xdr_shift_buf(struct xdr_buf *buf, size_t len)
-{
- xdr_shrink_bufhead(buf, buf->head->iov_len - len);
-}
-EXPORT_SYMBOL_GPL(xdr_shift_buf);
-
/**
* xdr_stream_pos - Return the current offset from the start of the xdr_stream
* @xdr: pointer to struct xdr_stream
From: Chuck Lever <[email protected]>
Done as part of hardening the server-side RPC header decoding path.
Since the server-side of the Linux kernel SunRPC implementation
ignores the contents of the Call's machinename field, there's no
need for its RPC_AUTH_UNIX authenticator to reject names that are
larger than UNX_MAXNODENAME.
Signed-off-by: Chuck Lever <[email protected]>
---
include/linux/sunrpc/msg_prot.h | 5 +++
net/sunrpc/svcauth_unix.c | 71 ++++++++++++++++++++++++++++-----------
2 files changed, 56 insertions(+), 20 deletions(-)
diff --git a/include/linux/sunrpc/msg_prot.h b/include/linux/sunrpc/msg_prot.h
index 02117ed0fa2e..c4b0eb2b2f04 100644
--- a/include/linux/sunrpc/msg_prot.h
+++ b/include/linux/sunrpc/msg_prot.h
@@ -34,6 +34,11 @@ enum rpc_auth_flavors {
RPC_AUTH_GSS_SPKMP = 390011,
};
+/* Maximum size (in octets) of the machinename in an AUTH_UNIX
+ * credential (per RFC 5531 Appendix A)
+ */
+#define RPC_MAX_MACHINENAME (255)
+
/* Maximum size (in bytes) of an rpc credential or verifier */
#define RPC_MAX_AUTH_SIZE (400)
diff --git a/net/sunrpc/svcauth_unix.c b/net/sunrpc/svcauth_unix.c
index 95354f03bb05..b6aef9c5113b 100644
--- a/net/sunrpc/svcauth_unix.c
+++ b/net/sunrpc/svcauth_unix.c
@@ -867,26 +867,45 @@ struct auth_ops svcauth_tls = {
};
+/**
+ * svcauth_unix_accept - Decode and validate incoming RPC_AUTH_SYS credential
+ * @rqstp: RPC transaction
+ *
+ * Return values:
+ * %SVC_OK: Both credential and verifier are valid
+ * %SVC_DENIED: Credential or verifier is not valid
+ * %SVC_GARBAGE: Failed to decode credential or verifier
+ * %SVC_CLOSE: Temporary failure
+ *
+ * rqstp->rq_auth_stat is set as mandated by RFC 5531.
+ */
static int
svcauth_unix_accept(struct svc_rqst *rqstp)
{
- struct kvec *argv = &rqstp->rq_arg.head[0];
struct kvec *resv = &rqstp->rq_res.head[0];
+ struct xdr_stream *xdr = &rqstp->rq_arg_stream;
struct svc_cred *cred = &rqstp->rq_cred;
struct user_namespace *userns;
- u32 slen, i;
- int len = argv->iov_len;
+ u32 flavor, len, i;
+ void *body;
+ __be32 *p;
+
+ svcxdr_init_decode(rqstp);
- if ((len -= 3*4) < 0)
+ /*
+ * This implementation ignores the length of the Call's
+ * credential body field and the timestamp and machinename
+ * fields.
+ */
+ p = xdr_inline_decode(xdr, XDR_UNIT * 3);
+ if (!p)
+ return SVC_GARBAGE;
+ len = be32_to_cpup(p + 2);
+ if (len > RPC_MAX_MACHINENAME)
+ return SVC_GARBAGE;
+ if (!xdr_inline_decode(xdr, len))
return SVC_GARBAGE;
- svc_getu32(argv); /* length */
- svc_getu32(argv); /* time stamp */
- slen = XDR_QUADLEN(svc_getnl(argv)); /* machname length */
- if (slen > 64 || (len -= (slen + 3)*4) < 0)
- goto badcred;
- argv->iov_base = (void*)((__be32*)argv->iov_base + slen); /* skip machname */
- argv->iov_len -= slen*4;
/*
* Note: we skip uid_valid()/gid_valid() checks here for
* backwards compatibility with clients that use -1 id's.
@@ -896,20 +915,33 @@ svcauth_unix_accept(struct svc_rqst *rqstp)
*/
userns = (rqstp->rq_xprt && rqstp->rq_xprt->xpt_cred) ?
rqstp->rq_xprt->xpt_cred->user_ns : &init_user_ns;
- cred->cr_uid = make_kuid(userns, svc_getnl(argv)); /* uid */
- cred->cr_gid = make_kgid(userns, svc_getnl(argv)); /* gid */
- slen = svc_getnl(argv); /* gids length */
- if (slen > UNX_NGROUPS || (len -= (slen + 2)*4) < 0)
+ if (xdr_stream_decode_u32(xdr, &i) < 0)
+ return SVC_GARBAGE;
+ cred->cr_uid = make_kuid(userns, i);
+ if (xdr_stream_decode_u32(xdr, &i) < 0)
+ return SVC_GARBAGE;
+ cred->cr_gid = make_kgid(userns, i);
+
+ if (xdr_stream_decode_u32(xdr, &len) < 0)
+ return SVC_GARBAGE;
+ if (len > UNX_NGROUPS)
goto badcred;
- cred->cr_group_info = groups_alloc(slen);
+ p = xdr_inline_decode(xdr, XDR_UNIT * len);
+ if (!p)
+ return SVC_GARBAGE;
+ cred->cr_group_info = groups_alloc(len);
if (cred->cr_group_info == NULL)
return SVC_CLOSE;
- for (i = 0; i < slen; i++) {
- kgid_t kgid = make_kgid(userns, svc_getnl(argv));
+ for (i = 0; i < len; i++) {
+ kgid_t kgid = make_kgid(userns, be32_to_cpup(p++));
cred->cr_group_info->gid[i] = kgid;
}
groups_sort(cred->cr_group_info);
- if (svc_getu32(argv) != htonl(RPC_AUTH_NULL) || svc_getu32(argv) != 0) {
+
+ /* Call's verf field: */
+ if (xdr_stream_decode_opaque_auth(xdr, &flavor, &body, &len) < 0)
+ return SVC_GARBAGE;
+ if (flavor != RPC_AUTH_NULL || len != 0) {
rqstp->rq_auth_stat = rpc_autherr_badverf;
return SVC_DENIED;
}
@@ -919,7 +951,6 @@ svcauth_unix_accept(struct svc_rqst *rqstp)
svc_putnl(resv, 0);
rqstp->rq_cred.cr_flavor = RPC_AUTH_UNIX;
- svcxdr_init_decode(rqstp);
return SVC_OK;
badcred:
From: Chuck Lever <[email protected]>
Done as part of hardening the server-side RPC header decoding path.
Signed-off-by: Chuck Lever <[email protected]>
---
include/linux/sunrpc/xdr.h | 1 +
net/sunrpc/auth_gss/svcauth_gss.c | 47 ++++++++++++++++++++++++-------------
net/sunrpc/xdr.c | 15 ++++++++++++
3 files changed, 47 insertions(+), 16 deletions(-)
diff --git a/include/linux/sunrpc/xdr.h b/include/linux/sunrpc/xdr.h
index 8b5c9d0cdcb5..accfe8d6e283 100644
--- a/include/linux/sunrpc/xdr.h
+++ b/include/linux/sunrpc/xdr.h
@@ -247,6 +247,7 @@ extern int xdr_reserve_space_vec(struct xdr_stream *xdr, struct kvec *vec,
size_t nbytes);
extern void __xdr_commit_encode(struct xdr_stream *xdr);
extern void xdr_truncate_encode(struct xdr_stream *xdr, size_t len);
+extern void xdr_truncate_decode(struct xdr_stream *xdr, size_t len);
extern int xdr_restrict_buflen(struct xdr_stream *xdr, int newbuflen);
extern void xdr_write_pages(struct xdr_stream *xdr, struct page **pages,
unsigned int base, unsigned int len);
diff --git a/net/sunrpc/auth_gss/svcauth_gss.c b/net/sunrpc/auth_gss/svcauth_gss.c
index 33fe307372d0..d049db997ab7 100644
--- a/net/sunrpc/auth_gss/svcauth_gss.c
+++ b/net/sunrpc/auth_gss/svcauth_gss.c
@@ -904,13 +904,14 @@ EXPORT_SYMBOL_GPL(svcauth_gss_register_pseudoflavor);
* proc_req_arg_t arg;
* };
*/
-static int
-svcauth_gss_unwrap_integ(struct svc_rqst *rqstp, struct xdr_buf *buf, u32 seq,
- struct gss_ctx *ctx)
+static noinline_for_stack int
+svcauth_gss_unwrap_integ(struct svc_rqst *rqstp, u32 seq, struct gss_ctx *ctx)
{
struct gss_svc_data *gsd = rqstp->rq_auth_data;
+ struct xdr_stream *xdr = &rqstp->rq_arg_stream;
+ u32 len, offset, seq_num, maj_stat;
+ struct xdr_buf *buf = xdr->buf;
struct xdr_buf databody_integ;
- u32 len, seq_num, maj_stat;
struct xdr_netobj checksum;
/* NFS READ normally uses splice to send data in-place. However
@@ -925,29 +926,43 @@ svcauth_gss_unwrap_integ(struct svc_rqst *rqstp, struct xdr_buf *buf, u32 seq,
if (rqstp->rq_deferred)
return 0;
- len = svc_getnl(&buf->head[0]);
- if (len & 3)
+ if (xdr_stream_decode_u32(xdr, &len) < 0)
goto unwrap_failed;
- if (len > buf->len)
+ if (len & 3)
goto unwrap_failed;
- if (xdr_buf_subsegment(buf, &databody_integ, 0, len))
+ offset = xdr_stream_pos(xdr);
+ if (xdr_buf_subsegment(buf, &databody_integ, offset, len))
goto unwrap_failed;
- if (xdr_decode_word(buf, len, &checksum.len))
+ /*
+ * The xdr_stream now points to the @seq_num field. The next
+ * XDR data item is the @arg field, which contains the clear
+ * text RPC program payload. The checksum, which follows the
+ * @arg field, is located and decoded without updating the
+ * xdr_stream.
+ */
+
+ offset += len;
+ if (xdr_decode_word(buf, offset, &checksum.len))
goto unwrap_failed;
if (checksum.len > sizeof(gsd->gsd_scratch))
goto unwrap_failed;
checksum.data = gsd->gsd_scratch;
- if (read_bytes_from_xdr_buf(buf, len + 4, checksum.data, checksum.len))
+ if (read_bytes_from_xdr_buf(buf, offset + XDR_UNIT, checksum.data,
+ checksum.len))
goto unwrap_failed;
+
maj_stat = gss_verify_mic(ctx, &databody_integ, &checksum);
if (maj_stat != GSS_S_COMPLETE)
goto bad_mic;
- seq_num = svc_getnl(&buf->head[0]);
+
+ /* The received seqno is protected by the checksum. */
+ if (xdr_stream_decode_u32(xdr, &seq_num) < 0)
+ goto unwrap_failed;
if (seq_num != seq)
goto bad_seqno;
- /* trim off the mic and padding at the end before returning */
- xdr_buf_trim(buf, round_up_to_quad(checksum.len) + 4);
+
+ xdr_truncate_decode(xdr, XDR_UNIT + checksum.len);
return 0;
unwrap_failed:
@@ -1652,11 +1667,11 @@ svcauth_gss_accept(struct svc_rqst *rqstp)
/* placeholders for length and seq. number: */
svc_putnl(resv, 0);
svc_putnl(resv, 0);
- if (svcauth_gss_unwrap_integ(rqstp, &rqstp->rq_arg,
- gc->gc_seq, rsci->mechctx))
+ svcxdr_init_decode(rqstp);
+ if (svcauth_gss_unwrap_integ(rqstp, gc->gc_seq,
+ rsci->mechctx))
goto garbage_args;
rqstp->rq_auth_slack = RPC_MAX_AUTH_SIZE;
- svcxdr_init_decode(rqstp);
break;
case RPC_GSS_SVC_PRIVACY:
/* placeholders for length and seq. number: */
diff --git a/net/sunrpc/xdr.c b/net/sunrpc/xdr.c
index 4845ba2113fd..c7e89921d511 100644
--- a/net/sunrpc/xdr.c
+++ b/net/sunrpc/xdr.c
@@ -1192,6 +1192,21 @@ void xdr_truncate_encode(struct xdr_stream *xdr, size_t len)
}
EXPORT_SYMBOL(xdr_truncate_encode);
+/**
+ * xdr_truncate_decode - Truncate a decoding stream
+ * @xdr: pointer to struct xdr_stream
+ * @len: Number of bytes to remove
+ *
+ */
+void xdr_truncate_decode(struct xdr_stream *xdr, size_t len)
+{
+ unsigned int nbytes = xdr_align_size(len);
+
+ xdr->buf->len -= nbytes;
+ xdr->nwords -= XDR_QUADLEN(nbytes);
+}
+EXPORT_SYMBOL_GPL(xdr_truncate_decode);
+
/**
* xdr_restrict_buflen - decrease available buffer space
* @xdr: pointer to xdr_stream
From: Chuck Lever <[email protected]>
Micro-optimizations:
1. The value of rqstp->rq_auth_stat is replaced no matter which
arm of the switch is taken, so the initial assignment can be
safely removed.
2. Avoid checking the value of gc->gc_proc twice in the I/O
(RPC_GSS_PROC_DATA) path.
The cost is a little extra code redundancy.
Signed-off-by: Chuck Lever <[email protected]>
---
net/sunrpc/auth_gss/svcauth_gss.c | 14 +++++++++-----
1 file changed, 9 insertions(+), 5 deletions(-)
diff --git a/net/sunrpc/auth_gss/svcauth_gss.c b/net/sunrpc/auth_gss/svcauth_gss.c
index 8362fc143754..363c25198547 100644
--- a/net/sunrpc/auth_gss/svcauth_gss.c
+++ b/net/sunrpc/auth_gss/svcauth_gss.c
@@ -1608,17 +1608,19 @@ svcauth_gss_accept(struct svc_rqst *rqstp)
if (crlen != round_up_to_quad(gc->gc_ctx.len) + 5 * 4)
goto auth_err;
- if ((gc->gc_proc != RPC_GSS_PROC_DATA) && (rqstp->rq_proc != 0))
- goto auth_err;
-
svcxdr_init_decode(rqstp);
- rqstp->rq_auth_stat = rpc_autherr_badverf;
+
switch (gc->gc_proc) {
case RPC_GSS_PROC_INIT:
case RPC_GSS_PROC_CONTINUE_INIT:
+ if (rqstp->rq_proc != 0)
+ goto auth_err;
return svcauth_gss_proc_init(rqstp, gc);
- case RPC_GSS_PROC_DATA:
case RPC_GSS_PROC_DESTROY:
+ if (rqstp->rq_proc != 0)
+ goto auth_err;
+ fallthrough;
+ case RPC_GSS_PROC_DATA:
/* Look up the context, and check the verifier: */
rqstp->rq_auth_stat = rpcsec_gsserr_credproblem;
rsci = gss_svc_searchbyctx(sn->rsc_cache, &gc->gc_ctx);
@@ -1634,6 +1636,8 @@ svcauth_gss_accept(struct svc_rqst *rqstp)
}
break;
default:
+ if (rqstp->rq_proc != 0)
+ goto auth_err;
rqstp->rq_auth_stat = rpc_autherr_rejectedcred;
goto auth_err;
}
From: Chuck Lever <[email protected]>
Now that all vs_dispatch functions invoke svcxdr_init_decode(), it
is common code and can be pushed down into the generic RPC server.
Signed-off-by: Chuck Lever <[email protected]>
---
fs/lockd/svc.c | 1 -
fs/nfs/callback_xdr.c | 1 -
fs/nfsd/nfssvc.c | 1 -
net/sunrpc/svc.c | 1 +
4 files changed, 1 insertion(+), 3 deletions(-)
diff --git a/fs/lockd/svc.c b/fs/lockd/svc.c
index 59ef8a1f843f..e56d85335599 100644
--- a/fs/lockd/svc.c
+++ b/fs/lockd/svc.c
@@ -695,7 +695,6 @@ static int nlmsvc_dispatch(struct svc_rqst *rqstp, __be32 *statp)
{
const struct svc_procedure *procp = rqstp->rq_procinfo;
- svcxdr_init_decode(rqstp);
if (!procp->pc_decode(rqstp, &rqstp->rq_arg_stream))
goto out_decode_err;
diff --git a/fs/nfs/callback_xdr.c b/fs/nfs/callback_xdr.c
index d0cccddb7d08..46d3f5986b4e 100644
--- a/fs/nfs/callback_xdr.c
+++ b/fs/nfs/callback_xdr.c
@@ -984,7 +984,6 @@ nfs_callback_dispatch(struct svc_rqst *rqstp, __be32 *statp)
{
const struct svc_procedure *procp = rqstp->rq_procinfo;
- svcxdr_init_decode(rqstp);
svcxdr_init_encode(rqstp);
*statp = procp->pc_func(rqstp);
diff --git a/fs/nfsd/nfssvc.c b/fs/nfsd/nfssvc.c
index 56fba1cba3af..5375a28b102a 100644
--- a/fs/nfsd/nfssvc.c
+++ b/fs/nfsd/nfssvc.c
@@ -1040,7 +1040,6 @@ int nfsd_dispatch(struct svc_rqst *rqstp, __be32 *statp)
*/
rqstp->rq_cachetype = proc->pc_cachetype;
- svcxdr_init_decode(rqstp);
if (!proc->pc_decode(rqstp, &rqstp->rq_arg_stream))
goto out_decode_err;
diff --git a/net/sunrpc/svc.c b/net/sunrpc/svc.c
index 85f0c3cfc877..96806c3c61a5 100644
--- a/net/sunrpc/svc.c
+++ b/net/sunrpc/svc.c
@@ -1302,6 +1302,7 @@ svc_process_common(struct svc_rqst *rqstp, struct kvec *argv, struct kvec *resv)
if (progp == NULL)
goto err_bad_prog;
+ svcxdr_init_decode(rqstp);
rpc_stat = progp->pg_init_request(rqstp, progp, &process);
switch (rpc_stat) {
case rpc_success:
From: Chuck Lever <[email protected]>
Clean up: Group these together for legibility.
Signed-off-by: Chuck Lever <[email protected]>
---
net/sunrpc/svc.c | 10 +++-------
1 file changed, 3 insertions(+), 7 deletions(-)
diff --git a/net/sunrpc/svc.c b/net/sunrpc/svc.c
index e4f8a177763e..e473c19b2f8d 100644
--- a/net/sunrpc/svc.c
+++ b/net/sunrpc/svc.c
@@ -1248,19 +1248,15 @@ svc_process_common(struct svc_rqst *rqstp, struct kvec *argv, struct kvec *resv)
__set_bit(RQ_USEDEFERRAL, &rqstp->rq_flags);
__clear_bit(RQ_DROPME, &rqstp->rq_flags);
+ /* Construct the first words of the reply: */
svc_putu32(resv, rqstp->rq_xid);
+ svc_putnl(resv, RPC_REPLY);
+ reply_statp = resv->iov_base + resv->iov_len;
vers = svc_getnl(argv);
-
- /* First words of reply: */
- svc_putnl(resv, 1); /* REPLY */
-
if (vers != 2) /* RPC version number */
goto err_bad_rpc;
- /* Save position in case we later decide to reject: */
- reply_statp = resv->iov_base + resv->iov_len;
-
svc_putnl(resv, 0); /* ACCEPT */
rqstp->rq_prog = prog = svc_getnl(argv); /* program number */
From: Chuck Lever <[email protected]>
gss_read_common_verf() is now just a wrapper for dup_netobj(), thus
it can be replaced with direct calls to dup_netobj().
Signed-off-by: Chuck Lever <[email protected]>
---
net/sunrpc/auth_gss/svcauth_gss.c | 26 +++++---------------------
1 file changed, 5 insertions(+), 21 deletions(-)
diff --git a/net/sunrpc/auth_gss/svcauth_gss.c b/net/sunrpc/auth_gss/svcauth_gss.c
index ba32c05f0258..5da4a3d48a7d 100644
--- a/net/sunrpc/auth_gss/svcauth_gss.c
+++ b/net/sunrpc/auth_gss/svcauth_gss.c
@@ -1090,18 +1090,6 @@ gss_write_init_verf(struct cache_detail *cd, struct svc_rqst *rqstp,
return rc;
}
-static inline int
-gss_read_common_verf(struct rpc_gss_wire_cred *gc,
- struct kvec *argv, __be32 *authp,
- struct xdr_netobj *in_handle)
-{
- if (dup_netobj(in_handle, &gc->gc_ctx))
- return SVC_CLOSE;
- *authp = rpc_autherr_badverf;
-
- return 0;
-}
-
static inline int
gss_read_verf(struct rpc_gss_wire_cred *gc,
struct kvec *argv, __be32 *authp,
@@ -1109,12 +1097,9 @@ gss_read_verf(struct rpc_gss_wire_cred *gc,
struct xdr_netobj *in_token)
{
struct xdr_netobj tmpobj;
- int res;
-
- res = gss_read_common_verf(gc, argv, authp, in_handle);
- if (res)
- return res;
+ if (dup_netobj(in_handle, &gc->gc_ctx))
+ return SVC_CLOSE;
if (svc_safe_getnetobj(argv, &tmpobj)) {
kfree(in_handle->data);
return SVC_DENIED;
@@ -1151,12 +1136,11 @@ static int gss_read_proxy_verf(struct svc_rqst *rqstp,
{
struct kvec *argv = &rqstp->rq_arg.head[0];
unsigned int length, pgto_offs, pgfrom_offs;
- int pages, i, res, pgto, pgfrom;
size_t inlen, to_offs, from_offs;
+ int pages, i, pgto, pgfrom;
- res = gss_read_common_verf(gc, argv, &rqstp->rq_auth_stat, in_handle);
- if (res)
- return res;
+ if (dup_netobj(in_handle, &gc->gc_ctx))
+ return SVC_CLOSE;
inlen = svc_getnl(argv);
if (inlen > (argv->iov_len + rqstp->rq_arg.page_len)) {
From: Chuck Lever <[email protected]>
Now the entire RPC Call header parsing path is handled via struct
xdr_stream-based decoders.
Signed-off-by: Chuck Lever <[email protected]>
---
net/sunrpc/svc.c | 28 +++++++++++++++++-----------
net/sunrpc/svc_xprt.c | 1 -
2 files changed, 17 insertions(+), 12 deletions(-)
diff --git a/net/sunrpc/svc.c b/net/sunrpc/svc.c
index 571151a17e87..94f7efca60fc 100644
--- a/net/sunrpc/svc.c
+++ b/net/sunrpc/svc.c
@@ -1249,7 +1249,6 @@ svc_process_common(struct svc_rqst *rqstp, struct kvec *resv)
svc_putnl(resv, RPC_REPLY);
reply_statp = resv->iov_base + resv->iov_len;
- svcxdr_init_decode(rqstp);
p = xdr_inline_decode(&rqstp->rq_arg_stream, XDR_UNIT * 4);
if (unlikely(!p))
goto err_short_len;
@@ -1425,9 +1424,8 @@ svc_process_common(struct svc_rqst *rqstp, struct kvec *resv)
int
svc_process(struct svc_rqst *rqstp)
{
- struct kvec *argv = &rqstp->rq_arg.head[0];
struct kvec *resv = &rqstp->rq_res.head[0];
- __be32 dir;
+ __be32 *p;
#if IS_ENABLED(CONFIG_FAIL_SUNRPC)
if (!fail_sunrpc.ignore_server_disconnect &&
@@ -1450,16 +1448,21 @@ svc_process(struct svc_rqst *rqstp)
rqstp->rq_res.tail[0].iov_base = NULL;
rqstp->rq_res.tail[0].iov_len = 0;
- dir = svc_getu32(argv);
- if (dir != rpc_call)
+ svcxdr_init_decode(rqstp);
+ p = xdr_inline_decode(&rqstp->rq_arg_stream, XDR_UNIT * 2);
+ if (unlikely(!p))
+ goto out_drop;
+ rqstp->rq_xid = *p++;
+ if (unlikely(*p != rpc_call))
goto out_baddir;
+
if (!svc_process_common(rqstp, resv))
goto out_drop;
return svc_send(rqstp);
out_baddir:
svc_printk(rqstp, "bad direction 0x%08x, dropping request\n",
- be32_to_cpu(dir));
+ be32_to_cpu(*p));
rqstp->rq_server->sv_stats->rpcbadfmt++;
out_drop:
svc_drop(rqstp);
@@ -1476,7 +1479,6 @@ int
bc_svc_process(struct svc_serv *serv, struct rpc_rqst *req,
struct svc_rqst *rqstp)
{
- struct kvec *argv = &rqstp->rq_arg.head[0];
struct kvec *resv = &rqstp->rq_res.head[0];
struct rpc_task *task;
int proc_error;
@@ -1511,12 +1513,16 @@ bc_svc_process(struct svc_serv *serv, struct rpc_rqst *req,
/* reset result send buffer "put" position */
resv->iov_len = 0;
+ svcxdr_init_decode(rqstp);
+
/*
- * Skip the next two words because they've already been
- * processed in the transport
+ * Skip the XID and calldir fields because they've already
+ * been processed by the caller.
*/
- svc_getu32(argv); /* XID */
- svc_getnl(argv); /* CALLDIR */
+ if (!xdr_inline_decode(&rqstp->rq_arg_stream, XDR_UNIT * 2)) {
+ error = -EINVAL;
+ goto out;
+ }
/* Parse and execute the bc call */
proc_error = svc_process_common(rqstp, resv);
diff --git a/net/sunrpc/svc_xprt.c b/net/sunrpc/svc_xprt.c
index 2106003645a7..b4697984b4b2 100644
--- a/net/sunrpc/svc_xprt.c
+++ b/net/sunrpc/svc_xprt.c
@@ -890,7 +890,6 @@ int svc_recv(struct svc_rqst *rqstp, long timeout)
xprt->xpt_ops->xpo_secure_port(rqstp);
rqstp->rq_chandle.defer = svc_defer;
- rqstp->rq_xid = svc_getu32(&rqstp->rq_arg.head[0]);
if (serv->sv_stats)
serv->sv_stats->netcnt++;
From: Chuck Lever <[email protected]>
Done as part of hardening the server-side RPC header decoding path.
Signed-off-by: Chuck Lever <[email protected]>
---
net/sunrpc/svc.c | 20 +++++++++-----------
1 file changed, 9 insertions(+), 11 deletions(-)
diff --git a/net/sunrpc/svc.c b/net/sunrpc/svc.c
index 3c1574f362fe..f292b898f200 100644
--- a/net/sunrpc/svc.c
+++ b/net/sunrpc/svc.c
@@ -1231,17 +1231,13 @@ svc_process_common(struct svc_rqst *rqstp, struct kvec *argv, struct kvec *resv)
const struct svc_procedure *procp = NULL;
struct svc_serv *serv = rqstp->rq_server;
struct svc_process_info process;
- __be32 *statp;
- u32 vers;
+ __be32 *p, *statp;
__be32 rpc_stat;
int auth_res, rc;
__be32 *reply_statp;
rpc_stat = rpc_success;
- if (argv->iov_len < 6*4)
- goto err_short_len;
-
/* Will be turned off by GSS integrity and privacy services */
__set_bit(RQ_SPLICE_OK, &rqstp->rq_flags);
/* Will be turned off only when NFSv4 Sessions are used */
@@ -1253,15 +1249,18 @@ svc_process_common(struct svc_rqst *rqstp, struct kvec *argv, struct kvec *resv)
svc_putnl(resv, RPC_REPLY);
reply_statp = resv->iov_base + resv->iov_len;
- vers = svc_getnl(argv);
- if (vers != 2) /* RPC version number */
+ svcxdr_init_decode(rqstp);
+ p = xdr_inline_decode(&rqstp->rq_arg_stream, XDR_UNIT * 4);
+ if (unlikely(!p))
+ goto err_short_len;
+ if (*p++ != cpu_to_be32(RPC_VERSION))
goto err_bad_rpc;
svc_putnl(resv, 0); /* ACCEPT */
- rqstp->rq_prog = svc_getnl(argv); /* program number */
- rqstp->rq_vers = svc_getnl(argv); /* version number */
- rqstp->rq_proc = svc_getnl(argv); /* procedure number */
+ rqstp->rq_prog = be32_to_cpup(p++);
+ rqstp->rq_vers = be32_to_cpup(p++);
+ rqstp->rq_proc = be32_to_cpup(p);
for (progp = serv->sv_program; progp; progp = progp->pg_next)
if (rqstp->rq_prog == progp->pg_prog)
@@ -1272,7 +1271,6 @@ svc_process_common(struct svc_rqst *rqstp, struct kvec *argv, struct kvec *resv)
* We do this before anything else in order to get a decent
* auth verifier.
*/
- svcxdr_init_decode(rqstp);
auth_res = svc_authenticate(rqstp);
/* Also give the program a chance to reject this call: */
if (auth_res == SVC_OK && progp)
From: Chuck Lever <[email protected]>
The entire RPC_GSS_PROC_INIT path is converted over to xdr_stream
for decoding the Call credential and verifier.
Done as part of hardening the server-side RPC header decoding path.
Signed-off-by: Chuck Lever <[email protected]>
---
net/sunrpc/auth_gss/svcauth_gss.c | 87 +++++++++++++++++++++++++------------
1 file changed, 58 insertions(+), 29 deletions(-)
diff --git a/net/sunrpc/auth_gss/svcauth_gss.c b/net/sunrpc/auth_gss/svcauth_gss.c
index 5b03a97e32b7..8e8dec664a89 100644
--- a/net/sunrpc/auth_gss/svcauth_gss.c
+++ b/net/sunrpc/auth_gss/svcauth_gss.c
@@ -1112,39 +1112,43 @@ static int gss_read_proxy_verf(struct svc_rqst *rqstp,
struct xdr_netobj *in_handle,
struct gssp_in_token *in_token)
{
- struct kvec *argv = &rqstp->rq_arg.head[0];
+ struct xdr_stream *xdr = &rqstp->rq_arg_stream;
unsigned int length, pgto_offs, pgfrom_offs;
- size_t inlen, to_offs, from_offs;
int pages, i, pgto, pgfrom;
+ size_t to_offs, from_offs;
+ u32 inlen;
if (dup_netobj(in_handle, &gc->gc_ctx))
return SVC_CLOSE;
- inlen = svc_getnl(argv);
- if (inlen > (argv->iov_len + rqstp->rq_arg.page_len)) {
- kfree(in_handle->data);
- return SVC_DENIED;
- }
+ /*
+ * RFC 2203 Section 5.2.2
+ *
+ * struct rpc_gss_init_arg {
+ * opaque gss_token<>;
+ * };
+ */
+ if (xdr_stream_decode_u32(xdr, &inlen) < 0)
+ goto out_denied_free;
+ if (inlen > xdr_stream_remaining(xdr))
+ goto out_denied_free;
pages = DIV_ROUND_UP(inlen, PAGE_SIZE);
in_token->pages = kcalloc(pages, sizeof(struct page *), GFP_KERNEL);
- if (!in_token->pages) {
- kfree(in_handle->data);
- return SVC_DENIED;
- }
+ if (!in_token->pages)
+ goto out_denied_free;
in_token->page_base = 0;
in_token->page_len = inlen;
for (i = 0; i < pages; i++) {
in_token->pages[i] = alloc_page(GFP_KERNEL);
if (!in_token->pages[i]) {
- kfree(in_handle->data);
gss_free_in_token_pages(in_token);
- return SVC_DENIED;
+ goto out_denied_free;
}
}
- length = min_t(unsigned int, inlen, argv->iov_len);
- memcpy(page_address(in_token->pages[0]), argv->iov_base, length);
+ length = min_t(unsigned int, inlen, (char *)xdr->end - (char *)xdr->p);
+ memcpy(page_address(in_token->pages[0]), xdr->p, length);
inlen -= length;
to_offs = length;
@@ -1167,6 +1171,10 @@ static int gss_read_proxy_verf(struct svc_rqst *rqstp,
inlen -= length;
}
return 0;
+
+out_denied_free:
+ kfree(in_handle->data);
+ return SVC_DENIED;
}
static inline int
@@ -1196,27 +1204,45 @@ gss_write_resv(struct kvec *resv, size_t size_limit,
* the upcall results are available, write the verifier and result.
* Otherwise, drop the request pending an answer to the upcall.
*/
-static int svcauth_gss_legacy_init(struct svc_rqst *rqstp,
- struct rpc_gss_wire_cred *gc)
+static int
+svcauth_gss_legacy_init(struct svc_rqst *rqstp,
+ struct rpc_gss_wire_cred *gc)
{
- struct kvec *argv = &rqstp->rq_arg.head[0];
+ struct xdr_stream *xdr = &rqstp->rq_arg_stream;
struct kvec *resv = &rqstp->rq_res.head[0];
struct rsi *rsip, rsikey;
- struct xdr_netobj tmpobj;
+ __be32 *p;
+ u32 len;
int ret;
struct sunrpc_net *sn = net_generic(SVC_NET(rqstp), sunrpc_net_id);
memset(&rsikey, 0, sizeof(rsikey));
if (dup_netobj(&rsikey.in_handle, &gc->gc_ctx))
return SVC_CLOSE;
- if (svc_safe_getnetobj(argv, &tmpobj)) {
+
+ /*
+ * RFC 2203 Section 5.2.2
+ *
+ * struct rpc_gss_init_arg {
+ * opaque gss_token<>;
+ * };
+ */
+ if (xdr_stream_decode_u32(xdr, &len) < 0) {
+ kfree(rsikey.in_handle.data);
+ return SVC_DENIED;
+ }
+ p = xdr_inline_decode(xdr, len);
+ if (!p) {
kfree(rsikey.in_handle.data);
return SVC_DENIED;
}
- if (dup_netobj(&rsikey.in_token, &tmpobj)) {
+ rsikey.in_token.data = kmalloc(len, GFP_KERNEL);
+ if (ZERO_OR_NULL_PTR(rsikey.in_token.data)) {
kfree(rsikey.in_handle.data);
return SVC_CLOSE;
}
+ memcpy(rsikey.in_token.data, p, len);
+ rsikey.in_token.len = len;
/* Perform upcall, or find upcall result: */
rsip = rsi_lookup(sn->rsi_cache, &rsikey);
@@ -1237,7 +1263,6 @@ static int svcauth_gss_legacy_init(struct svc_rqst *rqstp,
rsip->major_status, rsip->minor_status))
goto out;
- svcxdr_init_decode(rqstp);
ret = SVC_COMPLETE;
out:
cache_put(&rsip->h, sn->rsi_cache);
@@ -1366,7 +1391,6 @@ static int svcauth_gss_proxy_init(struct svc_rqst *rqstp,
ud.major_status, ud.minor_status))
goto out;
- svcxdr_init_decode(rqstp);
ret = SVC_COMPLETE;
out:
gss_free_in_token_pages(&ud.in_token);
@@ -1404,14 +1428,19 @@ static bool use_gss_proxy(struct net *net)
static noinline_for_stack int
svcauth_gss_proc_init(struct svc_rqst *rqstp, struct rpc_gss_wire_cred *gc)
{
- struct kvec *argv = rqstp->rq_arg.head;
+ struct xdr_stream *xdr = &rqstp->rq_arg_stream;
+ u32 flavor, len;
+ void *body;
- if (argv->iov_len < 2 * 4)
- return SVC_DENIED;
- if (svc_getnl(argv) != RPC_AUTH_NULL)
- return SVC_DENIED;
- if (svc_getnl(argv) != 0)
+ svcxdr_init_decode(rqstp);
+
+ /* Call's verf field: */
+ if (xdr_stream_decode_opaque_auth(xdr, &flavor, &body, &len) < 0)
+ return SVC_GARBAGE;
+ if (flavor != RPC_AUTH_NULL || len != 0) {
+ rqstp->rq_auth_stat = rpc_autherr_badverf;
return SVC_DENIED;
+ }
if (gc->gc_proc == RPC_GSS_PROC_INIT && gc->gc_ctx.len != 0) {
rqstp->rq_auth_stat = rpc_autherr_badcred;
On Mon, 2023-01-02 at 12:05 -0500, Chuck Lever wrote:
> Happy new year, campers.
>
> The following series has been percolating for quite a while, thanks
> to the extended 6.1-rc cycle. I'd like to get this work reviewed
> for possible inclusion in v6.3, so I'm starting early.
>
> The purpose of this series is to replace the svc_get* macros in the
> Linux kernel server's RPC call header parsing code with xdr_stream
> helpers. I've measured no change in CPU utilization after the
> overhaul; svc_recv() and friends remain the highest CPU consumers by
> an order of magnitude.
>
> Memory safety: Buffer bounds checking after decoding each XDR item
> is more memory-safe than the current decoding mechanism. Subsequent
> memory safety improvements to the xdr_stream helpers will benefit
> all who use them.
>
> Audit friendliness: The new code has lots of comments and other
> clean-up to help align it with the relevant RPC specifications. The
> use of common helpers also makes the decoders easier to audit.
>
> I've split the full series in half to make it easier to review. The
> patches posted here handle RPC call header decoding. Remaining
> patches, to be posted later, deal with RPC reply header encoding.
>
> Yes, there are a lot of patches, but they are each small, easily
> chewed mechanical changes.
>
> ---
>
> Chuck Lever (25):
> SUNRPC: Push svcxdr_init_decode() into svc_process_common()
> SUNRPC: Move svcxdr_init_decode() into ->accept methods
> SUNRPC: Add an XDR decoding helper for struct opaque_auth
> SUNRPC: Convert svcauth_null_accept() to use xdr_stream
> SUNRPC: Convert svcauth_unix_accept() to use xdr_stream
> SUNRPC: Convert svcauth_tls_accept() to use xdr_stream
> SUNRPC: Move the server-side GSS upcall to a noinline function
> SUNRPC: Hoist common verifier decoding code into svcauth_gss_proc_init()
> SUNRPC: Remove gss_read_common_verf()
> SUNRPC: Remove gss_read_verf()
> SUNRPC: Convert server-side GSS upcall helpers to use xdr_stream
> SUNRPC: Replace read_u32_from_xdr_buf() with existing XDR helper
> SUNRPC: Rename automatic variables in unwrap_integ_data()
> SUNRPC: Convert unwrap_integ_data() to use xdr_stream
> SUNRPC: Rename automatic variables in unwrap_priv_data()
> SUNRPC: Convert unwrap_priv_data() to use xdr_stream
> SUNRPC: Convert gss_verify_header() to use xdr_stream
> SUNRPC: Clean up svcauth_gss_accept's NULL procedure check
> SUNRPC: Convert the svcauth_gss_accept() pre-amble to use xdr_stream
> SUNRPC: Hoist init_decode out of svc_authenticate()
> SUNRPC: Re-order construction of the first reply fields
> SUNRPC: Eliminate unneeded variable
> SUNRPC: Decode most of RPC header with xdr_stream
> SUNRPC: Remove svc_process_common's argv parameter
> SUNRPC: Hoist svcxdr_init_decode() into svc_process()
>
>
> fs/lockd/svc.c | 1 -
> fs/nfs/callback_xdr.c | 1 -
> fs/nfsd/nfssvc.c | 1 -
> include/linux/sunrpc/msg_prot.h | 5 +
> include/linux/sunrpc/xdr.h | 5 +-
> net/sunrpc/auth_gss/svcauth_gss.c | 512 ++++++++++++++++--------------
> net/sunrpc/svc.c | 69 ++--
> net/sunrpc/svc_xprt.c | 1 -
> net/sunrpc/svcauth.c | 13 +-
> net/sunrpc/svcauth_unix.c | 132 +++++---
> net/sunrpc/xdr.c | 50 ++-
> 11 files changed, 468 insertions(+), 322 deletions(-)
>
> --
> Chuck Lever
>
I went through the whole set and it's all a mostly logical set of
methodical changes. You can add:
Reviewed-by: Jeff Layton <[email protected]>
> On Jan 3, 2023, at 9:52 AM, Jeff Layton <[email protected]> wrote:
>
> On Mon, 2023-01-02 at 12:05 -0500, Chuck Lever wrote:
>> Happy new year, campers.
>>
>> The following series has been percolating for quite a while, thanks
>> to the extended 6.1-rc cycle. I'd like to get this work reviewed
>> for possible inclusion in v6.3, so I'm starting early.
>>
>> The purpose of this series is to replace the svc_get* macros in the
>> Linux kernel server's RPC call header parsing code with xdr_stream
>> helpers. I've measured no change in CPU utilization after the
>> overhaul; svc_recv() and friends remain the highest CPU consumers by
>> an order of magnitude.
>>
>> Memory safety: Buffer bounds checking after decoding each XDR item
>> is more memory-safe than the current decoding mechanism. Subsequent
>> memory safety improvements to the xdr_stream helpers will benefit
>> all who use them.
>>
>> Audit friendliness: The new code has lots of comments and other
>> clean-up to help align it with the relevant RPC specifications. The
>> use of common helpers also makes the decoders easier to audit.
>>
>> I've split the full series in half to make it easier to review. The
>> patches posted here handle RPC call header decoding. Remaining
>> patches, to be posted later, deal with RPC reply header encoding.
>>
>> Yes, there are a lot of patches, but they are each small, easily
>> chewed mechanical changes.
>>
>> ---
>>
>> Chuck Lever (25):
>> SUNRPC: Push svcxdr_init_decode() into svc_process_common()
>> SUNRPC: Move svcxdr_init_decode() into ->accept methods
>> SUNRPC: Add an XDR decoding helper for struct opaque_auth
>> SUNRPC: Convert svcauth_null_accept() to use xdr_stream
>> SUNRPC: Convert svcauth_unix_accept() to use xdr_stream
>> SUNRPC: Convert svcauth_tls_accept() to use xdr_stream
>> SUNRPC: Move the server-side GSS upcall to a noinline function
>> SUNRPC: Hoist common verifier decoding code into svcauth_gss_proc_init()
>> SUNRPC: Remove gss_read_common_verf()
>> SUNRPC: Remove gss_read_verf()
>> SUNRPC: Convert server-side GSS upcall helpers to use xdr_stream
>> SUNRPC: Replace read_u32_from_xdr_buf() with existing XDR helper
>> SUNRPC: Rename automatic variables in unwrap_integ_data()
>> SUNRPC: Convert unwrap_integ_data() to use xdr_stream
>> SUNRPC: Rename automatic variables in unwrap_priv_data()
>> SUNRPC: Convert unwrap_priv_data() to use xdr_stream
>> SUNRPC: Convert gss_verify_header() to use xdr_stream
>> SUNRPC: Clean up svcauth_gss_accept's NULL procedure check
>> SUNRPC: Convert the svcauth_gss_accept() pre-amble to use xdr_stream
>> SUNRPC: Hoist init_decode out of svc_authenticate()
>> SUNRPC: Re-order construction of the first reply fields
>> SUNRPC: Eliminate unneeded variable
>> SUNRPC: Decode most of RPC header with xdr_stream
>> SUNRPC: Remove svc_process_common's argv parameter
>> SUNRPC: Hoist svcxdr_init_decode() into svc_process()
>>
>>
>> fs/lockd/svc.c | 1 -
>> fs/nfs/callback_xdr.c | 1 -
>> fs/nfsd/nfssvc.c | 1 -
>> include/linux/sunrpc/msg_prot.h | 5 +
>> include/linux/sunrpc/xdr.h | 5 +-
>> net/sunrpc/auth_gss/svcauth_gss.c | 512 ++++++++++++++++--------------
>> net/sunrpc/svc.c | 69 ++--
>> net/sunrpc/svc_xprt.c | 1 -
>> net/sunrpc/svcauth.c | 13 +-
>> net/sunrpc/svcauth_unix.c | 132 +++++---
>> net/sunrpc/xdr.c | 50 ++-
>> 11 files changed, 468 insertions(+), 322 deletions(-)
>>
>> --
>> Chuck Lever
>>
>
> I went through the whole set and it's all a mostly logical set of
> methodical changes. You can add:
>
> Reviewed-by: Jeff Layton <[email protected]>
Thanks Jeff! I've applied these and pushed them to nfsd's for-next,
but comments are still open, and testing results are welcome.
--
Chuck Lever