• Gui-Dong Han's avatar
    serial: core: Fix atomicity violation in uart_tiocmget · 30926783
    Gui-Dong Han authored
    In uart_tiocmget():
        result = uport->mctrl;
        uart_port_lock_irq(uport);
        result |= uport->ops->get_mctrl(uport);
        uart_port_unlock_irq(uport);
        ...
        return result;
    
    In uart_update_mctrl():
        uart_port_lock_irqsave(port, &flags);
        ...
        port->mctrl = (old & ~clear) | set;
        ...
        port->ops->set_mctrl(port, port->mctrl);
        ...
        uart_port_unlock_irqrestore(port, flags);
    
    An atomicity violation is identified due to the concurrent execution of
    uart_tiocmget() and uart_update_mctrl(). After assigning
    result = uport->mctrl, the mctrl value may change in uart_update_mctrl(),
    leading to a mismatch between the value returned by
    uport->ops->get_mctrl(uport) and the mctrl value previously read.
    This can result in uart_tiocmget() returning an incorrect value.
    
    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.
    
    To address this issue, it is suggested to move the line
    result = uport->mctrl inside the uart_port_lock block to ensure atomicity
    and prevent the mctrl value from being altered during the execution of
    uart_tiocmget(). With this patch applied, our tool no longer reports the
    bug, with the kernel configuration allyesconfig for x86_64. Due to the
    absence of the requisite hardware, we are unable to conduct runtime
    testing of the patch. Therefore, our verification is solely based on code
    logic analysis.
    
    [1] https://sites.google.com/view/basscheck/
    
    Fixes: c5f4644e ("[PATCH] Serial: Adjust serial locking")
    Cc: stable@vger.kernel.org
    Signed-off-by: default avatarGui-Dong Han <2045gemini@gmail.com>
    Link: https://lore.kernel.org/r/20240112113624.17048-1-2045gemini@gmail.comSigned-off-by: default avatarGreg Kroah-Hartman <gregkh@linuxfoundation.org>
    30926783
serial_core.c 92.6 KB