Re: [PATCH 2/6] drm/panel: Add generic MIPI-DSI panel driver with BPF init sequences

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 6 potential issue(s) to consider:
- [High] bpf: Use-After-Free when BPF program detaches from an unbound panel
- [High] bpf: Out-of-bounds read due to missing NUL-termination on user strings
- [High] bpf: Out-of-bounds array access via negative enum indices in kfuncs
- [Medium] drm/panel: Missing hotplug event prevents display pipeline activation
- [Medium] bpf: Uninitialized kernel memory leaked over DSI bus via nullable pointers
- [Low] bpf: Tracepoints log incorrect status data instead of return value
--

commit 1f60161986e7e142490428d5f24fb3b831dddb58
Author: Maxime Ripard <mripard@kernel.org>
Subject: drm/panel: Add generic MIPI-DSI panel driver with BPF init sequences

This patch introduces a generic MIPI-DSI panel driver that uses BPF
programs loaded from userspace for panel-specific behavior. The driver
registers a disconnected drm_bridge, exposing sleepable kfuncs to
control regulators, GPIOs, and DSI data transfer.

> diff --git a/drivers/gpu/drm/panel/bpf/panel-bpf-mipi-dsi-ops.c b/drivers/gpu/drm/panel/bpf/panel-bpf-mipi-dsi-ops.c
> --- /dev/null
> +++ b/drivers/gpu/drm/panel/bpf/panel-bpf-mipi-dsi-ops.c
> @@ -32,25 +32,25 @@
>  static int panel_bpf_mipi_dsi_bpf_init_member(const struct btf_type *t,
>  					      const struct btf_member *member,
>  					      void *kdata, const void *udata)
>  {
>  	const struct drm_panel_dsi_bpf_ops *uops =
>  		(const struct drm_panel_dsi_bpf_ops *)udata;
>  	struct drm_panel_dsi_bpf_ops *kops =
>  		(struct drm_panel_dsi_bpf_ops *)kdata;
>  	u32 moff;
>  
>  	moff = __btf_member_bit_offset(t, member) / 8;
>  
>  	switch (moff) {
>  	case offsetof(struct drm_panel_dsi_bpf_ops, panel_id):
>  		memcpy(kops->panel_id, uops->panel_id,
>  		       sizeof(kops->panel_id));
>  		return 1;
>  	case offsetof(struct drm_panel_dsi_bpf_ops, compatible):
>  		memcpy(kops->compatible, uops->compatible,
>  		       sizeof(kops->compatible));
>  		return 1;

[Severity: High]
Does this memcpy() ensure the string is NUL-terminated? If userspace 
provides a string that is fully packed with non-NUL characters up to 
the length of the array, the kernel string will remain unterminated.

[ ... ]

> @@ -78,12 +78,12 @@
>  static int panel_bpf_mipi_dsi_bpf_check_config(struct panel_bpf_mipi_dsi *panel,
>  					       struct drm_panel_dsi_bpf_ops *ops)
>  {
>  	struct mipi_dsi_device *dsi = panel->dsi;
>  	struct device *dev = &dsi->dev;
>  
>  	if (!of_device_is_compatible(dev->of_node, ops->compatible)) {

[Severity: High]
Can this result in an out-of-bounds read? If ops->compatible was copied from 
userspace without a NUL terminator in panel_bpf_mipi_dsi_bpf_init_member(), 
the OF core functions like of_device_is_compatible() will read past the 
buffer boundary when attempting string comparison.

>  		dev_err(dev,
>  			"BPF compatible \"%s\" doesn't match panel\n",
>  			ops->compatible);
>  		return -EINVAL;
>  	}

[ ... ]

> @@ -134,25 +134,25 @@
>  static int panel_bpf_mipi_dsi_bpf_reg(void *kdata, struct bpf_link *link)
>  {
>  	struct drm_panel_dsi_bpf_ops *ops = kdata;
>  	struct panel_bpf_mipi_dsi *panel;
>  	int ret;
>  
>  	trace_panel_bpf_mipi_dsi_reg(ops->panel_id);
>  
>  	guard(mutex)(&panel_bpf_mipi_dsi_list_lock);
>  
>  	panel = panel_bpf_mipi_dsi_find_panel_unlocked(ops->panel_id);
>  	if (!panel)
>  		return -ENODEV;
>  
>  	guard(mutex)(&panel->bpf_lock);
>  
>  	if (panel->bpf_ops)
>  		return -EBUSY;
>  
>  	ret = panel_bpf_mipi_dsi_bpf_check_config(panel, ops);
>  	if (ret)
>  		return ret;
>  
>  	ops->bridge = &panel->bridge;
>  	panel->bpf_ops = ops;
>  
>  	return 0;
>  }

[Severity: Medium]
When a BPF program successfully attaches here, it changes the bridge's 
detect callback behavior from returning connector_status_disconnected to 
connector_status_connected. Should this function also broadcast a hotplug 
notification to userspace so the display pipeline can be brought up without 
waiting for the next KMS polling cycle?

[ ... ]

> @@ -168,21 +168,21 @@
>  static void panel_bpf_mipi_dsi_bpf_unreg(void *kdata, struct bpf_link *link)
>  {
>  	struct drm_panel_dsi_bpf_ops *ops = kdata;
>  	struct panel_bpf_mipi_dsi *panel;
>  
>  	trace_panel_bpf_mipi_dsi_unreg(ops->panel_id);
>  
>  	if (!ops->bridge)
>  		return;
>  
>  	panel = drm_bridge_to_bpf_panel(ops->bridge);
>  
>  	scoped_guard(mutex, &panel->bpf_lock) {

[Severity: High]
Can this sequence trigger a use-after-free when unregistering? 

If a MIPI DSI device unbinds, devres triggers panel_bpf_mipi_dsi_list_cleanup() 
which removes the panel from the global list but doesn't detach the BPF 
program or clear ops->bridge before freeing the panel structure. 

If userspace subsequently unregisters the BPF struct_ops link, 
panel_bpf_mipi_dsi_bpf_unreg() appears to use the dangling ops->bridge 
pointer to resolve the freed panel structure, and then locks its bpf_lock.

> diff --git a/drivers/gpu/drm/panel/bpf/panel-bpf-mipi-dsi-kfuncs.c b/drivers/gpu/drm/panel/bpf/panel-bpf-mipi-dsi-kfuncs.c
> --- /dev/null
> +++ b/drivers/gpu/drm/panel/bpf/panel-bpf-mipi-dsi-kfuncs.c
> @@ -36,15 +36,15 @@
>  __bpf_kfunc int panel_bpf_mipi_dsi_regulator_enable_and_wait(struct panel_bpf_mipi_dsi_ctx *ctx,
>  							     enum panel_bpf_mipi_dsi_supply supply,
>  							     u32 settle_ms)
>  {
>  	struct panel_bpf_mipi_dsi *panel = bpf_ctx_to_bpf_panel(ctx);
>  	int ret;
>  
>  	if (supply >= PANEL_BPF_MIPI_DSI_SUPPLY_COUNT)
>  		return -EINVAL;

[Severity: High]
Does this bounds check adequately protect against out-of-bounds array accesses?
The BPF verifier validates kfunc enum parameters as signed scalar values,
which could be negative. A negative index here would pass the upper-bound
check and allow an out-of-bounds read into panel->supplies. This same 
pattern exists for the GPIO enum checks elsewhere in this file.

>  
>  	trace_panel_bpf_mipi_dsi_regulator_enable_and_wait(panel->panel_id,
>  							   panel->supplies[supply].supply,
>  							   settle_ms);

[ ... ]

> @@ -177,19 +177,19 @@
>  __bpf_kfunc int panel_bpf_mipi_dsi_dcs_write_and_wait(struct panel_bpf_mipi_dsi_ctx *ctx,
>  						      u8 cmd, const u8 *data__nullable,
>  						      u32 data__nullable__sz,
>  						      u32 settle_ms)
>  {
>  	struct panel_bpf_mipi_dsi *panel = bpf_ctx_to_bpf_panel(ctx);
>  	int ret;
>  
>  	trace_panel_bpf_mipi_dsi_dcs_write_and_wait(panel->panel_id, cmd, data__nullable,
>  						    data__nullable__sz, settle_ms);

[Severity: Low]
Is this tracepoint logging the correct information? The trace event definition 
expects the bus transaction's return value for the fifth argument (ret), but 
this code passes the sleep duration settle_ms into the ret parameter.

>  
>  	ret = mipi_dsi_dcs_write(panel->dsi, cmd, data__nullable,
>  				 data__nullable__sz);

[Severity: Medium]
Could this result in uninitialized kernel memory being leaked over the DSI bus?
When data__nullable is NULL, the BPF verifier doesn't enforce that 
data__nullable__sz is zero. mipi_dsi_dcs_write() will allocate a buffer of 
size data__nullable__sz, skip the memcpy because the source is NULL, and then 
transmit the uninitialized kmalloc buffer to the display. Should there be a 
check for data__nullable == NULL && data__nullable__sz > 0?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260928-drm-mipi-dsi-panel-ebpf-v1-0-5244926aace4@kernel.org?part=2