On Mon, Sep 16, 2024 at 12:00:01PM -0400, Chuck Lever wrote:
> On Mon, Sep 16, 2024 at 06:14:31PM +0300, Dan Carpenter wrote:
> > Hello Chuck Lever,
> >
> > Commit b68e4c5c3227 ("SUNRPC: Convert unwrap_integ_data() to use
> > xdr_stream") from Jan 2, 2023 (linux-next), leads to the following
> > Smatch static checker warning:
> >
> > net/sunrpc/auth_gss/svcauth_gss.c:895 svcauth_gss_unwrap_integ()
> > warn: potential user controlled sizeof overflow 'offset + 4'
> >
> > net/sunrpc/auth_gss/svcauth_gss.c
> > 859 static noinline_for_stack int
> > 860 svcauth_gss_unwrap_integ(struct svc_rqst *rqstp, u32 seq, struct gss_ctx *ctx)
> > 861 {
> > 862 struct gss_svc_data *gsd = rqstp->rq_auth_data;
> > 863 struct xdr_stream *xdr = &rqstp->rq_arg_stream;
> > 864 u32 len, offset, seq_num, maj_stat;
> > 865 struct xdr_buf *buf = xdr->buf;
> > 866 struct xdr_buf databody_integ;
> > 867 struct xdr_netobj checksum;
> > 868
> > 869 /* Did we already verify the signature on the original pass through? */
> > 870 if (rqstp->rq_deferred)
> > 871 return 0;
> > 872
> > 873 if (xdr_stream_decode_u32(xdr, &len) < 0)
> > ^^^^
> > 874 goto unwrap_failed;
> > 875 if (len & 3)
> >
> > There used a if (len > buf->len) here but it was deleted.
>
> True, there is no /explicit/ bounds check, but AFAICT,
> xdr_buf_subsegment() will return -1 if the value of @len is larger
> than the remaining space in @buf.
>
Ah yes. I see that now. Thanks.
regards,
dan carpenter
