If a zero length is passed to kmalloc() it returns 0x10, which is
not a valid address. gss_verify_mic() subsequently crashes when it
attempts to dereference that pointer.
Instead of allocating this memory on every call based on an
untrusted size value, use a piece of dynamically-allocated scratch
memory that is always available.
Signed-off-by: Chuck Lever <[email protected]>
---
net/sunrpc/auth_gss/svcauth_gss.c | 55 ++++++++++++++++++++++---------------
1 file changed, 32 insertions(+), 23 deletions(-)
diff --git a/net/sunrpc/auth_gss/svcauth_gss.c b/net/sunrpc/auth_gss/svcauth_gss.c
index 9a5db285d4ae..148bb0a7fa5b 100644
--- a/net/sunrpc/auth_gss/svcauth_gss.c
+++ b/net/sunrpc/auth_gss/svcauth_gss.c
@@ -49,11 +49,36 @@
#include <linux/sunrpc/svcauth.h>
#include <linux/sunrpc/svcauth_gss.h>
#include <linux/sunrpc/cache.h>
+#include <linux/sunrpc/gss_krb5.h>
#include <trace/events/rpcgss.h>
#include "gss_rpc_upcall.h"
+/*
+ * Unfortunately there isn't a maximum checksum size exported via the
+ * GSS API. Manufacture one based on GSS mechanisms supported by this
+ * implementation.
+ */
+#define GSS_MAX_CKSUMSIZE (GSS_KRB5_TOK_HDR_LEN + GSS_KRB5_MAX_CKSUM_LEN)
+
+/*
+ * This value may be increased in the future to accommodate other
+ * usage of the scratch buffer.
+ */
+#define GSS_SCRATCH_SIZE GSS_MAX_CKSUMSIZE
+
+struct gss_svc_data {
+ /* decoded gss client cred: */
+ struct rpc_gss_wire_cred clcred;
+ /* save a pointer to the beginning of the encoded verifier,
+ * for use in encryption/checksumming in svcauth_gss_release: */
+ __be32 *verf_start;
+ struct rsc *rsci;
+
+ /* for temporary results */
+ u8 gsd_scratch[GSS_SCRATCH_SIZE];
+};
/* The rpcsec_init cache is used for mapping RPCSEC_GSS_{,CONT_}INIT requests
* into replies.
@@ -887,13 +912,11 @@ read_u32_from_xdr_buf(struct xdr_buf *buf, int base, u32 *obj)
static int
unwrap_integ_data(struct svc_rqst *rqstp, struct xdr_buf *buf, u32 seq, struct gss_ctx *ctx)
{
+ struct gss_svc_data *gsd = rqstp->rq_auth_data;
u32 integ_len, rseqno, maj_stat;
- int stat = -EINVAL;
struct xdr_netobj mic;
struct xdr_buf integ_buf;
- mic.data = NULL;
-
/* NFS READ normally uses splice to send data in-place. However
* the data in cache can change after the reply's MIC is computed
* but before the RPC reply is sent. To prevent the client from
@@ -917,11 +940,9 @@ unwrap_integ_data(struct svc_rqst *rqstp, struct xdr_buf *buf, u32 seq, struct g
/* copy out mic... */
if (read_u32_from_xdr_buf(buf, integ_len, &mic.len))
goto unwrap_failed;
- if (mic.len > RPC_MAX_AUTH_SIZE)
- goto unwrap_failed;
- mic.data = kmalloc(mic.len, GFP_KERNEL);
- if (!mic.data)
+ if (mic.len > sizeof(gsd->gsd_scratch))
goto unwrap_failed;
+ mic.data = gsd->gsd_scratch;
if (read_bytes_from_xdr_buf(buf, integ_len + 4, mic.data, mic.len))
goto unwrap_failed;
maj_stat = gss_verify_mic(ctx, &integ_buf, &mic);
@@ -932,20 +953,17 @@ unwrap_integ_data(struct svc_rqst *rqstp, struct xdr_buf *buf, u32 seq, struct g
goto bad_seqno;
/* trim off the mic and padding at the end before returning */
xdr_buf_trim(buf, round_up_to_quad(mic.len) + 4);
- stat = 0;
-out:
- kfree(mic.data);
- return stat;
+ return 0;
unwrap_failed:
trace_rpcgss_svc_unwrap_failed(rqstp);
- goto out;
+ return -EINVAL;
bad_seqno:
trace_rpcgss_svc_seqno_bad(rqstp, seq, rseqno);
- goto out;
+ return -EINVAL;
bad_mic:
trace_rpcgss_svc_mic(rqstp, maj_stat);
- goto out;
+ return -EINVAL;
}
static inline int
@@ -1023,15 +1041,6 @@ unwrap_priv_data(struct svc_rqst *rqstp, struct xdr_buf *buf, u32 seq, struct gs
return -EINVAL;
}
-struct gss_svc_data {
- /* decoded gss client cred: */
- struct rpc_gss_wire_cred clcred;
- /* save a pointer to the beginning of the encoded verifier,
- * for use in encryption/checksumming in svcauth_gss_release: */
- __be32 *verf_start;
- struct rsc *rsci;
-};
-
static int
svcauth_gss_set_client(struct svc_rqst *rqstp)
{
> On Nov 27, 2022, at 12:17 PM, Chuck Lever <[email protected]> wrote:
>
> If a zero length is passed to kmalloc() it returns 0x10, which is
> not a valid address. gss_verify_mic() subsequently crashes when it
> attempts to dereference that pointer.
>
> Instead of allocating this memory on every call based on an
> untrusted size value, use a piece of dynamically-allocated scratch
> memory that is always available.
>
> Signed-off-by: Chuck Lever <[email protected]>
> ---
> net/sunrpc/auth_gss/svcauth_gss.c | 55 ++++++++++++++++++++++---------------
> 1 file changed, 32 insertions(+), 23 deletions(-)
I forgot to include this one in yesterday's series, and posted
it separately. Can I add "Reviewed-by: Jeff" to this one too?
> diff --git a/net/sunrpc/auth_gss/svcauth_gss.c b/net/sunrpc/auth_gss/svcauth_gss.c
> index 9a5db285d4ae..148bb0a7fa5b 100644
> --- a/net/sunrpc/auth_gss/svcauth_gss.c
> +++ b/net/sunrpc/auth_gss/svcauth_gss.c
> @@ -49,11 +49,36 @@
> #include <linux/sunrpc/svcauth.h>
> #include <linux/sunrpc/svcauth_gss.h>
> #include <linux/sunrpc/cache.h>
> +#include <linux/sunrpc/gss_krb5.h>
>
> #include <trace/events/rpcgss.h>
>
> #include "gss_rpc_upcall.h"
>
> +/*
> + * Unfortunately there isn't a maximum checksum size exported via the
> + * GSS API. Manufacture one based on GSS mechanisms supported by this
> + * implementation.
> + */
> +#define GSS_MAX_CKSUMSIZE (GSS_KRB5_TOK_HDR_LEN + GSS_KRB5_MAX_CKSUM_LEN)
> +
> +/*
> + * This value may be increased in the future to accommodate other
> + * usage of the scratch buffer.
> + */
> +#define GSS_SCRATCH_SIZE GSS_MAX_CKSUMSIZE
> +
> +struct gss_svc_data {
> + /* decoded gss client cred: */
> + struct rpc_gss_wire_cred clcred;
> + /* save a pointer to the beginning of the encoded verifier,
> + * for use in encryption/checksumming in svcauth_gss_release: */
> + __be32 *verf_start;
> + struct rsc *rsci;
> +
> + /* for temporary results */
> + u8 gsd_scratch[GSS_SCRATCH_SIZE];
> +};
>
> /* The rpcsec_init cache is used for mapping RPCSEC_GSS_{,CONT_}INIT requests
> * into replies.
> @@ -887,13 +912,11 @@ read_u32_from_xdr_buf(struct xdr_buf *buf, int base, u32 *obj)
> static int
> unwrap_integ_data(struct svc_rqst *rqstp, struct xdr_buf *buf, u32 seq, struct gss_ctx *ctx)
> {
> + struct gss_svc_data *gsd = rqstp->rq_auth_data;
> u32 integ_len, rseqno, maj_stat;
> - int stat = -EINVAL;
> struct xdr_netobj mic;
> struct xdr_buf integ_buf;
>
> - mic.data = NULL;
> -
> /* NFS READ normally uses splice to send data in-place. However
> * the data in cache can change after the reply's MIC is computed
> * but before the RPC reply is sent. To prevent the client from
> @@ -917,11 +940,9 @@ unwrap_integ_data(struct svc_rqst *rqstp, struct xdr_buf *buf, u32 seq, struct g
> /* copy out mic... */
> if (read_u32_from_xdr_buf(buf, integ_len, &mic.len))
> goto unwrap_failed;
> - if (mic.len > RPC_MAX_AUTH_SIZE)
> - goto unwrap_failed;
> - mic.data = kmalloc(mic.len, GFP_KERNEL);
> - if (!mic.data)
> + if (mic.len > sizeof(gsd->gsd_scratch))
> goto unwrap_failed;
> + mic.data = gsd->gsd_scratch;
> if (read_bytes_from_xdr_buf(buf, integ_len + 4, mic.data, mic.len))
> goto unwrap_failed;
> maj_stat = gss_verify_mic(ctx, &integ_buf, &mic);
> @@ -932,20 +953,17 @@ unwrap_integ_data(struct svc_rqst *rqstp, struct xdr_buf *buf, u32 seq, struct g
> goto bad_seqno;
> /* trim off the mic and padding at the end before returning */
> xdr_buf_trim(buf, round_up_to_quad(mic.len) + 4);
> - stat = 0;
> -out:
> - kfree(mic.data);
> - return stat;
> + return 0;
>
> unwrap_failed:
> trace_rpcgss_svc_unwrap_failed(rqstp);
> - goto out;
> + return -EINVAL;
> bad_seqno:
> trace_rpcgss_svc_seqno_bad(rqstp, seq, rseqno);
> - goto out;
> + return -EINVAL;
> bad_mic:
> trace_rpcgss_svc_mic(rqstp, maj_stat);
> - goto out;
> + return -EINVAL;
> }
>
> static inline int
> @@ -1023,15 +1041,6 @@ unwrap_priv_data(struct svc_rqst *rqstp, struct xdr_buf *buf, u32 seq, struct gs
> return -EINVAL;
> }
>
> -struct gss_svc_data {
> - /* decoded gss client cred: */
> - struct rpc_gss_wire_cred clcred;
> - /* save a pointer to the beginning of the encoded verifier,
> - * for use in encryption/checksumming in svcauth_gss_release: */
> - __be32 *verf_start;
> - struct rsc *rsci;
> -};
> -
> static int
> svcauth_gss_set_client(struct svc_rqst *rqstp)
> {
>
>
--
Chuck Lever
On Sun, 2022-11-27 at 12:17 -0500, Chuck Lever wrote:
> If a zero length is passed to kmalloc() it returns 0x10, which is
> not a valid address. gss_verify_mic() subsequently crashes when it
> attempts to dereference that pointer.
>
> Instead of allocating this memory on every call based on an
> untrusted size value, use a piece of dynamically-allocated scratch
> memory that is always available.
>
> Signed-off-by: Chuck Lever <[email protected]>
> ---
> net/sunrpc/auth_gss/svcauth_gss.c | 55 ++++++++++++++++++++++---------------
> 1 file changed, 32 insertions(+), 23 deletions(-)
>
> diff --git a/net/sunrpc/auth_gss/svcauth_gss.c b/net/sunrpc/auth_gss/svcauth_gss.c
> index 9a5db285d4ae..148bb0a7fa5b 100644
> --- a/net/sunrpc/auth_gss/svcauth_gss.c
> +++ b/net/sunrpc/auth_gss/svcauth_gss.c
> @@ -49,11 +49,36 @@
> #include <linux/sunrpc/svcauth.h>
> #include <linux/sunrpc/svcauth_gss.h>
> #include <linux/sunrpc/cache.h>
> +#include <linux/sunrpc/gss_krb5.h>
>
> #include <trace/events/rpcgss.h>
>
> #include "gss_rpc_upcall.h"
>
> +/*
> + * Unfortunately there isn't a maximum checksum size exported via the
> + * GSS API. Manufacture one based on GSS mechanisms supported by this
> + * implementation.
> + */
> +#define GSS_MAX_CKSUMSIZE (GSS_KRB5_TOK_HDR_LEN + GSS_KRB5_MAX_CKSUM_LEN)
> +
> +/*
> + * This value may be increased in the future to accommodate other
> + * usage of the scratch buffer.
> + */
> +#define GSS_SCRATCH_SIZE GSS_MAX_CKSUMSIZE
> +
> +struct gss_svc_data {
> + /* decoded gss client cred: */
> + struct rpc_gss_wire_cred clcred;
> + /* save a pointer to the beginning of the encoded verifier,
> + * for use in encryption/checksumming in svcauth_gss_release: */
> + __be32 *verf_start;
> + struct rsc *rsci;
> +
> + /* for temporary results */
> + u8 gsd_scratch[GSS_SCRATCH_SIZE];
> +};
>
> /* The rpcsec_init cache is used for mapping RPCSEC_GSS_{,CONT_}INIT requests
> * into replies.
> @@ -887,13 +912,11 @@ read_u32_from_xdr_buf(struct xdr_buf *buf, int base, u32 *obj)
> static int
> unwrap_integ_data(struct svc_rqst *rqstp, struct xdr_buf *buf, u32 seq, struct gss_ctx *ctx)
> {
> + struct gss_svc_data *gsd = rqstp->rq_auth_data;
> u32 integ_len, rseqno, maj_stat;
> - int stat = -EINVAL;
> struct xdr_netobj mic;
> struct xdr_buf integ_buf;
>
> - mic.data = NULL;
> -
> /* NFS READ normally uses splice to send data in-place. However
> * the data in cache can change after the reply's MIC is computed
> * but before the RPC reply is sent. To prevent the client from
> @@ -917,11 +940,9 @@ unwrap_integ_data(struct svc_rqst *rqstp, struct xdr_buf *buf, u32 seq, struct g
> /* copy out mic... */
> if (read_u32_from_xdr_buf(buf, integ_len, &mic.len))
> goto unwrap_failed;
> - if (mic.len > RPC_MAX_AUTH_SIZE)
> - goto unwrap_failed;
> - mic.data = kmalloc(mic.len, GFP_KERNEL);
> - if (!mic.data)
> + if (mic.len > sizeof(gsd->gsd_scratch))
> goto unwrap_failed;
> + mic.data = gsd->gsd_scratch;
> if (read_bytes_from_xdr_buf(buf, integ_len + 4, mic.data, mic.len))
> goto unwrap_failed;
> maj_stat = gss_verify_mic(ctx, &integ_buf, &mic);
> @@ -932,20 +953,17 @@ unwrap_integ_data(struct svc_rqst *rqstp, struct xdr_buf *buf, u32 seq, struct g
> goto bad_seqno;
> /* trim off the mic and padding at the end before returning */
> xdr_buf_trim(buf, round_up_to_quad(mic.len) + 4);
> - stat = 0;
> -out:
> - kfree(mic.data);
> - return stat;
> + return 0;
>
> unwrap_failed:
> trace_rpcgss_svc_unwrap_failed(rqstp);
> - goto out;
> + return -EINVAL;
> bad_seqno:
> trace_rpcgss_svc_seqno_bad(rqstp, seq, rseqno);
> - goto out;
> + return -EINVAL;
> bad_mic:
> trace_rpcgss_svc_mic(rqstp, maj_stat);
> - goto out;
> + return -EINVAL;
> }
>
> static inline int
> @@ -1023,15 +1041,6 @@ unwrap_priv_data(struct svc_rqst *rqstp, struct xdr_buf *buf, u32 seq, struct gs
> return -EINVAL;
> }
>
> -struct gss_svc_data {
> - /* decoded gss client cred: */
> - struct rpc_gss_wire_cred clcred;
> - /* save a pointer to the beginning of the encoded verifier,
> - * for use in encryption/checksumming in svcauth_gss_release: */
> - __be32 *verf_start;
> - struct rsc *rsci;
> -};
> -
> static int
> svcauth_gss_set_client(struct svc_rqst *rqstp)
> {
>
>
That makes a lot more sense!
Reviewed-by: Jeff Layton <[email protected]>
On Mon, 2022-11-28 at 12:12 -0500, Jeff Layton wrote:
> On Sun, 2022-11-27 at 12:17 -0500, Chuck Lever wrote:
> > If a zero length is passed to kmalloc() it returns 0x10, which is
> > not a valid address. gss_verify_mic() subsequently crashes when it
> > attempts to dereference that pointer.
> >
> > Instead of allocating this memory on every call based on an
> > untrusted size value, use a piece of dynamically-allocated scratch
> > memory that is always available.
> >
> > Signed-off-by: Chuck Lever <[email protected]>
> > ---
> > net/sunrpc/auth_gss/svcauth_gss.c | 55 ++++++++++++++++++++++---------------
> > 1 file changed, 32 insertions(+), 23 deletions(-)
> >
> > diff --git a/net/sunrpc/auth_gss/svcauth_gss.c b/net/sunrpc/auth_gss/svcauth_gss.c
> > index 9a5db285d4ae..148bb0a7fa5b 100644
> > --- a/net/sunrpc/auth_gss/svcauth_gss.c
> > +++ b/net/sunrpc/auth_gss/svcauth_gss.c
> > @@ -49,11 +49,36 @@
> > #include <linux/sunrpc/svcauth.h>
> > #include <linux/sunrpc/svcauth_gss.h>
> > #include <linux/sunrpc/cache.h>
> > +#include <linux/sunrpc/gss_krb5.h>
> >
> > #include <trace/events/rpcgss.h>
> >
> > #include "gss_rpc_upcall.h"
> >
> > +/*
> > + * Unfortunately there isn't a maximum checksum size exported via the
> > + * GSS API. Manufacture one based on GSS mechanisms supported by this
> > + * implementation.
> > + */
> > +#define GSS_MAX_CKSUMSIZE (GSS_KRB5_TOK_HDR_LEN + GSS_KRB5_MAX_CKSUM_LEN)
> > +
> > +/*
> > + * This value may be increased in the future to accommodate other
> > + * usage of the scratch buffer.
> > + */
> > +#define GSS_SCRATCH_SIZE GSS_MAX_CKSUMSIZE
> > +
> > +struct gss_svc_data {
> > + /* decoded gss client cred: */
> > + struct rpc_gss_wire_cred clcred;
> > + /* save a pointer to the beginning of the encoded verifier,
> > + * for use in encryption/checksumming in svcauth_gss_release: */
> > + __be32 *verf_start;
> > + struct rsc *rsci;
> > +
> > + /* for temporary results */
> > + u8 gsd_scratch[GSS_SCRATCH_SIZE];
> > +};
> >
> > /* The rpcsec_init cache is used for mapping RPCSEC_GSS_{,CONT_}INIT requests
> > * into replies.
> > @@ -887,13 +912,11 @@ read_u32_from_xdr_buf(struct xdr_buf *buf, int base, u32 *obj)
> > static int
> > unwrap_integ_data(struct svc_rqst *rqstp, struct xdr_buf *buf, u32 seq, struct gss_ctx *ctx)
> > {
> > + struct gss_svc_data *gsd = rqstp->rq_auth_data;
> > u32 integ_len, rseqno, maj_stat;
> > - int stat = -EINVAL;
> > struct xdr_netobj mic;
> > struct xdr_buf integ_buf;
> >
> > - mic.data = NULL;
> > -
> > /* NFS READ normally uses splice to send data in-place. However
> > * the data in cache can change after the reply's MIC is computed
> > * but before the RPC reply is sent. To prevent the client from
> > @@ -917,11 +940,9 @@ unwrap_integ_data(struct svc_rqst *rqstp, struct xdr_buf *buf, u32 seq, struct g
> > /* copy out mic... */
> > if (read_u32_from_xdr_buf(buf, integ_len, &mic.len))
> > goto unwrap_failed;
> > - if (mic.len > RPC_MAX_AUTH_SIZE)
> > - goto unwrap_failed;
> > - mic.data = kmalloc(mic.len, GFP_KERNEL);
> > - if (!mic.data)
> > + if (mic.len > sizeof(gsd->gsd_scratch))
> > goto unwrap_failed;
> > + mic.data = gsd->gsd_scratch;
> > if (read_bytes_from_xdr_buf(buf, integ_len + 4, mic.data, mic.len))
> > goto unwrap_failed;
> > maj_stat = gss_verify_mic(ctx, &integ_buf, &mic);
> > @@ -932,20 +953,17 @@ unwrap_integ_data(struct svc_rqst *rqstp, struct xdr_buf *buf, u32 seq, struct g
> > goto bad_seqno;
> > /* trim off the mic and padding at the end before returning */
> > xdr_buf_trim(buf, round_up_to_quad(mic.len) + 4);
> > - stat = 0;
> > -out:
> > - kfree(mic.data);
> > - return stat;
> > + return 0;
> >
> > unwrap_failed:
> > trace_rpcgss_svc_unwrap_failed(rqstp);
> > - goto out;
> > + return -EINVAL;
> > bad_seqno:
> > trace_rpcgss_svc_seqno_bad(rqstp, seq, rseqno);
> > - goto out;
> > + return -EINVAL;
> > bad_mic:
> > trace_rpcgss_svc_mic(rqstp, maj_stat);
> > - goto out;
> > + return -EINVAL;
> > }
> >
> > static inline int
> > @@ -1023,15 +1041,6 @@ unwrap_priv_data(struct svc_rqst *rqstp, struct xdr_buf *buf, u32 seq, struct gs
> > return -EINVAL;
> > }
> >
> > -struct gss_svc_data {
> > - /* decoded gss client cred: */
> > - struct rpc_gss_wire_cred clcred;
> > - /* save a pointer to the beginning of the encoded verifier,
> > - * for use in encryption/checksumming in svcauth_gss_release: */
> > - __be32 *verf_start;
> > - struct rsc *rsci;
> > -};
> > -
> > static int
> > svcauth_gss_set_client(struct svc_rqst *rqstp)
> > {
> >
> >
>
> That makes a lot more sense!
>
> Reviewed-by: Jeff Layton <[email protected]>
How did you find this, btw? Is there a bug report or something?
--
Jeff Layton <[email protected]>
> On Nov 28, 2022, at 12:13 PM, Jeff Layton <[email protected]> wrote:
>
> On Mon, 2022-11-28 at 12:12 -0500, Jeff Layton wrote:
>> On Sun, 2022-11-27 at 12:17 -0500, Chuck Lever wrote:
>>> If a zero length is passed to kmalloc() it returns 0x10, which is
>>> not a valid address. gss_verify_mic() subsequently crashes when it
>>> attempts to dereference that pointer.
>>>
>>> Instead of allocating this memory on every call based on an
>>> untrusted size value, use a piece of dynamically-allocated scratch
>>> memory that is always available.
>>>
>>> Signed-off-by: Chuck Lever <[email protected]>
>>> ---
>>> net/sunrpc/auth_gss/svcauth_gss.c | 55 ++++++++++++++++++++++---------------
>>> 1 file changed, 32 insertions(+), 23 deletions(-)
>>>
>>> diff --git a/net/sunrpc/auth_gss/svcauth_gss.c b/net/sunrpc/auth_gss/svcauth_gss.c
>>> index 9a5db285d4ae..148bb0a7fa5b 100644
>>> --- a/net/sunrpc/auth_gss/svcauth_gss.c
>>> +++ b/net/sunrpc/auth_gss/svcauth_gss.c
>>> @@ -49,11 +49,36 @@
>>> #include <linux/sunrpc/svcauth.h>
>>> #include <linux/sunrpc/svcauth_gss.h>
>>> #include <linux/sunrpc/cache.h>
>>> +#include <linux/sunrpc/gss_krb5.h>
>>>
>>> #include <trace/events/rpcgss.h>
>>>
>>> #include "gss_rpc_upcall.h"
>>>
>>> +/*
>>> + * Unfortunately there isn't a maximum checksum size exported via the
>>> + * GSS API. Manufacture one based on GSS mechanisms supported by this
>>> + * implementation.
>>> + */
>>> +#define GSS_MAX_CKSUMSIZE (GSS_KRB5_TOK_HDR_LEN + GSS_KRB5_MAX_CKSUM_LEN)
>>> +
>>> +/*
>>> + * This value may be increased in the future to accommodate other
>>> + * usage of the scratch buffer.
>>> + */
>>> +#define GSS_SCRATCH_SIZE GSS_MAX_CKSUMSIZE
>>> +
>>> +struct gss_svc_data {
>>> + /* decoded gss client cred: */
>>> + struct rpc_gss_wire_cred clcred;
>>> + /* save a pointer to the beginning of the encoded verifier,
>>> + * for use in encryption/checksumming in svcauth_gss_release: */
>>> + __be32 *verf_start;
>>> + struct rsc *rsci;
>>> +
>>> + /* for temporary results */
>>> + u8 gsd_scratch[GSS_SCRATCH_SIZE];
>>> +};
>>>
>>> /* The rpcsec_init cache is used for mapping RPCSEC_GSS_{,CONT_}INIT requests
>>> * into replies.
>>> @@ -887,13 +912,11 @@ read_u32_from_xdr_buf(struct xdr_buf *buf, int base, u32 *obj)
>>> static int
>>> unwrap_integ_data(struct svc_rqst *rqstp, struct xdr_buf *buf, u32 seq, struct gss_ctx *ctx)
>>> {
>>> + struct gss_svc_data *gsd = rqstp->rq_auth_data;
>>> u32 integ_len, rseqno, maj_stat;
>>> - int stat = -EINVAL;
>>> struct xdr_netobj mic;
>>> struct xdr_buf integ_buf;
>>>
>>> - mic.data = NULL;
>>> -
>>> /* NFS READ normally uses splice to send data in-place. However
>>> * the data in cache can change after the reply's MIC is computed
>>> * but before the RPC reply is sent. To prevent the client from
>>> @@ -917,11 +940,9 @@ unwrap_integ_data(struct svc_rqst *rqstp, struct xdr_buf *buf, u32 seq, struct g
>>> /* copy out mic... */
>>> if (read_u32_from_xdr_buf(buf, integ_len, &mic.len))
>>> goto unwrap_failed;
>>> - if (mic.len > RPC_MAX_AUTH_SIZE)
>>> - goto unwrap_failed;
>>> - mic.data = kmalloc(mic.len, GFP_KERNEL);
>>> - if (!mic.data)
>>> + if (mic.len > sizeof(gsd->gsd_scratch))
>>> goto unwrap_failed;
>>> + mic.data = gsd->gsd_scratch;
>>> if (read_bytes_from_xdr_buf(buf, integ_len + 4, mic.data, mic.len))
>>> goto unwrap_failed;
>>> maj_stat = gss_verify_mic(ctx, &integ_buf, &mic);
>>> @@ -932,20 +953,17 @@ unwrap_integ_data(struct svc_rqst *rqstp, struct xdr_buf *buf, u32 seq, struct g
>>> goto bad_seqno;
>>> /* trim off the mic and padding at the end before returning */
>>> xdr_buf_trim(buf, round_up_to_quad(mic.len) + 4);
>>> - stat = 0;
>>> -out:
>>> - kfree(mic.data);
>>> - return stat;
>>> + return 0;
>>>
>>> unwrap_failed:
>>> trace_rpcgss_svc_unwrap_failed(rqstp);
>>> - goto out;
>>> + return -EINVAL;
>>> bad_seqno:
>>> trace_rpcgss_svc_seqno_bad(rqstp, seq, rseqno);
>>> - goto out;
>>> + return -EINVAL;
>>> bad_mic:
>>> trace_rpcgss_svc_mic(rqstp, maj_stat);
>>> - goto out;
>>> + return -EINVAL;
>>> }
>>>
>>> static inline int
>>> @@ -1023,15 +1041,6 @@ unwrap_priv_data(struct svc_rqst *rqstp, struct xdr_buf *buf, u32 seq, struct gs
>>> return -EINVAL;
>>> }
>>>
>>> -struct gss_svc_data {
>>> - /* decoded gss client cred: */
>>> - struct rpc_gss_wire_cred clcred;
>>> - /* save a pointer to the beginning of the encoded verifier,
>>> - * for use in encryption/checksumming in svcauth_gss_release: */
>>> - __be32 *verf_start;
>>> - struct rsc *rsci;
>>> -};
>>> -
>>> static int
>>> svcauth_gss_set_client(struct svc_rqst *rqstp)
>>> {
>>>
>>>
>>
>> That makes a lot more sense!
>>
>> Reviewed-by: Jeff Layton <[email protected]>
>
> How did you find this, btw? Is there a bug report or something?
I recently fixed the same problem on the client-side. I managed
to trigger the client-side problem while working on the server
GSS overhaul.
--
Chuck Lever