Fix uninitialized `ops` member's pointers to avoid kernel Oops in `irq_sim_request_resources()`.

Signed-off-by: Gyeyoung Baek <gye976@gmail.com>
---
 kernel/irq/irq_sim.c | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/kernel/irq/irq_sim.c b/kernel/irq/irq_sim.c
index 1a3d483548e2..67fd1de5d197 100644
--- a/kernel/irq/irq_sim.c
+++ b/kernel/irq/irq_sim.c
@@ -222,8 +222,12 @@ struct irq_domain *irq_domain_create_sim_full(struct fwnode_handle *fwnode,
 	work_ctx->pending = no_free_ptr(pending);
 	work_ctx->user_data = data;
 
-	if (ops)
+	if (ops) {
 		memcpy(&work_ctx->ops, ops, sizeof(*ops));
+	} else {
+		work_ctx->ops.irq_sim_irq_released = NULL;
+		work_ctx->ops.irq_sim_irq_requested = NULL;
+	}
 
 	return no_free_ptr(work_ctx)->domain;
 }
-- 
2.43.0

On Tue, May 27 2025 at 18:35, Gyeyoung Baek wrote:

The subject line prefix is wrong. See

https://www.kernel.org/doc/html/latest/process/maintainer-tip.html#patch-subject

> Fix uninitialized `ops` member's pointers to avoid kernel Oops in

You cannot fix an uninitialized pointer. You only can initialize it
properly.

> `irq_sim_request_resources()`.

No backticks required. fun() is clear on it's own.

Also please describe how this ends up with an oops in
irq_sim_request_resources(). The point is that any dereference of an
uninitialized pointer is resulting in a problem and it does not matter
where.

Dereferencing an uninitialized pointer can cause an Ooops or worse it
can call into some random code when the uninitialized memory contained a
valid pointer, which is way harder to debug than a plain crash.

> index 1a3d483548e2..67fd1de5d197 100644
> --- a/kernel/irq/irq_sim.c
> +++ b/kernel/irq/irq_sim.c
> @@ -222,8 +222,12 @@ struct irq_domain *irq_domain_create_sim_full(struct fwnode_handle *fwnode,
>  	work_ctx->pending = no_free_ptr(pending);
>  	work_ctx->user_data = data;
>  
> -	if (ops)
> +	if (ops) {
>  		memcpy(&work_ctx->ops, ops, sizeof(*ops));
> +	} else {
> +		work_ctx->ops.irq_sim_irq_released = NULL;
> +		work_ctx->ops.irq_sim_irq_requested = NULL;
> +	}

The obvious fix is way more simple. Just allocate work_ctx with
kzalloc() instead of kmalloc(), no?

Thanks,

        tglx

Hi Thomas, thanks for your review.

On Wed, Jun 11, 2025 at 3:39 PM Thomas Gleixner <tglx@linutronix.de> wrote:
>
> On Tue, May 27 2025 at 18:35, Gyeyoung Baek wrote:
>
> The subject line prefix is wrong. See
>
> https://www.kernel.org/doc/html/latest/process/maintainer-tip.html#patch-subject

Thank you for providing the guideline, I see now how to.

> > Fix uninitialized `ops` member's pointers to avoid kernel Oops in
>
> You cannot fix an uninitialized pointer. You only can initialize it
> properly.

Yes, I will update the subject to 'Initialize properly' in v2.

---

> Also please describe how this ends up with an oops in
> irq_sim_request_resources(). The point is that any dereference of an
> uninitialized pointer is resulting in a problem and it does not matter
> where.

Yes, It looks good to just remove irq_sim_request_resources() from the
commit message.

> Dereferencing an uninitialized pointer can cause an Ooops or worse it
> can call into some random code when the uninitialized memory contained a
> valid pointer, which is way harder to debug than a plain crash.

Yes, then I will remove irq_sim_request_resources() from the commit message,
and explain it consistently with the subject.

---

> > -     if (ops)
> > +     if (ops) {
> >               memcpy(&work_ctx->ops, ops, sizeof(*ops));
> > +     } else {
> > +             work_ctx->ops.irq_sim_irq_released = NULL;
> > +             work_ctx->ops.irq_sim_irq_requested = NULL;
> > +     }
>
> The obvious fix is way more simple. Just allocate work_ctx with
> kzalloc() instead of kmalloc(), no?

Yes, that's a much simpler and cleaner fix.
then I will send a v2 patch according to your reviews, Thank you.

--
Best Regards,
Gyeyoung