linux-kernel - Re: [PATCH] serial: core: Fix double fetch in uart_throttle/uart

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: Fri, 12 Jan 2024 14:45:54 +0100
From: Greg KH <gregkh@...uxfoundation.org>
To: Gui-Dong Han <2045gemini@...il.com>
Cc: jirislaby@...nel.org, ilpo.jarvinen@...ux.intel.com, tony@...mide.com,
	l.sanfilippo@...bus.com, john.ogness@...utronix.de,
	tglx@...utronix.de, andriy.shevchenko@...ux.intel.com,
	linux-kernel@...r.kernel.org, linux-serial@...r.kernel.org,
	baijiaju1990@...look.com, stable@...r.kernel.org
Subject: Re: [PATCH] serial: core: Fix double fetch in
 uart_throttle/uart_unthrottle

On Fri, Jan 12, 2024 at 08:18:44PM +0800, Gui-Dong Han wrote:
> In uart_throttle() and uart_unthrottle():
>     if (port->status & mask) {
>         port->ops->throttle/unthrottle(port);
>         mask &= ~port->status;
>     }
>     // Code segment utilizing the mask value to determine UART behavior
> 
> In uart_change_line_settings():
>     uart_port_lock_irq(uport);
>     // Code segment responsible for updating uport->status
>     uart_port_unlock_irq(uport);
> 
> In the uart_throttle() and uart_unthrottle() functions, there is a double
> fetch issue due to concurrent execution with uart_change_line_settings().
> In uart_throttle() and uart_unthrottle(), the check
> if (port->status & mask) is made, followed by mask &= ~port->status,
> where the relevant bits are cleared. However, port->status may be modified
> in uart_change_line_settings(). The current implementation does not ensure
> atomicity in the access and modification of port->status and mask. This
> can result in mask being updated based on a modified port->status value,
> leading to improper UART actions.

What would be modifying the status and mask at the same point in time?
Are you sure that it is possible do this?

> This possible bug is found by an experimental static analysis tool
> developed by our team, BassCheck[1]. This tool analyzes the locking APIs
> to extract function pairs that can be concurrently executed, and then
> analyzes the instructions in the paired functions to identify possible
> concurrency bugs including data races and atomicity violations. The above
> possible bug is reported when our tool analyzes the source code of
> Linux 5.17.

5.17 is VERY old and obsolete, please work against 6.7 at the oldest.
No one can take a patch for 5.17 anymore, you know this :(

thanks,

greg k-h