[PATCH] usb: gadget: f_mass_storage: Fix potential integer overflow in check_command_size_in_blocks()

[PATCH] usb: gadget: f_mass_storage: Fix potential integer overflow in check_command_size_in_blocks()

[pip-izony]

From: Seungjin Bae <eeodqql09@gmail.com>

The `check_command_size_in_blocks()` function calculates the data size
in bytes by left shifting `common->data_size_from_cmnd` by the block
size (`common->curlun->blkbits`). However, it does not validate whether
this shift operation will cause an integer overflow.

Initially, the block size is set up in `fsg_lun_open()` , and the
`common->data_size_from_cmnd` is set up in `do_scsi_command()`. During
initialization, there is no integer overflow check for the interaction
between two variables.

So if a malicious USB host sends a SCSI READ or WRITE command
requesting a large amount of data (`common->data_size_from_cmnd`), the
left shift operation can wrap around. This results in a truncated data
size, which can bypass boundary checks and potentially lead to memory
corruption or out-of-bounds accesses.

Fix this by ensuring that `common->data_size_from_cmnd` is not greater
than `(U32_MAX >> common->curlun->blkbits)` before performing the shift.

Fixes: 144974e7f9e3 ("usb: gadget: mass_storage: support multi-luns with different logic block size")
Signed-off-by: Seungjin Bae <eeodqql09@gmail.com>
---
 drivers/usb/gadget/function/f_mass_storage.c | 14 ++++++++++++--
 1 file changed, 12 insertions(+), 2 deletions(-)

diff --git a/drivers/usb/gadget/function/f_mass_storage.c b/drivers/usb/gadget/function/f_mass_storage.c
index 6af96e2b44eb..12e7ef2e074e 100644
--- a/drivers/usb/gadget/function/f_mass_storage.c
+++ b/drivers/usb/gadget/function/f_mass_storage.c
@@ -1853,8 +1853,18 @@ static int check_command_size_in_blocks(struct fsg_common *common,
 		int cmnd_size, enum data_direction data_dir,
 		unsigned int mask, int needs_medium, const char *name)
 {
-	if (common->curlun)
-		common->data_size_from_cmnd <<= common->curlun->blkbits;
+	unsigned int blkbits;
+
+	if (common->curlun) {
+		blkbits = common->curlun->blkbits;
+		if (cmnd_size > 10 &&
+			common->data_size_from_cmnd > (U32_MAX >> blkbits)) {
+			common->phase_error = 1;
+			return -EINVAL;
+		}
+		common->data_size_from_cmnd <<= blkbits;
+	}
+
 	return check_command(common, cmnd_size, data_dir,
 			mask, needs_medium, name);
 }
-- 
2.43.0

Re: [PATCH] usb: gadget: f_mass_storage: Fix potential integer overflow in check_command_size_in_blocks()

[Alan Stern]

On Wed, Feb 25, 2026 at 06:49:58AM -0500, pip-izony wrote:
> From: Seungjin Bae <eeodqql09@gmail.com>
> 
> The `check_command_size_in_blocks()` function calculates the data size
> in bytes by left shifting `common->data_size_from_cmnd` by the block
> size (`common->curlun->blkbits`). However, it does not validate whether
> this shift operation will cause an integer overflow.
> 
> Initially, the block size is set up in `fsg_lun_open()` , and the
> `common->data_size_from_cmnd` is set up in `do_scsi_command()`. During
> initialization, there is no integer overflow check for the interaction
> between two variables.
> 
> So if a malicious USB host sends a SCSI READ or WRITE command
> requesting a large amount of data (`common->data_size_from_cmnd`), the
> left shift operation can wrap around. This results in a truncated data
> size, which can bypass boundary checks and potentially lead to memory
> corruption or out-of-bounds accesses.
> 
> Fix this by ensuring that `common->data_size_from_cmnd` is not greater
> than `(U32_MAX >> common->curlun->blkbits)` before performing the shift.
> 
> Fixes: 144974e7f9e3 ("usb: gadget: mass_storage: support multi-luns with different logic block size")
> Signed-off-by: Seungjin Bae <eeodqql09@gmail.com>
> ---

Good catch.

>  drivers/usb/gadget/function/f_mass_storage.c | 14 ++++++++++++--
>  1 file changed, 12 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/usb/gadget/function/f_mass_storage.c b/drivers/usb/gadget/function/f_mass_storage.c
> index 6af96e2b44eb..12e7ef2e074e 100644
> --- a/drivers/usb/gadget/function/f_mass_storage.c
> +++ b/drivers/usb/gadget/function/f_mass_storage.c
> @@ -1853,8 +1853,18 @@ static int check_command_size_in_blocks(struct fsg_common *common,
>  		int cmnd_size, enum data_direction data_dir,
>  		unsigned int mask, int needs_medium, const char *name)
>  {
> -	if (common->curlun)
> -		common->data_size_from_cmnd <<= common->curlun->blkbits;
> +	unsigned int blkbits;
> +
> +	if (common->curlun) {
> +		blkbits = common->curlun->blkbits;
> +		if (cmnd_size > 10 &&

I would remove this test.  The check is valid even when cmnd_size is 6, 
and doing the test and conditional branch probably takes more time than 
you would save by avoiding the shift and comparison below.

Otherwise this is fine.

Alan Stern

> +			common->data_size_from_cmnd > (U32_MAX >> blkbits)) {
> +			common->phase_error = 1;
> +			return -EINVAL;
> +		}
> +		common->data_size_from_cmnd <<= blkbits;
> +	}
> +
>  	return check_command(common, cmnd_size, data_dir,
>  			mask, needs_medium, name);
>  }
> -- 
> 2.43.0

Re: [PATCH] usb: gadget: f_mass_storage: Fix potential integer overflow in check_command_size_in_blocks()

[Kees Cook]

On Wed, Feb 25, 2026 at 06:49:58AM -0500, pip-izony wrote:
> From: Seungjin Bae <eeodqql09@gmail.com>
> 
> The `check_command_size_in_blocks()` function calculates the data size
> in bytes by left shifting `common->data_size_from_cmnd` by the block
> size (`common->curlun->blkbits`). However, it does not validate whether
> this shift operation will cause an integer overflow.
> 
> Initially, the block size is set up in `fsg_lun_open()` , and the
> `common->data_size_from_cmnd` is set up in `do_scsi_command()`. During
> initialization, there is no integer overflow check for the interaction
> between two variables.
> 
> So if a malicious USB host sends a SCSI READ or WRITE command
> requesting a large amount of data (`common->data_size_from_cmnd`), the
> left shift operation can wrap around. This results in a truncated data
> size, which can bypass boundary checks and potentially lead to memory
> corruption or out-of-bounds accesses.
> 
> Fix this by ensuring that `common->data_size_from_cmnd` is not greater
> than `(U32_MAX >> common->curlun->blkbits)` before performing the shift.
> 
> Fixes: 144974e7f9e3 ("usb: gadget: mass_storage: support multi-luns with different logic block size")
> Signed-off-by: Seungjin Bae <eeodqql09@gmail.com>
> ---
>  drivers/usb/gadget/function/f_mass_storage.c | 14 ++++++++++++--
>  1 file changed, 12 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/usb/gadget/function/f_mass_storage.c b/drivers/usb/gadget/function/f_mass_storage.c
> index 6af96e2b44eb..12e7ef2e074e 100644
> --- a/drivers/usb/gadget/function/f_mass_storage.c
> +++ b/drivers/usb/gadget/function/f_mass_storage.c
> @@ -1853,8 +1853,18 @@ static int check_command_size_in_blocks(struct fsg_common *common,
>  		int cmnd_size, enum data_direction data_dir,
>  		unsigned int mask, int needs_medium, const char *name)
>  {
> -	if (common->curlun)
> -		common->data_size_from_cmnd <<= common->curlun->blkbits;
> +	unsigned int blkbits;
> +
> +	if (common->curlun) {
> +		blkbits = common->curlun->blkbits;
> +		if (cmnd_size > 10 &&
> +			common->data_size_from_cmnd > (U32_MAX >> blkbits)) {
> +			common->phase_error = 1;
> +			return -EINVAL;
> +		}
> +		common->data_size_from_cmnd <<= blkbits;
> +	}
> +

Can check_shl_overflow() be used here to make this easier?

>  	return check_command(common, cmnd_size, data_dir,
>  			mask, needs_medium, name);
>  }
> -- 
> 2.43.0

-- 
Kees Cook

Re: [PATCH] usb: gadget: f_mass_storage: Fix potential integer overflow in check_command_size_in_blocks()

[Seungjin Bae]

2026년 2월 25일 (수) PM 4:00, Kees Cook <kees@kernel.org>님이 작성:
>
> On Wed, Feb 25, 2026 at 06:49:58AM -0500, pip-izony wrote:
> > From: Seungjin Bae <eeodqql09@gmail.com>
> >
> > The `check_command_size_in_blocks()` function calculates the data size
> > in bytes by left shifting `common->data_size_from_cmnd` by the block
> > size (`common->curlun->blkbits`). However, it does not validate whether
> > this shift operation will cause an integer overflow.
> >
> > Initially, the block size is set up in `fsg_lun_open()` , and the
> > `common->data_size_from_cmnd` is set up in `do_scsi_command()`. During
> > initialization, there is no integer overflow check for the interaction
> > between two variables.
> >
> > So if a malicious USB host sends a SCSI READ or WRITE command
> > requesting a large amount of data (`common->data_size_from_cmnd`), the
> > left shift operation can wrap around. This results in a truncated data
> > size, which can bypass boundary checks and potentially lead to memory
> > corruption or out-of-bounds accesses.
> >
> > Fix this by ensuring that `common->data_size_from_cmnd` is not greater
> > than `(U32_MAX >> common->curlun->blkbits)` before performing the shift.
> >
> > Fixes: 144974e7f9e3 ("usb: gadget: mass_storage: support multi-luns with different logic block size")
> > Signed-off-by: Seungjin Bae <eeodqql09@gmail.com>
> > ---
> >  drivers/usb/gadget/function/f_mass_storage.c | 14 ++++++++++++--
> >  1 file changed, 12 insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/usb/gadget/function/f_mass_storage.c b/drivers/usb/gadget/function/f_mass_storage.c
> > index 6af96e2b44eb..12e7ef2e074e 100644
> > --- a/drivers/usb/gadget/function/f_mass_storage.c
> > +++ b/drivers/usb/gadget/function/f_mass_storage.c
> > @@ -1853,8 +1853,18 @@ static int check_command_size_in_blocks(struct fsg_common *common,
> >               int cmnd_size, enum data_direction data_dir,
> >               unsigned int mask, int needs_medium, const char *name)
> >  {
> > -     if (common->curlun)
> > -             common->data_size_from_cmnd <<= common->curlun->blkbits;
> > +     unsigned int blkbits;
> > +
> > +     if (common->curlun) {
> > +             blkbits = common->curlun->blkbits;
> > +             if (cmnd_size > 10 &&
> > +                     common->data_size_from_cmnd > (U32_MAX >> blkbits)) {
> > +                     common->phase_error = 1;
> > +                     return -EINVAL;
> > +             }
> > +             common->data_size_from_cmnd <<= blkbits;
> > +     }
> > +
>
> Can check_shl_overflow() be used here to make this easier?

Yes, definitely.
I will update the code to use it in v2.
>
> >       return check_command(common, cmnd_size, data_dir,
> >                       mask, needs_medium, name);
> >  }
> > --
> > 2.43.0
>
> --
> Kees Cook