diff mbox series

[v2] nfsd: prevent integer overflow in decode_cb_compound4res()

Message ID ed14a180-56c8-40f2-acf7-26a787eb3769@suswa.mountain (mailing list archive)
State New
Headers show
Series [v2] nfsd: prevent integer overflow in decode_cb_compound4res() | expand

Commit Message

Dan Carpenter Sept. 19, 2024, 8:12 a.m. UTC
If "length" is >= U32_MAX - 3 then the "length + 4" addition can result
in an integer overflow.  The impact of this bug is not totally clear to
me, but it's safer to not allow the integer overflow.

Check that "length" is valid right away before doing any math.

Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2")
Signed-off-by: Dan Carpenter <dan.carpenter@linaro.org>
---
v2: Check that "len" is valid instead of just checking for integer
    overflows.

 fs/nfsd/nfs4callback.c | 2 ++
 1 file changed, 2 insertions(+)

Comments

Jeff Layton Sept. 19, 2024, 8:44 a.m. UTC | #1
On Thu, 2024-09-19 at 11:12 +0300, Dan Carpenter wrote:
> If "length" is >= U32_MAX - 3 then the "length + 4" addition can result
> in an integer overflow.  The impact of this bug is not totally clear to
> me, but it's safer to not allow the integer overflow.
> 
> Check that "length" is valid right away before doing any math.
> 
> Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2")
> Signed-off-by: Dan Carpenter <dan.carpenter@linaro.org>
> ---
> v2: Check that "len" is valid instead of just checking for integer
>     overflows.
> 
>  fs/nfsd/nfs4callback.c | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/fs/nfsd/nfs4callback.c b/fs/nfsd/nfs4callback.c
> index 43b8320c8255..0f5b7b6fba74 100644
> --- a/fs/nfsd/nfs4callback.c
> +++ b/fs/nfsd/nfs4callback.c
> @@ -317,6 +317,8 @@ static int decode_cb_compound4res(struct xdr_stream *xdr,
>  	hdr->status = be32_to_cpup(p++);
>  	/* Ignore the tag */
>  	length = be32_to_cpup(p++);
> +	if (unlikely(length > xdr->buf->len))
> +		goto out_overflow;
>  	p = xdr_inline_decode(xdr, length + 4);
>  	if (unlikely(p == NULL))
>  		goto out_overflow;

Reviewed-by: Jeff Layton <jlayton@kernel.org>
Chuck Lever Sept. 19, 2024, 2:02 p.m. UTC | #2
> On Sep 19, 2024, at 1:12 AM, Dan Carpenter <dan.carpenter@linaro.org> wrote:
> 
> If "length" is >= U32_MAX - 3 then the "length + 4" addition can result
> in an integer overflow.  The impact of this bug is not totally clear to
> me, but it's safer to not allow the integer overflow.
> 
> Check that "length" is valid right away before doing any math.
> 
> Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2")
> Signed-off-by: Dan Carpenter <dan.carpenter@linaro.org>
> ---
> v2: Check that "len" is valid instead of just checking for integer
>    overflows.
> 
> fs/nfsd/nfs4callback.c | 2 ++
> 1 file changed, 2 insertions(+)
> 
> diff --git a/fs/nfsd/nfs4callback.c b/fs/nfsd/nfs4callback.c
> index 43b8320c8255..0f5b7b6fba74 100644
> --- a/fs/nfsd/nfs4callback.c
> +++ b/fs/nfsd/nfs4callback.c
> @@ -317,6 +317,8 @@ static int decode_cb_compound4res(struct xdr_stream *xdr,
> hdr->status = be32_to_cpup(p++);
> /* Ignore the tag */
> length = be32_to_cpup(p++);
> + if (unlikely(length > xdr->buf->len))
> + goto out_overflow;
> p = xdr_inline_decode(xdr, length + 4);
> if (unlikely(p == NULL))
> goto out_overflow;
> -- 
> 2.34.1
> 

Hi Dan, I've already gone with

https://lore.kernel.org/linux-nfs/172658972371.2454.15715383792386404543.stgit@oracle-102.chuck.lever.oracle.com.nfsv4.dev/T/#u

Let me know if that doesn't make sense.

--
Chuck Lever
Dan Carpenter Sept. 20, 2024, 11:10 a.m. UTC | #3
On Thu, Sep 19, 2024 at 02:02:19PM +0000, Chuck Lever III wrote:
> 
> 
> > On Sep 19, 2024, at 1:12 AM, Dan Carpenter <dan.carpenter@linaro.org> wrote:
> > 
> > If "length" is >= U32_MAX - 3 then the "length + 4" addition can result
> > in an integer overflow.  The impact of this bug is not totally clear to
> > me, but it's safer to not allow the integer overflow.
> > 
> > Check that "length" is valid right away before doing any math.
> > 
> > Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2")
> > Signed-off-by: Dan Carpenter <dan.carpenter@linaro.org>
> > ---
> > v2: Check that "len" is valid instead of just checking for integer
> >    overflows.
> > 
> > fs/nfsd/nfs4callback.c | 2 ++
> > 1 file changed, 2 insertions(+)
> > 
> > diff --git a/fs/nfsd/nfs4callback.c b/fs/nfsd/nfs4callback.c
> > index 43b8320c8255..0f5b7b6fba74 100644
> > --- a/fs/nfsd/nfs4callback.c
> > +++ b/fs/nfsd/nfs4callback.c
> > @@ -317,6 +317,8 @@ static int decode_cb_compound4res(struct xdr_stream *xdr,
> > hdr->status = be32_to_cpup(p++);
> > /* Ignore the tag */
> > length = be32_to_cpup(p++);
> > + if (unlikely(length > xdr->buf->len))
> > + goto out_overflow;
> > p = xdr_inline_decode(xdr, length + 4);
> > if (unlikely(p == NULL))
> > goto out_overflow;
> > -- 
> > 2.34.1
> > 
> 
> Hi Dan, I've already gone with
> 
> https://lore.kernel.org/linux-nfs/172658972371.2454.15715383792386404543.stgit@oracle-102.chuck.lever.oracle.com.nfsv4.dev/T/#u
> 
> Let me know if that doesn't make sense.

Oh, sorry, I got mixed up which things were patched.  That looks good.
The truth is I always prefer when people give me a reported by tag so I
don't have to write a v2 patch.  It just saves a back and forth.
Thanks!

regards,
dan carpenter
diff mbox series

Patch

diff --git a/fs/nfsd/nfs4callback.c b/fs/nfsd/nfs4callback.c
index 43b8320c8255..0f5b7b6fba74 100644
--- a/fs/nfsd/nfs4callback.c
+++ b/fs/nfsd/nfs4callback.c
@@ -317,6 +317,8 @@  static int decode_cb_compound4res(struct xdr_stream *xdr,
 	hdr->status = be32_to_cpup(p++);
 	/* Ignore the tag */
 	length = be32_to_cpup(p++);
+	if (unlikely(length > xdr->buf->len))
+		goto out_overflow;
 	p = xdr_inline_decode(xdr, length + 4);
 	if (unlikely(p == NULL))
 		goto out_overflow;