* [PATCH 1/4] rpc: clean up decoding of gssproxy linux creds
2013-09-05 15:49 gss-proxy fix for large kmalloc J. Bruce Fields
@ 2013-09-05 15:49 ` J. Bruce Fields
2013-09-05 15:49 ` [PATCH 2/4] rpc: comment on linux_cred encoding, treat all as unsigned J. Bruce Fields
` (2 subsequent siblings)
3 siblings, 0 replies; 8+ messages in thread
From: J. Bruce Fields @ 2013-09-05 15:49 UTC (permalink / raw)
To: linux-nfs; +Cc: J. Bruce Fields
From: "J. Bruce Fields" <bfields@redhat.com>
We can use the normal coding infrastructure here.
Two minor behavior changes:
- we're assuming no wasted space at the end of the linux cred.
That seems to match gss-proxy's behavior, and I can't see why
it would need to do differently in the future.
- NGROUPS_MAX check added: note groups_alloc doesn't do this,
this is the caller's responsibility.
Signed-off-by: J. Bruce Fields <bfields@redhat.com>
---
net/sunrpc/auth_gss/gss_rpc_xdr.c | 32 +++++++++++++-------------------
1 file changed, 13 insertions(+), 19 deletions(-)
diff --git a/net/sunrpc/auth_gss/gss_rpc_xdr.c b/net/sunrpc/auth_gss/gss_rpc_xdr.c
index 3c85d1c..f5067b2 100644
--- a/net/sunrpc/auth_gss/gss_rpc_xdr.c
+++ b/net/sunrpc/auth_gss/gss_rpc_xdr.c
@@ -166,14 +166,14 @@ static int dummy_dec_opt_array(struct xdr_stream *xdr,
return 0;
}
-static int get_s32(void **p, void *max, s32 *res)
+static int get_s32(struct xdr_stream *xdr, s32 *res)
{
- void *base = *p;
- void *next = (void *)((char *)base + sizeof(s32));
- if (unlikely(next > max || next < base))
+ __be32 *p;
+
+ p = xdr_inline_decode(xdr, 4);
+ if (!p)
return -EINVAL;
- memcpy(res, base, sizeof(s32));
- *p = next;
+ memcpy(res, p, sizeof(s32));
return 0;
}
@@ -182,7 +182,6 @@ static int gssx_dec_linux_creds(struct xdr_stream *xdr,
{
u32 length;
__be32 *p;
- void *q, *end;
s32 tmp;
int N, i, err;
@@ -192,33 +191,28 @@ static int gssx_dec_linux_creds(struct xdr_stream *xdr,
length = be32_to_cpup(p);
- /* FIXME: we do not want to use the scratch buffer for this one
- * may need to use functions that allows us to access an io vector
- * directly */
- p = xdr_inline_decode(xdr, length);
- if (unlikely(p == NULL))
+ if (length > (3 + NGROUPS_MAX) * sizeof(u32))
return -ENOSPC;
- q = p;
- end = q + length;
-
/* uid */
- err = get_s32(&q, end, &tmp);
+ err = get_s32(xdr, &tmp);
if (err)
return err;
creds->cr_uid = make_kuid(&init_user_ns, tmp);
/* gid */
- err = get_s32(&q, end, &tmp);
+ err = get_s32(xdr, &tmp);
if (err)
return err;
creds->cr_gid = make_kgid(&init_user_ns, tmp);
/* number of additional gid's */
- err = get_s32(&q, end, &tmp);
+ err = get_s32(xdr, &tmp);
if (err)
return err;
N = tmp;
+ if ((3 + N) * sizeof(u32) != length)
+ return -EINVAL;
creds->cr_group_info = groups_alloc(N);
if (creds->cr_group_info == NULL)
return -ENOMEM;
@@ -226,7 +220,7 @@ static int gssx_dec_linux_creds(struct xdr_stream *xdr,
/* gid's */
for (i = 0; i < N; i++) {
kgid_t kgid;
- err = get_s32(&q, end, &tmp);
+ err = get_s32(xdr, &tmp);
if (err)
goto out_free_groups;
err = -EINVAL;
--
1.7.9.5
^ permalink raw reply related [flat|nested] 8+ messages in thread* [PATCH 2/4] rpc: comment on linux_cred encoding, treat all as unsigned
2013-09-05 15:49 gss-proxy fix for large kmalloc J. Bruce Fields
2013-09-05 15:49 ` [PATCH 1/4] rpc: clean up decoding of gssproxy linux creds J. Bruce Fields
@ 2013-09-05 15:49 ` J. Bruce Fields
2013-09-05 15:49 ` [PATCH 3/4] rpc: fix huge kmalloc's in gss-proxy J. Bruce Fields
2013-09-05 15:49 ` [PATCH 4/4] rpc: let xdr layer allocate gssproxy receieve pages J. Bruce Fields
3 siblings, 0 replies; 8+ messages in thread
From: J. Bruce Fields @ 2013-09-05 15:49 UTC (permalink / raw)
To: linux-nfs; +Cc: J. Bruce Fields
From: "J. Bruce Fields" <bfields@redhat.com>
The encoding of linux creds is a bit confusing.
Also: I think in practice it doesn't really matter whether we treat any
of these things as signed or unsigned, but unsigned seems more
straightforward: uid_t/gid_t are unsigned and it simplifies the ngroups
overflow check.
Signed-off-by: J. Bruce Fields <bfields@redhat.com>
---
net/sunrpc/auth_gss/gss_rpc_xdr.c | 18 ++++++++++--------
1 file changed, 10 insertions(+), 8 deletions(-)
diff --git a/net/sunrpc/auth_gss/gss_rpc_xdr.c b/net/sunrpc/auth_gss/gss_rpc_xdr.c
index f5067b2..3c19c7d 100644
--- a/net/sunrpc/auth_gss/gss_rpc_xdr.c
+++ b/net/sunrpc/auth_gss/gss_rpc_xdr.c
@@ -166,14 +166,15 @@ static int dummy_dec_opt_array(struct xdr_stream *xdr,
return 0;
}
-static int get_s32(struct xdr_stream *xdr, s32 *res)
+static int get_host_u32(struct xdr_stream *xdr, u32 *res)
{
__be32 *p;
p = xdr_inline_decode(xdr, 4);
if (!p)
return -EINVAL;
- memcpy(res, p, sizeof(s32));
+ /* Contents of linux creds are all host-endian: */
+ memcpy(res, p, sizeof(u32));
return 0;
}
@@ -182,8 +183,9 @@ static int gssx_dec_linux_creds(struct xdr_stream *xdr,
{
u32 length;
__be32 *p;
- s32 tmp;
- int N, i, err;
+ u32 tmp;
+ u32 N;
+ int i, err;
p = xdr_inline_decode(xdr, 4);
if (unlikely(p == NULL))
@@ -195,19 +197,19 @@ static int gssx_dec_linux_creds(struct xdr_stream *xdr,
return -ENOSPC;
/* uid */
- err = get_s32(xdr, &tmp);
+ err = get_host_u32(xdr, &tmp);
if (err)
return err;
creds->cr_uid = make_kuid(&init_user_ns, tmp);
/* gid */
- err = get_s32(xdr, &tmp);
+ err = get_host_u32(xdr, &tmp);
if (err)
return err;
creds->cr_gid = make_kgid(&init_user_ns, tmp);
/* number of additional gid's */
- err = get_s32(xdr, &tmp);
+ err = get_host_u32(xdr, &tmp);
if (err)
return err;
N = tmp;
@@ -220,7 +222,7 @@ static int gssx_dec_linux_creds(struct xdr_stream *xdr,
/* gid's */
for (i = 0; i < N; i++) {
kgid_t kgid;
- err = get_s32(xdr, &tmp);
+ err = get_host_u32(xdr, &tmp);
if (err)
goto out_free_groups;
err = -EINVAL;
--
1.7.9.5
^ permalink raw reply related [flat|nested] 8+ messages in thread* [PATCH 3/4] rpc: fix huge kmalloc's in gss-proxy
2013-09-05 15:49 gss-proxy fix for large kmalloc J. Bruce Fields
2013-09-05 15:49 ` [PATCH 1/4] rpc: clean up decoding of gssproxy linux creds J. Bruce Fields
2013-09-05 15:49 ` [PATCH 2/4] rpc: comment on linux_cred encoding, treat all as unsigned J. Bruce Fields
@ 2013-09-05 15:49 ` J. Bruce Fields
2013-09-05 15:49 ` [PATCH 4/4] rpc: let xdr layer allocate gssproxy receieve pages J. Bruce Fields
3 siblings, 0 replies; 8+ messages in thread
From: J. Bruce Fields @ 2013-09-05 15:49 UTC (permalink / raw)
To: linux-nfs; +Cc: J. Bruce Fields
From: "J. Bruce Fields" <bfields@redhat.com>
The reply to a gssproxy can include up to NGROUPS_MAX gid's, which will
take up more than a page. We therefore need to allocate an array of
pages to hold the reply instead of trying to allocate a single huge
buffer.
Signed-off-by: J. Bruce Fields <bfields@redhat.com>
---
net/sunrpc/auth_gss/gss_rpc_upcall.c | 30 ++++++++++++++++++++++++++++++
net/sunrpc/auth_gss/gss_rpc_xdr.c | 3 +++
net/sunrpc/auth_gss/gss_rpc_xdr.h | 5 ++++-
3 files changed, 37 insertions(+), 1 deletion(-)
diff --git a/net/sunrpc/auth_gss/gss_rpc_upcall.c b/net/sunrpc/auth_gss/gss_rpc_upcall.c
index af7ffd4..be95af3 100644
--- a/net/sunrpc/auth_gss/gss_rpc_upcall.c
+++ b/net/sunrpc/auth_gss/gss_rpc_upcall.c
@@ -213,6 +213,30 @@ static int gssp_call(struct net *net, struct rpc_message *msg)
return status;
}
+static void gssp_free_receive_pages(struct gssx_arg_accept_sec_context *arg)
+{
+ int i;
+
+ for (i = 0; i < arg->npages && arg->pages[i]; i++)
+ __free_page(arg->pages[i]);
+}
+
+static int gssp_alloc_receive_pages(struct gssx_arg_accept_sec_context *arg)
+{
+ int i;
+
+ arg->npages = DIV_ROUND_UP(NGROUPS_MAX * 4, PAGE_SIZE);
+ arg->pages = kzalloc(arg->npages * sizeof(struct page *), GFP_KERNEL);
+
+ for (i=0; i < arg->npages; i++) {
+ arg->pages[i] = alloc_page(GFP_KERNEL);
+ if (arg->pages[i] == NULL) {
+ gssp_free_receive_pages(arg);
+ return -ENOMEM;
+ }
+ }
+ return 0;
+}
/*
* Public functions
@@ -261,10 +285,16 @@ int gssp_accept_sec_context_upcall(struct net *net,
arg.context_handle = &ctxh;
res.output_token->len = GSSX_max_output_token_sz;
+ ret = gssp_alloc_receive_pages(&arg);
+ if (ret)
+ return ret;
+
/* use nfs/ for targ_name ? */
ret = gssp_call(net, &msg);
+ gssp_free_receive_pages(&arg);
+
/* we need to fetch all data even in case of error so
* that we can free special strctures is they have been allocated */
data->major_status = res.status.major_status;
diff --git a/net/sunrpc/auth_gss/gss_rpc_xdr.c b/net/sunrpc/auth_gss/gss_rpc_xdr.c
index 3c19c7d..f0f78c5 100644
--- a/net/sunrpc/auth_gss/gss_rpc_xdr.c
+++ b/net/sunrpc/auth_gss/gss_rpc_xdr.c
@@ -780,6 +780,9 @@ void gssx_enc_accept_sec_context(struct rpc_rqst *req,
/* arg->options */
err = dummy_enc_opt_array(xdr, &arg->options);
+ xdr_inline_pages(&req->rq_rcv_buf,
+ PAGE_SIZE/2 /* pretty arbitrary */,
+ arg->pages, 0 /* page base */, arg->npages * PAGE_SIZE);
done:
if (err)
dprintk("RPC: gssx_enc_accept_sec_context: %d\n", err);
diff --git a/net/sunrpc/auth_gss/gss_rpc_xdr.h b/net/sunrpc/auth_gss/gss_rpc_xdr.h
index 1c98b27..685a688 100644
--- a/net/sunrpc/auth_gss/gss_rpc_xdr.h
+++ b/net/sunrpc/auth_gss/gss_rpc_xdr.h
@@ -147,6 +147,8 @@ struct gssx_arg_accept_sec_context {
struct gssx_cb *input_cb;
u32 ret_deleg_cred;
struct gssx_option_array options;
+ struct page **pages;
+ unsigned int npages;
};
struct gssx_res_accept_sec_context {
@@ -240,7 +242,8 @@ int gssx_dec_accept_sec_context(struct rpc_rqst *rqstp,
2 * GSSX_max_princ_sz + \
8 + 8 + 4 + 4 + 4)
#define GSSX_max_output_token_sz 1024
-#define GSSX_max_creds_sz (4 + 4 + 4 + NGROUPS_MAX * 4)
+/* grouplist not included; we allocate separate pages for that: */
+#define GSSX_max_creds_sz (4 + 4 + 4 /* + NGROUPS_MAX*4 */)
#define GSSX_RES_accept_sec_context_sz (GSSX_default_status_sz + \
GSSX_default_ctx_sz + \
GSSX_max_output_token_sz + \
--
1.7.9.5
^ permalink raw reply related [flat|nested] 8+ messages in thread* [PATCH 4/4] rpc: let xdr layer allocate gssproxy receieve pages
2013-09-05 15:49 gss-proxy fix for large kmalloc J. Bruce Fields
` (2 preceding siblings ...)
2013-09-05 15:49 ` [PATCH 3/4] rpc: fix huge kmalloc's in gss-proxy J. Bruce Fields
@ 2013-09-05 15:49 ` J. Bruce Fields
3 siblings, 0 replies; 8+ messages in thread
From: J. Bruce Fields @ 2013-09-05 15:49 UTC (permalink / raw)
To: linux-nfs; +Cc: J. Bruce Fields
From: "J. Bruce Fields" <bfields@redhat.com>
In theory the linux cred in a gssproxy reply can include up to
NGROUPS_MAX data, 256K of data. In the common case we expect it to be
shorter. So do as the nfsv3 ACL code does and let the xdr code allocate
the pages as they come in, instead of allocating a lot of pages that
won't typically be used.
Signed-off-by: J. Bruce Fields <bfields@redhat.com>
---
net/sunrpc/auth_gss/gss_rpc_upcall.c | 16 ++++++----------
1 file changed, 6 insertions(+), 10 deletions(-)
diff --git a/net/sunrpc/auth_gss/gss_rpc_upcall.c b/net/sunrpc/auth_gss/gss_rpc_upcall.c
index be95af3..f1eb0d1 100644
--- a/net/sunrpc/auth_gss/gss_rpc_upcall.c
+++ b/net/sunrpc/auth_gss/gss_rpc_upcall.c
@@ -223,18 +223,14 @@ static void gssp_free_receive_pages(struct gssx_arg_accept_sec_context *arg)
static int gssp_alloc_receive_pages(struct gssx_arg_accept_sec_context *arg)
{
- int i;
-
arg->npages = DIV_ROUND_UP(NGROUPS_MAX * 4, PAGE_SIZE);
arg->pages = kzalloc(arg->npages * sizeof(struct page *), GFP_KERNEL);
-
- for (i=0; i < arg->npages; i++) {
- arg->pages[i] = alloc_page(GFP_KERNEL);
- if (arg->pages[i] == NULL) {
- gssp_free_receive_pages(arg);
- return -ENOMEM;
- }
- }
+ /*
+ * XXX: actual pages are allocated by xdr layer in
+ * xdr_partial_copy_from_skb.
+ */
+ if (!arg->pages)
+ return -ENOMEM;
return 0;
}
--
1.7.9.5
^ permalink raw reply related [flat|nested] 8+ messages in thread
* [PATCH 1/4] rpc: clean up decoding of gssproxy linux creds
2013-09-05 16:30 [PATCH 00/12] Implement NFSv4 delegations, take 10 J. Bruce Fields
@ 2013-09-05 16:30 ` J. Bruce Fields
2013-09-05 16:38 ` J. Bruce Fields
0 siblings, 1 reply; 8+ messages in thread
From: J. Bruce Fields @ 2013-09-05 16:30 UTC (permalink / raw)
To: Al Viro; +Cc: linux-nfs, linux-fsdevel, jlayton, Dave Chinner, J. Bruce Fields
From: "J. Bruce Fields" <bfields@redhat.com>
We can use the normal coding infrastructure here.
Two minor behavior changes:
- we're assuming no wasted space at the end of the linux cred.
That seems to match gss-proxy's behavior, and I can't see why
it would need to do differently in the future.
- NGROUPS_MAX check added: note groups_alloc doesn't do this,
this is the caller's responsibility.
Signed-off-by: J. Bruce Fields <bfields@redhat.com>
---
net/sunrpc/auth_gss/gss_rpc_xdr.c | 32 +++++++++++++-------------------
1 file changed, 13 insertions(+), 19 deletions(-)
diff --git a/net/sunrpc/auth_gss/gss_rpc_xdr.c b/net/sunrpc/auth_gss/gss_rpc_xdr.c
index 3c85d1c..f5067b2 100644
--- a/net/sunrpc/auth_gss/gss_rpc_xdr.c
+++ b/net/sunrpc/auth_gss/gss_rpc_xdr.c
@@ -166,14 +166,14 @@ static int dummy_dec_opt_array(struct xdr_stream *xdr,
return 0;
}
-static int get_s32(void **p, void *max, s32 *res)
+static int get_s32(struct xdr_stream *xdr, s32 *res)
{
- void *base = *p;
- void *next = (void *)((char *)base + sizeof(s32));
- if (unlikely(next > max || next < base))
+ __be32 *p;
+
+ p = xdr_inline_decode(xdr, 4);
+ if (!p)
return -EINVAL;
- memcpy(res, base, sizeof(s32));
- *p = next;
+ memcpy(res, p, sizeof(s32));
return 0;
}
@@ -182,7 +182,6 @@ static int gssx_dec_linux_creds(struct xdr_stream *xdr,
{
u32 length;
__be32 *p;
- void *q, *end;
s32 tmp;
int N, i, err;
@@ -192,33 +191,28 @@ static int gssx_dec_linux_creds(struct xdr_stream *xdr,
length = be32_to_cpup(p);
- /* FIXME: we do not want to use the scratch buffer for this one
- * may need to use functions that allows us to access an io vector
- * directly */
- p = xdr_inline_decode(xdr, length);
- if (unlikely(p == NULL))
+ if (length > (3 + NGROUPS_MAX) * sizeof(u32))
return -ENOSPC;
- q = p;
- end = q + length;
-
/* uid */
- err = get_s32(&q, end, &tmp);
+ err = get_s32(xdr, &tmp);
if (err)
return err;
creds->cr_uid = make_kuid(&init_user_ns, tmp);
/* gid */
- err = get_s32(&q, end, &tmp);
+ err = get_s32(xdr, &tmp);
if (err)
return err;
creds->cr_gid = make_kgid(&init_user_ns, tmp);
/* number of additional gid's */
- err = get_s32(&q, end, &tmp);
+ err = get_s32(xdr, &tmp);
if (err)
return err;
N = tmp;
+ if ((3 + N) * sizeof(u32) != length)
+ return -EINVAL;
creds->cr_group_info = groups_alloc(N);
if (creds->cr_group_info == NULL)
return -ENOMEM;
@@ -226,7 +220,7 @@ static int gssx_dec_linux_creds(struct xdr_stream *xdr,
/* gid's */
for (i = 0; i < N; i++) {
kgid_t kgid;
- err = get_s32(&q, end, &tmp);
+ err = get_s32(xdr, &tmp);
if (err)
goto out_free_groups;
err = -EINVAL;
--
1.7.9.5
^ permalink raw reply related [flat|nested] 8+ messages in thread* Re: [PATCH 1/4] rpc: clean up decoding of gssproxy linux creds
2013-09-05 16:30 ` [PATCH 1/4] rpc: clean up decoding of gssproxy linux creds J. Bruce Fields
@ 2013-09-05 16:38 ` J. Bruce Fields
0 siblings, 0 replies; 8+ messages in thread
From: J. Bruce Fields @ 2013-09-05 16:38 UTC (permalink / raw)
To: J. Bruce Fields; +Cc: Al Viro, linux-nfs, linux-fsdevel, jlayton, Dave Chinner
On Thu, Sep 05, 2013 at 12:30:05PM -0400, J. Bruce Fields wrote:
> From: "J. Bruce Fields" <bfields@redhat.com>
>
> We can use the normal coding infrastructure here.
>
> Two minor behavior changes:
>
> - we're assuming no wasted space at the end of the linux cred.
> That seems to match gss-proxy's behavior, and I can't see why
> it would need to do differently in the future.
>
> - NGROUPS_MAX check added: note groups_alloc doesn't do this,
> this is the caller's responsibility.
Ignore the "n/4" patches, apologies, some left-over patches that got
picked up by the glob in my "git send-email 00*" command!
--b.
>
> Signed-off-by: J. Bruce Fields <bfields@redhat.com>
> ---
> net/sunrpc/auth_gss/gss_rpc_xdr.c | 32 +++++++++++++-------------------
> 1 file changed, 13 insertions(+), 19 deletions(-)
>
> diff --git a/net/sunrpc/auth_gss/gss_rpc_xdr.c b/net/sunrpc/auth_gss/gss_rpc_xdr.c
> index 3c85d1c..f5067b2 100644
> --- a/net/sunrpc/auth_gss/gss_rpc_xdr.c
> +++ b/net/sunrpc/auth_gss/gss_rpc_xdr.c
> @@ -166,14 +166,14 @@ static int dummy_dec_opt_array(struct xdr_stream *xdr,
> return 0;
> }
>
> -static int get_s32(void **p, void *max, s32 *res)
> +static int get_s32(struct xdr_stream *xdr, s32 *res)
> {
> - void *base = *p;
> - void *next = (void *)((char *)base + sizeof(s32));
> - if (unlikely(next > max || next < base))
> + __be32 *p;
> +
> + p = xdr_inline_decode(xdr, 4);
> + if (!p)
> return -EINVAL;
> - memcpy(res, base, sizeof(s32));
> - *p = next;
> + memcpy(res, p, sizeof(s32));
> return 0;
> }
>
> @@ -182,7 +182,6 @@ static int gssx_dec_linux_creds(struct xdr_stream *xdr,
> {
> u32 length;
> __be32 *p;
> - void *q, *end;
> s32 tmp;
> int N, i, err;
>
> @@ -192,33 +191,28 @@ static int gssx_dec_linux_creds(struct xdr_stream *xdr,
>
> length = be32_to_cpup(p);
>
> - /* FIXME: we do not want to use the scratch buffer for this one
> - * may need to use functions that allows us to access an io vector
> - * directly */
> - p = xdr_inline_decode(xdr, length);
> - if (unlikely(p == NULL))
> + if (length > (3 + NGROUPS_MAX) * sizeof(u32))
> return -ENOSPC;
>
> - q = p;
> - end = q + length;
> -
> /* uid */
> - err = get_s32(&q, end, &tmp);
> + err = get_s32(xdr, &tmp);
> if (err)
> return err;
> creds->cr_uid = make_kuid(&init_user_ns, tmp);
>
> /* gid */
> - err = get_s32(&q, end, &tmp);
> + err = get_s32(xdr, &tmp);
> if (err)
> return err;
> creds->cr_gid = make_kgid(&init_user_ns, tmp);
>
> /* number of additional gid's */
> - err = get_s32(&q, end, &tmp);
> + err = get_s32(xdr, &tmp);
> if (err)
> return err;
> N = tmp;
> + if ((3 + N) * sizeof(u32) != length)
> + return -EINVAL;
> creds->cr_group_info = groups_alloc(N);
> if (creds->cr_group_info == NULL)
> return -ENOMEM;
> @@ -226,7 +220,7 @@ static int gssx_dec_linux_creds(struct xdr_stream *xdr,
> /* gid's */
> for (i = 0; i < N; i++) {
> kgid_t kgid;
> - err = get_s32(&q, end, &tmp);
> + err = get_s32(xdr, &tmp);
> if (err)
> goto out_free_groups;
> err = -EINVAL;
> --
> 1.7.9.5
>
> --
> 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
^ permalink raw reply [flat|nested] 8+ messages in thread
* [PATCH 1/4] rpc: clean up decoding of gssproxy linux creds
2013-09-19 15:27 gss-proxy fixes for 3.11.x J. Bruce Fields
@ 2013-09-19 15:27 ` J. Bruce Fields
0 siblings, 0 replies; 8+ messages in thread
From: J. Bruce Fields @ 2013-09-19 15:27 UTC (permalink / raw)
To: stable; +Cc: linux-nfs, J. Bruce Fields
From: "J. Bruce Fields" <bfields@redhat.com>
commit 778e512bb1d3315c6b55832248cd30c566c081d7 upstream.
We can use the normal coding infrastructure here.
Two minor behavior changes:
- we're assuming no wasted space at the end of the linux cred.
That seems to match gss-proxy's behavior, and I can't see why
it would need to do differently in the future.
- NGROUPS_MAX check added: note groups_alloc doesn't do this,
this is the caller's responsibility.
Tested-by: Simo Sorce <simo@redhat.com>
Signed-off-by: J. Bruce Fields <bfields@redhat.com>
---
net/sunrpc/auth_gss/gss_rpc_xdr.c | 32 +++++++++++++-------------------
1 file changed, 13 insertions(+), 19 deletions(-)
diff --git a/net/sunrpc/auth_gss/gss_rpc_xdr.c b/net/sunrpc/auth_gss/gss_rpc_xdr.c
index 3c85d1c..f5067b2 100644
--- a/net/sunrpc/auth_gss/gss_rpc_xdr.c
+++ b/net/sunrpc/auth_gss/gss_rpc_xdr.c
@@ -166,14 +166,14 @@ static int dummy_dec_opt_array(struct xdr_stream *xdr,
return 0;
}
-static int get_s32(void **p, void *max, s32 *res)
+static int get_s32(struct xdr_stream *xdr, s32 *res)
{
- void *base = *p;
- void *next = (void *)((char *)base + sizeof(s32));
- if (unlikely(next > max || next < base))
+ __be32 *p;
+
+ p = xdr_inline_decode(xdr, 4);
+ if (!p)
return -EINVAL;
- memcpy(res, base, sizeof(s32));
- *p = next;
+ memcpy(res, p, sizeof(s32));
return 0;
}
@@ -182,7 +182,6 @@ static int gssx_dec_linux_creds(struct xdr_stream *xdr,
{
u32 length;
__be32 *p;
- void *q, *end;
s32 tmp;
int N, i, err;
@@ -192,33 +191,28 @@ static int gssx_dec_linux_creds(struct xdr_stream *xdr,
length = be32_to_cpup(p);
- /* FIXME: we do not want to use the scratch buffer for this one
- * may need to use functions that allows us to access an io vector
- * directly */
- p = xdr_inline_decode(xdr, length);
- if (unlikely(p == NULL))
+ if (length > (3 + NGROUPS_MAX) * sizeof(u32))
return -ENOSPC;
- q = p;
- end = q + length;
-
/* uid */
- err = get_s32(&q, end, &tmp);
+ err = get_s32(xdr, &tmp);
if (err)
return err;
creds->cr_uid = make_kuid(&init_user_ns, tmp);
/* gid */
- err = get_s32(&q, end, &tmp);
+ err = get_s32(xdr, &tmp);
if (err)
return err;
creds->cr_gid = make_kgid(&init_user_ns, tmp);
/* number of additional gid's */
- err = get_s32(&q, end, &tmp);
+ err = get_s32(xdr, &tmp);
if (err)
return err;
N = tmp;
+ if ((3 + N) * sizeof(u32) != length)
+ return -EINVAL;
creds->cr_group_info = groups_alloc(N);
if (creds->cr_group_info == NULL)
return -ENOMEM;
@@ -226,7 +220,7 @@ static int gssx_dec_linux_creds(struct xdr_stream *xdr,
/* gid's */
for (i = 0; i < N; i++) {
kgid_t kgid;
- err = get_s32(&q, end, &tmp);
+ err = get_s32(xdr, &tmp);
if (err)
goto out_free_groups;
err = -EINVAL;
--
1.7.9.5
^ permalink raw reply related [flat|nested] 8+ messages in thread