[PATCH -next] serial: core: fix NULL/dangling port_dev on failed re-register

[PATCH -next] serial: core: fix NULL/dangling port_dev on failed re-register

[Gaosheng Cui]

From: Cui GaoSheng <cuigaosheng1@huawei.com>

serial8250_unregister_port() removes a port and then re-registers it
against the legacy serial8250 device when that device is still
present, ignoring the return value of uart_add_one_port(). When the
re-register fails, the error path of serial_core_register_port()
removes the port device but leaves port->port_dev dangling. A port
that was never successfully registered (e.g. serial_core_port_device_add()
failed) also reaches the unregister path with port->port_dev unset.
serial8250 matches ports on ->dev, so such half-registered slots
still reach serial_core_unregister_port() on the next unbind:

  Oops: general protection fault ... SMP KASAN PTI
  KASAN: null-ptr-deref in range [0x40-0x47]
  RIP: serial_core_unregister_port+0xe5/0xa00
    serial8250_unregister_port+0x103
    serial8250_remove+0x61
    device_release_driver_internal+0x38c
    unbind_store+0xd1
    kernfs_fop_write_iter+0x329
    vfs_write+0x5d0
    ksys_write+0xf7
    do_syscall_64+0xdd
    entry_SYSCALL_64_after_hwframe+0x77

Fix the error path to clear port->port_dev and restore the state slot
(guarded by line range) to the "not registered" state, and bail out of
serial_core_unregister_port() when port_dev is NULL (either dangling
from a failed re-register or never set).

Fixes: 84a9582fd203 ("serial: core: Start managing serial controllers to enable runtime PM")
Signed-off-by: Cui GaoSheng <cuigaosheng1@huawei.com>
---
 drivers/tty/serial/serial_core.c | 25 ++++++++++++++++++++++++-
 1 file changed, 24 insertions(+), 1 deletion(-)

diff --git a/drivers/tty/serial/serial_core.c b/drivers/tty/serial/serial_core.c
index 95774b0f1484..62d93c5964e2 100644
--- a/drivers/tty/serial/serial_core.c
+++ b/drivers/tty/serial/serial_core.c
@@ -3358,6 +3358,17 @@ int serial_core_register_port(struct uart_driver *drv, struct uart_port *port)
 
 err_unregister_port_dev:
 	serial_base_port_device_remove(port->port_dev);
+	/*
+	 * serial_base_port_device_remove() dropped the last reference to the
+	 * port device, so leave no dangling ->port_dev behind. Restore the
+	 * state slot to the same "not registered" state that
+	 * serial_core_remove_one_port() produces on the normal path; the
+	 * line range guard also covers the -EINVAL exit of
+	 * serial_core_add_one_port() where line >= drv->nr.
+	 */
+	port->port_dev = NULL;
+	if (port->line < drv->nr && drv->state[port->line].uart_port == port)
+		drv->state[port->line].uart_port = NULL;
 
 err_unregister_ctrl_dev:
 	serial_base_ctrl_device_remove(new_ctrl_dev);
@@ -3373,11 +3384,23 @@ void serial_core_unregister_port(struct uart_driver *drv, struct uart_port *port
 {
 	struct device *phys_dev = port->dev;
 	struct serial_port_device *port_dev = port->port_dev;
-	struct serial_ctrl_device *ctrl_dev = serial_core_get_ctrl_dev(port_dev);
+	struct serial_ctrl_device *ctrl_dev;
 	int ctrl_id = port->ctrl_id;
 
+	/*
+	 * A port may be unregistered even though it was never successfully
+	 * registered (e.g. an allocation failure aborted serial_core_register_port()
+	 * part-way). Drivers like serial8250 remove ports matching on ->dev, so
+	 * those half-registered ports still reach us with port_dev == NULL. Do not
+	 * dereference it.
+	 */
+	if (!port_dev)
+		return;
+
 	guard(mutex)(&port_mutex);
 
+	ctrl_dev = serial_core_get_ctrl_dev(port_dev);
+
 	port->flags |= UPF_DEAD;
 
 	serial_core_remove_one_port(drv, port);
-- 
2.43.0

Re: [PATCH -next] serial: core: fix NULL/dangling port_dev on failed re-register

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] Unsafe state clearing in error path causes data race and resource leak on double-registration
--

commit 18ff022090858881e0c42eddefd1a71c7ed91334
Author: Cui GaoSheng <cuigaosheng1@huawei.com>

serial: core: fix NULL/dangling port_dev on failed re-register

Fixes a general protection fault in the serial unregister path by explicitly
clearing port->port_dev and bailing out if it is NULL. It also clears the
UART port slot in the driver state upon registration failure.

> diff --git a/drivers/tty/serial/serial_core.c b/drivers/tty/serial/serial_core.c
> index 95774b0f1484b..62d93c5964e25 100644
> --- a/drivers/tty/serial/serial_core.c
> +++ b/drivers/tty/serial/serial_core.c
> @@ -3358,6 +3358,17 @@ int serial_core_register_port(struct uart_driver *drv, struct uart_port *port)
>  
>  err_unregister_port_dev:
>  	serial_base_port_device_remove(port->port_dev);
> +	/*
> +	 * serial_base_port_device_remove() dropped the last reference to the
> +	 * port device, so leave no dangling ->port_dev behind. Restore the
> +	 * state slot to the same "not registered" state that
> +	 * serial_core_remove_one_port() produces on the normal path; the
> +	 * line range guard also covers the -EINVAL exit of
> +	 * serial_core_add_one_port() where line >= drv->nr.
> +	 */
> +	port->port_dev = NULL;
> +	if (port->line < drv->nr && drv->state[port->line].uart_port == port)
> +		drv->state[port->line].uart_port = NULL;

[Severity: High]
Can this error path clobber an active registration if a driver incorrectly
attempts to register an already-registered port?

If a driver double-registers the same port, serial_core_add_one_port() will
fail and return -EINVAL because state->uart_port is already set. The error
path here then evaluates drv->state[port->line].uart_port == port as true
due to the original successful registration. 

Does this forcibly clear drv->state[port->line].uart_port without acquiring
the required port->mutex, creating a data race and destroying the state of
the active registration? 

When the driver later calls uart_remove_one_port(), will it return early
because uart_port is NULL, potentially leaking tty devices, sysfs nodes,
and memory allocations?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260914135154.3260303-1-cuigaosheng1@huawei.com?part=1

Re: [PATCH -next] serial: core: fix NULL/dangling port_dev on failed re-register

[Greg KH]

On Mon, Sep 14, 2026 at 09:51:54PM +0800, Gaosheng Cui wrote:
> From: Cui GaoSheng <cuigaosheng1@huawei.com>
> 
> serial8250_unregister_port() removes a port and then re-registers it
> against the legacy serial8250 device when that device is still
> present, ignoring the return value of uart_add_one_port(). When the
> re-register fails, the error path of serial_core_register_port()
> removes the port device but leaves port->port_dev dangling. A port
> that was never successfully registered (e.g. serial_core_port_device_add()
> failed) also reaches the unregister path with port->port_dev unset.
> serial8250 matches ports on ->dev, so such half-registered slots
> still reach serial_core_unregister_port() on the next unbind:
> 
>   Oops: general protection fault ... SMP KASAN PTI
>   KASAN: null-ptr-deref in range [0x40-0x47]
>   RIP: serial_core_unregister_port+0xe5/0xa00
>     serial8250_unregister_port+0x103
>     serial8250_remove+0x61
>     device_release_driver_internal+0x38c
>     unbind_store+0xd1
>     kernfs_fop_write_iter+0x329
>     vfs_write+0x5d0
>     ksys_write+0xf7
>     do_syscall_64+0xdd
>     entry_SYSCALL_64_after_hwframe+0x77
> 
> Fix the error path to clear port->port_dev and restore the state slot
> (guarded by line range) to the "not registered" state, and bail out of
> serial_core_unregister_port() when port_dev is NULL (either dangling
> from a failed re-register or never set).
> 
> Fixes: 84a9582fd203 ("serial: core: Start managing serial controllers to enable runtime PM")
> Signed-off-by: Cui GaoSheng <cuigaosheng1@huawei.com>
> ---

Did you forget the Assited-by: tag?


>  drivers/tty/serial/serial_core.c | 25 ++++++++++++++++++++++++-
>  1 file changed, 24 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/tty/serial/serial_core.c b/drivers/tty/serial/serial_core.c
> index 95774b0f1484..62d93c5964e2 100644
> --- a/drivers/tty/serial/serial_core.c
> +++ b/drivers/tty/serial/serial_core.c
> @@ -3358,6 +3358,17 @@ int serial_core_register_port(struct uart_driver *drv, struct uart_port *port)
>  
>  err_unregister_port_dev:
>  	serial_base_port_device_remove(port->port_dev);
> +	/*
> +	 * serial_base_port_device_remove() dropped the last reference to the
> +	 * port device, so leave no dangling ->port_dev behind. Restore the
> +	 * state slot to the same "not registered" state that
> +	 * serial_core_remove_one_port() produces on the normal path; the
> +	 * line range guard also covers the -EINVAL exit of
> +	 * serial_core_add_one_port() where line >= drv->nr.
> +	 */
> +	port->port_dev = NULL;
> +	if (port->line < drv->nr && drv->state[port->line].uart_port == port)
> +		drv->state[port->line].uart_port = NULL;


How was this tested?


>  
>  err_unregister_ctrl_dev:
>  	serial_base_ctrl_device_remove(new_ctrl_dev);
> @@ -3373,11 +3384,23 @@ void serial_core_unregister_port(struct uart_driver *drv, struct uart_port *port
>  {
>  	struct device *phys_dev = port->dev;
>  	struct serial_port_device *port_dev = port->port_dev;
> -	struct serial_ctrl_device *ctrl_dev = serial_core_get_ctrl_dev(port_dev);
> +	struct serial_ctrl_device *ctrl_dev;
>  	int ctrl_id = port->ctrl_id;
>  
> +	/*
> +	 * A port may be unregistered even though it was never successfully
> +	 * registered (e.g. an allocation failure aborted serial_core_register_port()
> +	 * part-way). Drivers like serial8250 remove ports matching on ->dev, so
> +	 * those half-registered ports still reach us with port_dev == NULL. Do not
> +	 * dereference it.
> +	 */
> +	if (!port_dev)
> +		return;

What keeps port_dev from not changing right after you read it?

thanks,

greg k-h

Re: [PATCH -next] serial: core: fix NULL/dangling port_dev on failed re-register

[cuigaosheng]

Thanks for the review, I have submitted v2 of the patch.

I have tested on x86_64 linux-next (7.2.0-rc7) with KASAN, failslab and
fault injection debugfs enabled;

It takes two unbind rounds to reproduce since
serial8250_unregister_port() first unregisters and then re-registers
the port: the fault injection must hit the re-registration to plant
the stale port_dev, and only the next unbind dereferences it. The
reproducer is a small userspace program that loops over sysfs
bind/unbind of serial8250 and scans /proc/self/fail-nth from 1 to
6000, making the Nth slab allocation on the re-register path fail
with -ENOMEM:

   bind; fail_nth=N; unbind; fail_nth=0; bind; unbind

For fail-nth to reach this path, failslab's ignore-gfp-wait needs
to be cleared first, otherwise GFP_KERNEL allocations are skipped
before the fail-nth check and nothing gets injected.

Without the patch the scan hits the oops quoted in the commit
message. With the patch the full 6000-iteration scan completes
without any crash, the FAULT_INJECTION debug output in dmesg
confirms the injections were actually hitting the path, and the
ports keep working afterwards since a failed re-register is simply
retried on the next bind/unbind round.

Thanks,

On 2026/9/23 20:52, Greg KH wrote:
> On Mon, Sep 14, 2026 at 09:51:54PM +0800, Gaosheng Cui wrote:
>> From: Cui GaoSheng <cuigaosheng1@huawei.com>
>>
>> serial8250_unregister_port() removes a port and then re-registers it
>> against the legacy serial8250 device when that device is still
>> present, ignoring the return value of uart_add_one_port(). When the
>> re-register fails, the error path of serial_core_register_port()
>> removes the port device but leaves port->port_dev dangling. A port
>> that was never successfully registered (e.g. serial_core_port_device_add()
>> failed) also reaches the unregister path with port->port_dev unset.
>> serial8250 matches ports on ->dev, so such half-registered slots
>> still reach serial_core_unregister_port() on the next unbind:
>>
>>    Oops: general protection fault ... SMP KASAN PTI
>>    KASAN: null-ptr-deref in range [0x40-0x47]
>>    RIP: serial_core_unregister_port+0xe5/0xa00
>>      serial8250_unregister_port+0x103
>>      serial8250_remove+0x61
>>      device_release_driver_internal+0x38c
>>      unbind_store+0xd1
>>      kernfs_fop_write_iter+0x329
>>      vfs_write+0x5d0
>>      ksys_write+0xf7
>>      do_syscall_64+0xdd
>>      entry_SYSCALL_64_after_hwframe+0x77
>>
>> Fix the error path to clear port->port_dev and restore the state slot
>> (guarded by line range) to the "not registered" state, and bail out of
>> serial_core_unregister_port() when port_dev is NULL (either dangling
>> from a failed re-register or never set).
>>
>> Fixes: 84a9582fd203 ("serial: core: Start managing serial controllers to enable runtime PM")
>> Signed-off-by: Cui GaoSheng <cuigaosheng1@huawei.com>
>> ---
> Did you forget the Assited-by: tag?
>
>
>>   drivers/tty/serial/serial_core.c | 25 ++++++++++++++++++++++++-
>>   1 file changed, 24 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/tty/serial/serial_core.c b/drivers/tty/serial/serial_core.c
>> index 95774b0f1484..62d93c5964e2 100644
>> --- a/drivers/tty/serial/serial_core.c
>> +++ b/drivers/tty/serial/serial_core.c
>> @@ -3358,6 +3358,17 @@ int serial_core_register_port(struct uart_driver *drv, struct uart_port *port)
>>   
>>   err_unregister_port_dev:
>>   	serial_base_port_device_remove(port->port_dev);
>> +	/*
>> +	 * serial_base_port_device_remove() dropped the last reference to the
>> +	 * port device, so leave no dangling ->port_dev behind. Restore the
>> +	 * state slot to the same "not registered" state that
>> +	 * serial_core_remove_one_port() produces on the normal path; the
>> +	 * line range guard also covers the -EINVAL exit of
>> +	 * serial_core_add_one_port() where line >= drv->nr.
>> +	 */
>> +	port->port_dev = NULL;
>> +	if (port->line < drv->nr && drv->state[port->line].uart_port == port)
>> +		drv->state[port->line].uart_port = NULL;
>
> How was this tested?
>
>
>>   
>>   err_unregister_ctrl_dev:
>>   	serial_base_ctrl_device_remove(new_ctrl_dev);
>> @@ -3373,11 +3384,23 @@ void serial_core_unregister_port(struct uart_driver *drv, struct uart_port *port
>>   {
>>   	struct device *phys_dev = port->dev;
>>   	struct serial_port_device *port_dev = port->port_dev;
>> -	struct serial_ctrl_device *ctrl_dev = serial_core_get_ctrl_dev(port_dev);
>> +	struct serial_ctrl_device *ctrl_dev;
>>   	int ctrl_id = port->ctrl_id;
>>   
>> +	/*
>> +	 * A port may be unregistered even though it was never successfully
>> +	 * registered (e.g. an allocation failure aborted serial_core_register_port()
>> +	 * part-way). Drivers like serial8250 remove ports matching on ->dev, so
>> +	 * those half-registered ports still reach us with port_dev == NULL. Do not
>> +	 * dereference it.
>> +	 */
>> +	if (!port_dev)
>> +		return;
> What keeps port_dev from not changing right after you read it?
>
> thanks,
>
> greg k-h
>
> .

Re: [PATCH -next] serial: core: fix NULL/dangling port_dev on failed re-register

[Greg KH]

On Thu, Sep 24, 2026 at 09:28:23PM +0800, cuigaosheng wrote:
> Thanks for the review, I have submitted v2 of the patch.

Great, but please do not top-post, you just lost all relevant
information :(

> I have tested on x86_64 linux-next (7.2.0-rc7) with KASAN, failslab and
> fault injection debugfs enabled;
> 
> It takes two unbind rounds to reproduce since
> serial8250_unregister_port() first unregisters and then re-registers
> the port: the fault injection must hit the re-registration to plant
> the stale port_dev, and only the next unbind dereferences it.

What do you mean by this?  What fault injection and why do we care about
that if it can never hit in real life?

> The
> reproducer is a small userspace program that loops over sysfs
> bind/unbind of serial8250 and scans /proc/self/fail-nth from 1 to
> 6000, making the Nth slab allocation on the re-register path fail
> with -ENOMEM:

bind/unbind is not a normal operation that a user can do, and is for
debugging only.  So are you sure this is a real issue?

thanks,

greg k-h