linux-kernel - Re: [PATCH 01/18] serial: sc16is7xx: fix segfault when removing driver

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: <ZYMJhKvGAFgj9phL@smile.fi.intel.com>
Date: Wed, 20 Dec 2023 17:34:28 +0200
From: Andy Shevchenko <andriy.shevchenko@...el.com>
To: Hugo Villeneuve <hugo@...ovil.com>
Cc: gregkh@...uxfoundation.org, jirislaby@...nel.org, jringle@...dpoint.com,
	kubakici@...pl, phil@...pberrypi.org, bo.svangard@...eddedart.se,
	linux-kernel@...r.kernel.org, linux-serial@...r.kernel.org,
	Hugo Villeneuve <hvilleneuve@...onoff.com>, stable@...r.kernel.org
Subject: Re: [PATCH 01/18] serial: sc16is7xx: fix segfault when removing
 driver

On Tue, Dec 19, 2023 at 12:18:45PM -0500, Hugo Villeneuve wrote:
> From: Hugo Villeneuve <hvilleneuve@...onoff.com>
> 
> If a problem with a device occurs during probing, and we subsequently
> try to remove the driver, we get the following error:
> 
> $ rmmod sc16is7xx
> [   62.783166] Unable to handle kernel NULL pointer dereference at virtual address 0000000000000040
> [   62.994226] Call trace:
> [   62.996672]  serial_core_unregister_port+0x58/0x2b0
> [   63.001558]  serial_ctrl_unregister_port+0x18/0x30
> [   63.006354]  uart_remove_one_port+0x18/0x30
> [   63.010542]  sc16is7xx_spi_remove+0xc0/0x190 [sc16is7xx]
> Segmentation fault
> 
> Tested on a custom board with two SC16IS742 ICs, and by simulating a fault
> when probing channel (port) B of the second device.
> 
> The reason is that uart_remove_one_port() has already been called during
> probe in the out_ports: error path, and is called again in
> sc16is7xx_remove().
> 
> Fix the problem by clearing the device drvdata in probe error path to
> indicate that all resources have been deallocated. And only deallocate
> resources in sc16is7xx_remove() if device drvdata is non-null.

...

> +	dev_set_drvdata(dev, NULL);

I believe this is wrong approach to fix the issue as this one is prone
to be cleaned up in the future as we don't do this call explicitly for
the past ~15 years.

-- 
With Best Regards,
Andy Shevchenko