Re: [PATCH v15 4/5] gpio: rpmsg: add generic rpmsg GPIO driver

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 7 potential issue(s) to consider:
- [High] Data race between `gpio_rpmsg_eoi_irq()` and `gpio_rpmsg_eoi_work()` causes lost End-Of-Interrupt (EOI) signals.
- [High] Explicitly disabling an IRQ fails to mask it on the remote hardware and causes an infinite interrupt storm.
- [High] A timeout in `rpmsg_gpio_send_message()` permanently corrupts the request/response synchronization of the RPMSG channel.
- [High] Use-After-Free vulnerability due to incorrect teardown ordering between `devm_gpiochip_add_data`, `cancel_work_sync`, and the RPMSG endpoint.
- [High] Missing `gc->can_sleep = true` causes "scheduling while atomic" panics.
- [Medium] The RPMSG callback does not validate the incoming message length, risking out-of-bounds memory reads.
- [Medium] Implicit padding in `struct rpmsg_gpio_response` breaks cross-platform IPC.
--

--- Patch [4]: [PATCH v15 4/5] gpio: rpmsg: add generic rpmsg GPIO driver ---
commit 1ad498fb1fd9abd6b832044bd985e9e16c8577db
Author: Shenwei Wang <shenwei.wang@nxp.com>
Subject: gpio: rpmsg: add generic rpmsg GPIO driver

On an AMP platform, the system may include multiple processors communicating
via the RPMSG protocol. The driver implements the standard GPIO interface,
allowing the Linux side to control GPIO controllers which reside in the
remote processor via RPMSG protocol.

diff --git a/drivers/gpio/gpio-rpmsg.c b/drivers/gpio/gpio-rpmsg.c
new file mode 100644
index 0000000000000..5d2d59e6314b5
--- /dev/null
+++ b/drivers/gpio/gpio-rpmsg.c
@@ -0,0 +1,587 @@
[ ... ]
> +struct rpmsg_gpio_response {
> +	__u8 type;
> +	union {
> +		/* command reply */
> +		struct {
> +			__u8 status;
> +			__u8 value;
> +		};
> +
> +		/* interrupt notification */
> +		struct {
> +			__le16 line;
> +		};
> +	};
> +};

[Severity: Medium]
Does the implicit padding inserted after type break cross-platform IPC if
the remote firmware uses packed structures or a compiler with different
padding rules?

[ ... ]
> +static int rpmsg_gpio_send_message(struct rpmsg_gpio_port *port)
> +{
[ ... ]
> +	ret = wait_for_completion_timeout(&port->cmd_complete,
> +					  msecs_to_jiffies(RPMSG_TIMEOUT));
> +	if (ret == 0) {
> +		dev_err(&port->rpdev->dev, "rpmsg_send timeout! cmd=%d\n",
> +			port->send_msg->type);
> +		return -ETIMEDOUT;
> +	}

[Severity: High]
If the remote processor replies after this timeout, won't the next request
immediately complete using the stale response?

Since the protocol lacks transaction IDs, a single delayed response might
permanently break the channel synchronization for all future transactions.

[ ... ]
> +static void gpio_rpmsg_irq_bus_sync_unlock(struct irq_data *d)
> +{
> +	struct rpmsg_gpio_port *port = irq_data_get_irq_chip_data(d);
> +	u32 line = d->hwirq;
> +
> +	/*
> +	 * For mask irq, do nothing here.
> +	 * The remote system will mask interrupt after an interrupt occurs,
> +	 * and then send a notification to Linux system. After Linux system
> +	 * handles the notification, it sends an rpmsg back to the remote
> +	 * system to unmask this interrupt again.
> +	 */
> +	if (port->lines[line].irq_mask && !port->lines[line].irq_unmask) {
> +		port->lines[line].irq_mask = 0;
> +		mutex_unlock(&port->lock);
> +		return;
> +	}

[Severity: High]
When Linux explicitly disables an IRQ via disable_irq(), it sets the mask
flag. Will returning early here without sending a hardware mask command
cause the remote processor to endlessly send interrupts, creating an
interrupt storm?

[ ... ]
> +static void gpio_rpmsg_eoi_work(struct work_struct *work)
> +{
> +	struct rpmsg_gpio_port *port =
> +		container_of(work, struct rpmsg_gpio_port, eoi_work);
> +	unsigned long pending[BITS_TO_LONGS(GPIOS_PER_PORT_MAX)];
> +	unsigned int line;
> +
> +	guard(mutex)(&port->lock);
> +
> +	bitmap_copy(pending, port->pending_eoi, port->ngpios);
> +	bitmap_zero(port->pending_eoi, port->ngpios);

[Severity: High]
Can this non-atomic clearing race with gpio_rpmsg_eoi_irq()?

If a new interrupt fires and calls set_bit(d->hwirq, port->pending_eoi)
between this bitmap_copy() and bitmap_zero(), won't the new
end-of-interrupt signal be unconditionally overwritten and lost, locking
up that GPIO line?

[ ... ]
> +static int rpmsg_gpiochip_register(struct rpmsg_device *rpdev, u32 idx,
> +				   struct device_node *np, const char *name)
> +{
[ ... ]
> +	gc->direction_input = rpmsg_gpio_direction_input;
> +	gc->direction_output = rpmsg_gpio_direction_output;
> +	gc->get_direction = rpmsg_gpio_get_direction;
> +	gc->get = rpmsg_gpio_get;
> +	gc->set = rpmsg_gpio_set;

[Severity: High]
Since the get and set callbacks wait on a completion and use a mutex (both
of which can sleep), should gc->can_sleep be set to true?

Without it, gpiolib might allow atomic fast-path accesses, which could
trigger scheduling while atomic panics.

[ ... ]
> +static int rpmsg_gpio_channel_callback(struct rpmsg_device *rpdev, void *data,
> +				       int len, void *priv, u32 src)
> +{
> +	struct rpmsg_gpio_response *msg = data;
> +	struct rpmsg_gpio_port *port = NULL;
> +	u32 line;
> +
> +	port = dev_get_drvdata(&rpdev->dev);
> +
> +	if (!port) {
> +		dev_err(&rpdev->dev, "port is null\n");
> +		return -EINVAL;
> +	}
> +
> +	if (msg->type == GPIO_RPMSG_REPLY) {
> +		*port->recv_msg = *msg;

[Severity: Medium]
Is there a risk of an out-of-bounds read here if the remote processor
sends a malformed message smaller than sizeof(struct rpmsg_gpio_response)?

[ ... ]
> +static void rpmsg_gpio_channel_remove(struct rpmsg_device *rpdev)
> +{
> +	struct rpmsg_gpio_port *port = dev_get_drvdata(&rpdev->dev);
> +
> +	cancel_work_sync(&port->eoi_work);
> +}

[Severity: High]
Does the use of devm_gpiochip_add_data() during registration cause the GPIO
chip to remain active after rpmsg_gpio_channel_remove() completes?

If an interrupt arrives right after this cancel_work_sync(), couldn't the
work be re-queued and executed on freed memory once devres finally cleans
up the port?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260721204704.400781-1-shenwei.wang@oss.nxp.com?part=4