linux-kernel - Re: [PATCH 1/2] usbip: Fix seqnum sign extension issue in vhci_tx

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <5ad65964-dcf7-47d2-8e5f-bf82066e55f4@linuxfoundation.org>
Date: Thu, 2 Jan 2025 15:06:50 -0700
From: Shuah Khan <skhan@...uxfoundation.org>
To: Xiong Nandi <xndchn@...il.com>,
 Valentina Manea <valentina.manea.m@...il.com>, Shuah Khan
 <shuah@...nel.org>, Hongren Zheng <i@...ithal.me>,
 Greg Kroah-Hartman <gregkh@...uxfoundation.org>
Cc: "open list:USB OVER IP DRIVER" <linux-usb@...r.kernel.org>,
 open list <linux-kernel@...r.kernel.org>,
 Shuah Khan <skhan@...uxfoundation.org>
Subject: Re: [PATCH 1/2] usbip: Fix seqnum sign extension issue in vhci_tx_urb

On 12/31/24 09:15, Xiong Nandi wrote:
> The atomic_inc_return function returns an int, while priv->seqnum is an
> unsigned long. So we must cast the result to u32 to prevent potential
> sign extension and size mismatch issues.
> 

How did you find the problem?
> Signed-off-by: Xiong Nandi <xndchn@...il.com>
> ---
>   drivers/usb/usbip/vhci_hcd.c | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/usb/usbip/vhci_hcd.c b/drivers/usb/usbip/vhci_hcd.c
> index b03e5021c25b..f3f260e01791 100644
> --- a/drivers/usb/usbip/vhci_hcd.c
> +++ b/drivers/usb/usbip/vhci_hcd.c
> @@ -675,7 +675,7 @@ static void vhci_tx_urb(struct urb *urb, struct vhci_device *vdev)
>   
>   	spin_lock_irqsave(&vdev->priv_lock, flags);
>   
> -	priv->seqnum = atomic_inc_return(&vhci_hcd->seqnum);
> +	priv->seqnum = (u32)atomic_inc_return(&vhci_hcd->seqnum);

Why does it make sense to cast it to u32?

>   	if (priv->seqnum == 0xffff)
>   		dev_info(&urb->dev->dev, "seqnum max\n");


thanks,
-- Shuah