linux-kernel - Re: [PATCH] ssb: Fix potential NULL pointer dereference in ssb_device

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 for Android: free password hash cracker in your pocket

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

Message-ID: <CAOiHx==hd09hfzgkthp+BUsTaUFPM0YyJOkPrBOzLL4wz8cQ7w@mail.gmail.com>
Date: Thu, 29 Feb 2024 14:40:53 +0100
From: Jonas Gorski <jonas.gorski@...il.com>
To: Rand Deeb <rand.sec96@...il.com>
Cc: Michael Buesch <m@...s.ch>, linux-wireless@...r.kernel.org, linux-kernel@...r.kernel.org, 
	deeb.rand@...fident.ru, lvc-project@...uxtesting.org, 
	voskresenski.stanislav@...fident.ru
Subject: Re: [PATCH] ssb: Fix potential NULL pointer dereference in ssb_device_uevent

Hi,

On Thu, 29 Feb 2024 at 10:38, Rand Deeb <rand.sec96@...il.com> wrote:
>
> The ssb_device_uevent function first attempts to convert the 'dev' pointer
> to 'struct ssb_device *'. However, it mistakenly dereferences 'dev' before
> performing the NULL check, potentially leading to a NULL pointer
> dereference if 'dev' is NULL.
>
> To fix this issue, this patch moves the NULL check before dereferencing the
> 'dev' pointer, ensuring that the pointer is valid before attempting to use
> it.

Might be worth pointing out that dev_to_ssb_dev() does dereference
dev, in contrast to most (dev_)to_*_dev() helpers that just calculate
a new pointer from an offset via container_of(), and thus are a-okay
with NULL pointers (but I think this would be UB), or even explicitly
return NULL if the passed dev is NULL.

Though I wonder if dev can even be NULL at this point, or if the NULL
check is actually bogus and could be dropped.

AFAICT the caller of this function would be dev_uevent(), and it does it here:

        /* have the bus specific function add its stuff */
        if (dev->bus && dev->bus->uevent) {
                retval = dev->bus->uevent(dev, env);

which can only be possible if dev is non-NULL.

I can't really tell if uevent_show() would also call this function,
but even that one dereferences dev before calling uevent().

So from a first glance I would think dev is guaranteed to be non-NULL.

> (snip)

Best Regards,
Jonas