linux-kernel - Re: [PATCH] irq: Fix uninitialized pointers

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <CAKbEznsu=2O4b-3rHGVtqg=+s28Np2=cL+q8w6TbrkO6RPCVVg@mail.gmail.com>
Date: Thu, 12 Jun 2025 20:41:48 +0900
From: Gyeyoung Baek <gye976@...il.com>
To: Thomas Gleixner <tglx@...utronix.de>
Cc: linux-kernel@...r.kernel.org
Subject: Re: [PATCH] irq: Fix uninitialized pointers

Hi Thomas, thanks for your review.

On Wed, Jun 11, 2025 at 3:39 PM Thomas Gleixner <tglx@...utronix.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