linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] irq: Fix uninitialized pointers
@ 2025-05-27  9:35 Gyeyoung Baek
  2025-06-11  6:39 ` Thomas Gleixner
  0 siblings, 1 reply; 3+ messages in thread
From: Gyeyoung Baek @ 2025-05-27  9:35 UTC (permalink / raw)
  To: Thomas Gleixner; +Cc: linux-kernel, Gyeyoung Baek

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


^ permalink raw reply related	[flat|nested] 3+ messages in thread

* Re: [PATCH] irq: Fix uninitialized pointers
  2025-05-27  9:35 [PATCH] irq: Fix uninitialized pointers Gyeyoung Baek
@ 2025-06-11  6:39 ` Thomas Gleixner
  2025-06-12 11:41   ` Gyeyoung Baek
  0 siblings, 1 reply; 3+ messages in thread
From: Thomas Gleixner @ 2025-06-11  6:39 UTC (permalink / raw)
  To: Gyeyoung Baek; +Cc: linux-kernel, Gyeyoung Baek

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

^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [PATCH] irq: Fix uninitialized pointers
  2025-06-11  6:39 ` Thomas Gleixner
@ 2025-06-12 11:41   ` Gyeyoung Baek
  0 siblings, 0 replies; 3+ messages in thread
From: Gyeyoung Baek @ 2025-06-12 11:41 UTC (permalink / raw)
  To: Thomas Gleixner; +Cc: linux-kernel

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

^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2025-06-12 11:42 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-05-27  9:35 [PATCH] irq: Fix uninitialized pointers Gyeyoung Baek
2025-06-11  6:39 ` Thomas Gleixner
2025-06-12 11:41   ` Gyeyoung Baek

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).