Re: [PATCH] drm/omap: check IRQ wait allocation

[sashiko-bot@kernel.org]

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

Pre-existing issues:
- [High] omap_irq_wait_init and omap_irq_wait call sleeping functions (GFP_KERNEL allocation and wait_event_timeout) from encoder atomic enable/disable callbacks.
--

commit ae2889fe92d3ac71e93d1e5708680bea50475102
Author: Ruoyu Wang <ruoyuw560@gmail.com>

drm/omap: check IRQ wait allocation

This commit adds NULL pointer checks to handle memory allocation failures
in omap_irq_wait_init() and omap_irq_wait(), preventing potential NULL
pointer dereferences.

> diff --git a/drivers/gpu/drm/omapdrm/omap_irq.c b/drivers/gpu/drm/omapdrm/omap_irq.c
> index d0f895c43ee41..e0fa9617a3d30 100644
> --- a/drivers/gpu/drm/omapdrm/omap_irq.c
> +++ b/drivers/gpu/drm/omapdrm/omap_irq.c
> @@ -46,6 +46,9 @@ struct omap_irq_wait * omap_irq_wait_init(struct drm_device *dev,
>  	struct omap_irq_wait *wait = kzalloc_obj(*wait);

[Severity: High]
This is a pre-existing issue, but does this allocation sleep in an atomic
context? 

kzalloc_obj() defaults to a GFP_KERNEL allocation, which can sleep. If
omap_irq_wait_init() is called from encoder atomic enable/disable callbacks
(for example, via omap_crtc_set_enabled() during the DSS encoder enable
sequence), calling sleeping functions in an atomic context is forbidden and
can cause system instability or kernel warnings.

>  	unsigned long flags;
>  
> +	if (!wait)
> +		return NULL;
> +
>  	init_waitqueue_head(&wait->wq);

[ ... ]

> @@ -65,6 +68,9 @@ int omap_irq_wait(struct drm_device *dev, struct omap_irq_wait *wait,
>  	unsigned long flags;
>  	int ret;
>  
> +	if (!wait)
> +		return -ENOMEM;
> +
>  	ret = wait_event_timeout(wait->wq, (wait->count <= 0), timeout);

[Severity: High]
This isn't a bug introduced by this patch, but does wait_event_timeout()
sleep here while in an atomic context?

Since wait_event_timeout() is a sleeping function, calling it from encoder
atomic enable/disable callbacks violates DRM atomic context rules. Should
these operations be deferred to a non-atomic context, or should they be
converted to use non-sleeping hardware polling mechanisms instead?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260606040620.13-1-ruoyuw560@gmail.com?part=1