| Message ID | 166956944745.113279.2771726273440100988.stgit@klimt.1015granger.net (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | SUNRPC: Fix crasher in unwrap_integ_data() | expand |
> On Nov 27, 2022, at 12:17 PM, Chuck Lever <chuck.lever@oracle.com> 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 <chuck.lever@oracle.com> > --- > 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 <chuck.lever@oracle.com> > --- > 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 <jlayton@kernel.org>
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 <chuck.lever@oracle.com> > > --- > > 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 <jlayton@kernel.org> How did you find this, btw? Is there a bug report or something?
> On Nov 28, 2022, at 12:13 PM, Jeff Layton <jlayton@kernel.org> 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 <chuck.lever@oracle.com> >>> --- >>> 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 <jlayton@kernel.org> > > 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
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) {
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 <chuck.lever@oracle.com> --- net/sunrpc/auth_gss/svcauth_gss.c | 55 ++++++++++++++++++++++--------------- 1 file changed, 32 insertions(+), 23 deletions(-)