Re: [PATCH 1/3] serial: 8250_mxpcie: set the driver data before registering ports

[Linmao Li]

在 2026/8/19 17:07, Crescent Hsieh 写道:
> On Tue, Aug 18, 2026 at 05:39:16PM +0800, Linmao Li wrote:
> > mxpcie8250_rs485_config() looks the board up with dev_get_drvdata() on
> > the PCI device, but pci_set_drvdata() only runs after the registration
> > loop.  Where mxpcie8250_setup_port() presets rs485.flags to
> > SER_RS485_ENABLED, uart_configure_port() calls ->rs485_config() from
> > inside serial8250_register_8250_port(), and the callback dereferences a
> > NULL board pointer.
> >
> > Publish the driver data before the first port is registered.
> >
> > Fixes: d21a1509c623 ("serial: 8250_mxpcie: support serial interface mode switching")
> > Signed-off-by: Linmao Li <lilinmao@xxxxxxxxxx>
> Hi,
>
> Thanks for pointing this out.
>
> I tried to reproduce the NULL pointer dereference with a CP-134EL-A on
> an ACPI x86 system, but the callback was not reached during port
> registration.
>
> The driver initially sets the RS485 flags to 0x201
> (SER_RS485_ENABLED | SER_RS485_MODE_RS422) and directly programs the
> hardware interface for RS422. However, uart_get_rs485_mode() finds an
> ACPI firmware node and changes the flags to 0x202 because the node does
> not provide the linux,rs485-enabled-at-boot-time property.
Thank you for putting a board on it.

Your analysis matches what the code does: serial8250_register_8250_port()
calls uart_get_rs485_mode() before uart_add_one_port(), and since
6974711cf770 ("serial: Keep rs485 settings for devices without firmware
node") that helper leaves the driver's flags alone only when there is no
firmware node.  With one it clears SER_RS485_ENABLED, and sets it again
only for linux,rs485-enabled-at-boot-time, which your node does not have
- so uart_rs485_config() returns before ->rs485_config() and nothing
dereferences the NULL driver data.

So what the crash needs is an RS422/RS485-only board and
SER_RS485_ENABLED still set once uart_get_rs485_mode() has run: either
the device has no firmware node, or its node carries
linux,rs485-enabled-at-boot-time.  v2 says that in the commit message;
the change itself stays as it is.

I agree the state mismatch you describe is a separate issue.  It looks
like a driver-side decision - mxpcie8250_setup_port() programs the
interface for RS422 and presets rs485.flags, and only the flags are then
cleared - so I would rather leave it to you than guess which of the two
should win.

Thanks,
Linmao
> uart_configure_port() subsequently calls uart_rs485_config(), but the
> latter returns without calling mxpcie8250_rs485_config() because
> SER_RS485_ENABLED is no longer set. Therefore, the NULL driver-data
> pointer is not dereferenced on this system.
>
> This also leaves the software state inconsistent with the hardware
> after initialization. The hardware is configured for RS422, while
> TIOCGRS485 reports SER_RS485_MODE_RS422 without SER_RS485_ENABLED.
>
> I also reviewed commit 6974711cf770 ("serial: Keep rs485 settings for
> devices without firmware node") and its review discussion [1]. That
> change preserves driver-provided defaults when no firmware node exists.
> In this case, however, the PCI device has a generic ACPI firmware node
> without any RS485 properties.
>
> The driver-data ordering change still looks correct to me because
> SER_RS485_ENABLED is preserved on systems without a firmware node,
> allowing mxpcie8250_rs485_config() to be called during port
> registration. The case where an ACPI firmware node exists but provides
> no RS485 properties appears to be a separate issue that also needs to
> be addressed.
>
> [1]
> https://lore.kernel.org/all/20251023151229.11774-2-gerhard@xxxxxxxxxxxxxxxxxxxxx/
>
> ---
> Thanks,
> Crescent Hsieh