Re: [PATCH v2 2/3] dt-bindings: serial: maxim,max310x: describe per-channel rs485 subnodes

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:

Pre-existing issues:
- [High] Kernel panic in error path due to uninitialized port structures in `max310x_probe()`
--

commit 8da77c1c050ab79e593fff225d7174a5db92f3bd
Author: Tapio Reijonen <tapio.reijonen@vaisala.com>

dt-bindings: serial: maxim,max310x: describe per-channel rs485 subnodes

The MAX310x is a family of one- (max3107, max3108), two- (max3109) and
four-channel (max14830) UARTs. The binding pulls in
/schemas/serial/rs485.yaml at the chip level, describing a single set of
RS-485 properties - enough for the single-channel parts, but a
multi-channel chip can wire RS-485 differently on each channel.

> diff --git a/Documentation/devicetree/bindings/serial/maxim,max310x.yaml b/Documentation/devicetree/bindings/serial/maxim,max310x.yaml
> index 889eeaca64a02..988864e795741 100644
> --- a/Documentation/devicetree/bindings/serial/maxim,max310x.yaml
> +++ b/Documentation/devicetree/bindings/serial/maxim,max310x.yaml
> @@ -40,6 +40,36 @@ properties:
>      minItems: 1
>      maxItems: 16
>  
> +  "#address-cells":
> +    const: 1

[ ... ]

[Severity: High]
This isn't a bug introduced by this patch, but could the error path in
max310x_probe() cause a NULL pointer dereference?

In drivers/tty/serial/max310x.c, the driver allocates the port state structure
using devm_kzalloc(), meaning s->p[i].port.line is zero-initialized.

If the probe fails early before any channels are registered, the function
jumps to the out_uart cleanup label:

drivers/tty/serial/max310x.c:max310x_probe() {
    ...
out_uart:
    for (i = 0; i < devtype->nr; i++) {
        if (test_and_clear_bit(s->p[i].port.line, max310x_lines))
            uart_remove_one_port(&max310x_uart, &s->p[i].port);
    }
    ...
}

Because s->p[i].port.line is still 0, this tests bit 0 in max310x_lines.

If another max310x device on the system has already successfully probed, it
will have claimed line 0, so bit 0 in max310x_lines will be set.

Would test_and_clear_bit(0, max310x_lines) then succeed, incorrectly clear
the bit owned by the other device, and call uart_remove_one_port() with an
uninitialized port?

This appears to lead to a NULL pointer dereference when
serial_core_unregister_port() calls serial_core_get_ctrl_dev(port_dev),
dereferencing port_dev->dev.parent since port_dev is NULL for the uninitialized
port.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260601-b4-max310x-rs485-dt-v2-0-a105105f8e70@vaisala.com?part=2