[PATCH] drm/omap: check IRQ wait allocation

[PATCH] drm/omap: check IRQ wait allocation

[Ruoyu Wang]

omap_irq_wait_init() allocates an omap_irq_wait object and initializes
its waitqueue and list node immediately. If the allocation fails, the
waitqueue initialization dereferences NULL.

Return NULL when the wait object cannot be allocated and make
omap_irq_wait() report -ENOMEM if it is asked to wait on a missing
object. No wait entry has been installed in that case, so there is
nothing to remove from the wait list.

Signed-off-by: Ruoyu Wang <ruoyuw560@gmail.com>
---
 drivers/gpu/drm/omapdrm/omap_irq.c | 6 ++++++
 1 file changed, 6 insertions(+)

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);
 	unsigned long flags;
 
+	if (!wait)
+		return NULL;
+
 	init_waitqueue_head(&wait->wq);
 	wait->irqmask = irqmask;
 	wait->count = count;
@@ -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);
 
 	spin_lock_irqsave(&priv->wait_lock, flags);
-- 
2.34.1

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

[sashiko-bot]

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