Linux Serial subsystem development
 help / color / mirror / Atom feed
From: sashiko-bot@kernel.org
To: "Gaosheng Cui" <cuigaosheng1@huawei.com>
Cc: linux-serial@vger.kernel.org
Subject: Re: [PATCH -next] serial: core: fix NULL/dangling port_dev on failed re-register
Date: Mon, 14 Sep 2026 14:23:42 +0000	[thread overview]
Message-ID: <20260914142343.1A3B41F00898@smtp.kernel.org> (raw)
In-Reply-To: <20260914135154.3260303-1-cuigaosheng1@huawei.com>

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

  reply	other threads:[~2026-09-14 14:23 UTC|newest]

Thread overview: 5+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-09-14 13:51 [PATCH -next] serial: core: fix NULL/dangling port_dev on failed re-register Gaosheng Cui
2026-09-14 14:23 ` sashiko-bot [this message]
2026-09-23 12:52 ` Greg KH
2026-09-24 13:28   ` cuigaosheng
2026-09-24 14:37     ` Greg KH

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20260914142343.1A3B41F00898@smtp.kernel.org \
    --to=sashiko-bot@kernel.org \
    --cc=cuigaosheng1@huawei.com \
    --cc=linux-serial@vger.kernel.org \
    --cc=sashiko-reviews@lists.linux.dev \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox