Re: Linux kernel: memory leak in arch/powerpc/platforms/powernv/opal-irqchip.c: opal_event_init()

[Solar Designer <solar () openwall com>]

Hi,

This bug report is misaddressed.  Per upstream's preference and common
sense (given how many issue reports there are against the Linux kernel),
most Linux kernel (maybe-)issues should first be reported to Linux
kernel maintainers/lists or (if you're reasonably sure of significant
security relevance) to the Linux kernel security team, and only then (if
relevant) maybe also to general security lists.  Occasional exceptions
exist, such as for publicly exploited issues, but if you're new to this
chances are that you should play by the rules, not claim an exception.

The original Subject line on this message was just "memory leak".  That
was also inappropriate since it should contain at least the affected
project's name (when applicable).  I've edited the Subject line prior to
approving this message (as a moderator).

2639161967 also sent this message to linux-distros.  That was also
inappropriate for the reasons stated above (where it's not just
upstream's preference, but also part of the current instructions for
issue reporters), and additionally because linux-distros is only for
non-public issues, so it makes no sense to send anything to the public
oss-security and the private linux-distros at the same time.

On Mon, Sep 02, 2024 at 09:54:52AM +0800, 2639161967 wrote:
> in the newest linux release version, in /arch/powerpc/platforms/powernv/opal-irqchip.c file , the 
> opal_event_init function, the variable "name"defined in line 270, and is alloced memory in line 274 or 276, but not 
> free, cause many times memory leak, and most old release versions have the problem.

The code in question is:

int __init opal_event_init(void)
{
[...]
        /* Install interrupt handlers */
        for (i = 0; i < opal_irq_count; i++) {
                struct resource *r = &opal_irqs[i];
                const char *name;

                /* Prefix name */
                if (r->name && strlen(r->name))
                        name = kasprintf(GFP_KERNEL, "opal-%s", r->name);
                else
                        name = kasprintf(GFP_KERNEL, "opal");

                if (!name)
                        continue;
                /* Install interrupt handler */
                rc = request_irq(r->start, opal_interrupt, r->flags & IRQD_TRIGGER_MASK,
                                 name, NULL);
                if (rc) {
                        pr_warn("Error %d requesting OPAL irq %d\n", rc, (int)r->start);
                        continue;
                }
        }

As I understand, the "memory leak" is actually a non-issue, because
opal_event_init() is only called on initialization and request_irq()
retains a pointer to name.  So this is part of building data structures
that will normally remain around during the system's uptime.  Even if
this were an unintentional memory leak, it's pretty clearly not a
security issue because the leak would not be attacker-triggerable.

Perhaps name could reasonably be freed when request_irq() fails, so
inside the "if (rc) {" block.  Perhaps with some effort, these could
also be freed in opal_event_shutdown(), but it's safer not to bother.
Perhaps a source code comment may reasonably be added.

This "problem" report could be slightly less useless if you included the
methodology you used to find this, so that others could adjust it to
find and report real issues, even if usually non-security ones (so not
report them to here, but to appropriate Linux kernel maintainers and
lists, as per the MAINTAINERS file).

I'm CC'ing this to some maintainers in case they want to add to the "if
(rc) {" block, if only to silence static analysis tools, but other than
that I see nothing to do on this report.

Alexander