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