[PATCH 0/3] usb: musb-new: Address some Smatch issues

[PATCH 0/3] usb: musb-new: Address some Smatch issues

[Andrew Goodbody]

Smatch reported some issues with the musb-new driver which included a
variable being dereferenced before being null checked, an array index
being used before being limit checked and unsigned values being tested
for being negative.

Signed-off-by: Andrew Goodbody <andrew.goodbody@linaro.org>
---
Andrew Goodbody (3):
      usb: musb-new: Null check before dereference
      usb: musb-new: Limit check array index before use
      usb: musb-new: Cannot test unsigned member to be negative

 drivers/usb/musb-new/am35x.c           |  4 ++--
 drivers/usb/musb-new/musb_gadget_ep0.c |  5 ++++-
 drivers/usb/musb-new/omap2430.c        | 34 +++++++++++++++++++++-------------
 drivers/usb/musb-new/ti-musb.c         | 28 +++++++++++++++++-----------
 4 files changed, 44 insertions(+), 27 deletions(-)
---
base-commit: 9710d98e8942151fc0c62d54100d9d27e8263d04
change-id: 20250930-usb_musb-new-ed01ca60ba89

Best regards,
-- 
Andrew Goodbody <andrew.goodbody@linaro.org>

[PATCH 1/3] usb: musb-new: Null check before dereference

[Andrew Goodbody]

A null check for the variable 'data' was introduced before dereferencing
it for set_phy_power but other uses were not so protected. Add the null
check for other dereferences of 'data'.

This issue was found by Smatch.

Signed-off-by: Andrew Goodbody <andrew.goodbody@linaro.org>
---
 drivers/usb/musb-new/am35x.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/usb/musb-new/am35x.c b/drivers/usb/musb-new/am35x.c
index 42bc816e4f1e5ca0e6fa891c563fea65106e0b43..ca4d798642e731755473063df9507185b1d59205 100644
--- a/drivers/usb/musb-new/am35x.c
+++ b/drivers/usb/musb-new/am35x.c
@@ -402,7 +402,7 @@ static int am35x_musb_init(struct musb *musb)
 #endif
 
 	/* Reset the musb */
-	if (data->reset)
+	if (data && data->reset)
 		data->reset(data->dev);
 
 	/* Reset the controller */
@@ -417,7 +417,7 @@ static int am35x_musb_init(struct musb *musb)
 	musb->isr = am35x_musb_interrupt;
 
 	/* clear level interrupt */
-	if (data->clear_irq)
+	if (data && data->clear_irq)
 		data->clear_irq(data->dev);
 
 	return 0;

-- 
2.47.3

[PATCH 2/3] usb: musb-new: Limit check array index before use

[Andrew Goodbody]

epnum is used as an index into an array. The limit check for this index
should be performed before using it to access an element in the array to
prevent possible bounds overrun.

This issue was found by Smatch.

Signed-off-by: Andrew Goodbody <andrew.goodbody@linaro.org>
---
 drivers/usb/musb-new/musb_gadget_ep0.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/drivers/usb/musb-new/musb_gadget_ep0.c b/drivers/usb/musb-new/musb_gadget_ep0.c
index ea65326ab6264492ea668ddd047b360965a5ad19..25b1de6e58f9e2028e9b93a600d532ae0f5efcc1 100644
--- a/drivers/usb/musb-new/musb_gadget_ep0.c
+++ b/drivers/usb/musb-new/musb_gadget_ep0.c
@@ -96,6 +96,9 @@ static int service_tx_status_request(
 		if (!epnum) {
 			result[0] = 0;
 			break;
+		} else if (epnum >= MUSB_C_NUM_EPS) {
+			handled = -EINVAL;
+			break;
 		}
 
 		is_in = epnum & USB_DIR_IN;
@@ -107,7 +110,7 @@ static int service_tx_status_request(
 		}
 		regs = musb->endpoints[epnum].regs;
 
-		if (epnum >= MUSB_C_NUM_EPS || !ep->desc) {
+		if (!ep->desc) {
 			handled = -EINVAL;
 			break;
 		}

-- 
2.47.3

Re: [PATCH 2/3] usb: musb-new: Limit check array index before use

[Mattijs Korpershoek]

Hi Andrew,

Thank you for the patch.

On Tue, Sep 30, 2025 at 16:52, Andrew Goodbody <andrew.goodbody@linaro.org> wrote:

> epnum is used as an index into an array. The limit check for this index
> should be performed before using it to access an element in the array to
> prevent possible bounds overrun.
>
> This issue was found by Smatch.
>
> Signed-off-by: Andrew Goodbody <andrew.goodbody@linaro.org>

Reviewed-by: Mattijs Korpershoek <mkorpershoek@kernel.org>

> ---
>  drivers/usb/musb-new/musb_gadget_ep0.c | 5 ++++-
>  1 file changed, 4 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/usb/musb-new/musb_gadget_ep0.c b/drivers/usb/musb-new/musb_gadget_ep0.c
> index ea65326ab6264492ea668ddd047b360965a5ad19..25b1de6e58f9e2028e9b93a600d532ae0f5efcc1 100644
> --- a/drivers/usb/musb-new/musb_gadget_ep0.c
> +++ b/drivers/usb/musb-new/musb_gadget_ep0.c
> @@ -96,6 +96,9 @@ static int service_tx_status_request(
>  		if (!epnum) {
>  			result[0] = 0;
>  			break;
> +		} else if (epnum >= MUSB_C_NUM_EPS) {
> +			handled = -EINVAL;
> +			break;
>  		}
>  
>  		is_in = epnum & USB_DIR_IN;
> @@ -107,7 +110,7 @@ static int service_tx_status_request(
>  		}
>  		regs = musb->endpoints[epnum].regs;
>  
> -		if (epnum >= MUSB_C_NUM_EPS || !ep->desc) {
> +		if (!ep->desc) {
>  			handled = -EINVAL;
>  			break;
>  		}
>
> -- 
> 2.47.3

[PATCH 3/3] usb: musb-new: Cannot test unsigned member to be negative

[Andrew Goodbody]

You cannot test an unsigned member of a struct for being negative, the
test will always fail. Instead assign the return value of
fdtdec_get_int, which returns an int, to a temporary variable declared
as an int, so that it can be tested for being negative before being
assigned to the unsigned struct member.

This issue was found by Smatch.

Signed-off-by: Andrew Goodbody <andrew.goodbody@linaro.org>
---
 drivers/usb/musb-new/omap2430.c | 34 +++++++++++++++++++++-------------
 drivers/usb/musb-new/ti-musb.c  | 28 +++++++++++++++++-----------
 2 files changed, 38 insertions(+), 24 deletions(-)

diff --git a/drivers/usb/musb-new/omap2430.c b/drivers/usb/musb-new/omap2430.c
index ba600d0110239164ca7c287ba6347e20d9ae2931..7fd6639013a7fcdaaf9aee38bdaddaa82a66f3a5 100644
--- a/drivers/usb/musb-new/omap2430.c
+++ b/drivers/usb/musb-new/omap2430.c
@@ -142,41 +142,49 @@ static int omap2430_musb_of_to_plat(struct udevice *dev)
 	struct omap2430_musb_plat *plat = dev_get_plat(dev);
 	const void *fdt = gd->fdt_blob;
 	int node = dev_of_offset(dev);
+	int ret;
 
 	plat->base = (void *)dev_read_addr_ptr(dev);
 
-	plat->musb_config.multipoint = fdtdec_get_int(fdt, node, "multipoint",
-						      -1);
-	if (plat->musb_config.multipoint < 0) {
+	ret = fdtdec_get_int(fdt, node, "multipoint", -1);
+	if (ret < 0) {
 		pr_err("MUSB multipoint DT entry missing\n");
 		return -ENOENT;
+	} else {
+		plat->musb_config.multipoint = ret;
 	}
 
 	plat->musb_config.dyn_fifo = 1;
-	plat->musb_config.num_eps = fdtdec_get_int(fdt, node, "num-eps", -1);
-	if (plat->musb_config.num_eps < 0) {
+	ret = fdtdec_get_int(fdt, node, "num-eps", -1);
+	if (ret < 0) {
 		pr_err("MUSB num-eps DT entry missing\n");
 		return -ENOENT;
+	} else {
+		plat->musb_config.num_eps = ret;
 	}
 
-	plat->musb_config.ram_bits = fdtdec_get_int(fdt, node, "ram-bits", -1);
-	if (plat->musb_config.ram_bits < 0) {
+	ret = fdtdec_get_int(fdt, node, "ram-bits", -1);
+	if (ret < 0) {
 		pr_err("MUSB ram-bits DT entry missing\n");
 		return -ENOENT;
+	} else {
+		plat->musb_config.ram_bits = ret;
 	}
 
-	plat->plat.power = fdtdec_get_int(fdt, node, "power", -1);
-	if (plat->plat.power < 0) {
+	ret = fdtdec_get_int(fdt, node, "power", -1);
+	if (ret < 0) {
 		pr_err("MUSB power DT entry missing\n");
 		return -ENOENT;
+	} else {
+		plat->plat.power = ret;
 	}
 
-	plat->otg_board_data.interface_type = fdtdec_get_int(fdt, node,
-							     "interface-type",
-							     -1);
-	if (plat->otg_board_data.interface_type < 0) {
+	ret = fdtdec_get_int(fdt, node, "interface-type", -1);
+	if (ret < 0) {
 		pr_err("MUSB interface-type DT entry missing\n");
 		return -ENOENT;
+	} else {
+		plat->otg_board_data.interface_type = ret;
 	}
 
 #if 0 /* In a perfect world, mode would be set to OTG, mode 3 from DT */
diff --git a/drivers/usb/musb-new/ti-musb.c b/drivers/usb/musb-new/ti-musb.c
index 967d0953875f734ceeb4bd4be610f9cc562ce430..bcd31adba522fc55190fc43fd2dbd2dec93a2731 100644
--- a/drivers/usb/musb-new/ti-musb.c
+++ b/drivers/usb/musb-new/ti-musb.c
@@ -86,6 +86,7 @@ static int ti_musb_of_to_plat(struct udevice *dev)
 	int phys;
 	int ctrl_mod;
 	int usb_index;
+	int ret;
 	struct musb_hdrc_config *musb_config;
 
 	plat->base = devfdt_get_addr_index_ptr(dev, 1);
@@ -108,35 +109,40 @@ static int ti_musb_of_to_plat(struct udevice *dev)
 	musb_config = malloc(sizeof(struct musb_hdrc_config));
 	memset(musb_config, 0, sizeof(struct musb_hdrc_config));
 
-	musb_config->multipoint = fdtdec_get_int(fdt, node,
-						 "mentor,multipoint", -1);
-	if (musb_config->multipoint < 0) {
+	ret = fdtdec_get_int(fdt, node, "mentor,multipoint", -1);
+	if (ret < 0) {
 		pr_err("MUSB multipoint DT entry missing\n");
 		return -ENOENT;
+	} else {
+		musb_config->multipoint = ret;
 	}
 
 	musb_config->dyn_fifo = 1;
 
-	musb_config->num_eps = fdtdec_get_int(fdt, node, "mentor,num-eps",
-					      -1);
-	if (musb_config->num_eps < 0) {
+	ret = fdtdec_get_int(fdt, node, "mentor,num-eps", -1);
+	if (ret < 0) {
 		pr_err("MUSB num-eps DT entry missing\n");
 		return -ENOENT;
+	} else {
+		musb_config->num_eps = ret;
 	}
 
-	musb_config->ram_bits = fdtdec_get_int(fdt, node, "mentor,ram-bits",
-					       -1);
-	if (musb_config->ram_bits < 0) {
+	ret = fdtdec_get_int(fdt, node, "mentor,ram-bits", -1);
+	if (ret < 0) {
 		pr_err("MUSB ram-bits DT entry missing\n");
 		return -ENOENT;
+	} else {
+		musb_config->ram_bits = ret;
 	}
 
 	plat->plat.config = musb_config;
 
-	plat->plat.power = fdtdec_get_int(fdt, node, "mentor,power", -1);
-	if (plat->plat.power < 0) {
+	ret = fdtdec_get_int(fdt, node, "mentor,power", -1);
+	if (ret < 0) {
 		pr_err("MUSB mentor,power DT entry missing\n");
 		return -ENOENT;
+	} else {
+		plat->plat.power = ret;
 	}
 
 	plat->plat.platform_ops = &musb_dsps_ops;

-- 
2.47.3