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]
Date: Mon, 8 Jan 2024 10:26:07 -0500
From: Alan Stern <stern@...land.harvard.edu>
To: Udipto Goswami <quic_ugoswami@...cinc.com>
Cc: Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
  Krishna Kurapati <quic_kriskura@...cinc.com>, linux-usb@...r.kernel.org,
  linux-kernel@...r.kernel.org, stable@...r.kernel.org
Subject: Re: [PATCH v2] usb: core: Prevent null pointer dereference in
 update_port_device_state

On Mon, Jan 08, 2024 at 06:37:06PM +0530, Udipto Goswami wrote:
> Currently,the function update_port_device_state gets the usb_hub from
> udev->parent by calling usb_hub_to_struct_hub.
> However, in case the actconfig or the maxchild is 0, the usb_hub would
> be NULL and upon further accessing to get port_dev would result in null
> pointer dereference.
> 
> Fix this by introducing an if check after the usb_hub is populated.
> 
> Fixes: 83cb2604f641 ("usb: core: add sysfs entry for usb device state")
> Cc: stable@...r.kernel.org
> Signed-off-by: Udipto Goswami <quic_ugoswami@...cinc.com>
> ---
> v2: Introduced comment for the if check & CC'ed stable.
> 
>  drivers/usb/core/hub.c | 15 ++++++++++++---
>  1 file changed, 12 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/usb/core/hub.c b/drivers/usb/core/hub.c
> index ffd7c99e24a3..d40b5500f95b 100644
> --- a/drivers/usb/core/hub.c
> +++ b/drivers/usb/core/hub.c
> @@ -2053,9 +2053,18 @@ static void update_port_device_state(struct usb_device *udev)
>  
>  	if (udev->parent) {
>  		hub = usb_hub_to_struct_hub(udev->parent);
> -		port_dev = hub->ports[udev->portnum - 1];
> -		WRITE_ONCE(port_dev->state, udev->state);
> -		sysfs_notify_dirent(port_dev->state_kn);
> +
> +		/*
> +		 * usb_hub_to_struct_hub() if returns NULL can
> +		 * potentially cause NULL pointer dereference upon further
> +		 * access.
> +		 * Avoid this with an if check.
> +		 */

This is not what I meant.  It's perfectly obvious that if 
usb_hub_to_struct_hub() returns NULL then there will be a NULL-pointer 
dereference.  You don't need to explain that to anybody.

Instead, you need to explain why it is _possible_ for 
usb_hub_to_struct_hub() to return NULL.  The reason is because the 
lvstest driver messes around with usbcore internals without telling the 
hub driver, so hub will be NULL in cases where udev was created by 
lvstest.

Alan Stern

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ