linux-kernel - Re: [PATCH v1] timerqueue: use rb_entry_safe() in timerqueue

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: <87v8nis83n.ffs@tglx>
Date:   Mon, 14 Nov 2022 01:17:00 +0100
From:   Thomas Gleixner <tglx@...utronix.de>
To:     Barnabás Pőcze <pobrn@...tonmail.com>,
        linux-kernel@...r.kernel.org
Cc:     John Stultz <jstultz@...gle.com>
Subject: Re: [PATCH v1] timerqueue: use rb_entry_safe() in timerqueue_getnext()

On Thu, Oct 27 2022 at 21:37, Barnabás Pőcze wrote:

> When `timerqueue_getnext()` is called on an empty timerqueue
> the returned rb_node pointer will be NULL. Using `rb_entry()`
> on a potentially NULL pointer will only - coincidentally - work
> if the offset of the rb_node member is 0. This is currently the
> case for `timerqueue_node`, but should this ever change,
> `timerqueue_getnext()` will no longer work correctly on empty
> timerqueues. To avoid this, switch to using `rb_entry_safe()`.

I agree with the change but not with the argumentation above.

Whatever the current offset of node is does not matter at all,
really. This is a blantant missing NULL pointer check which works by
chance.

So there is no weasel wording justfied about "coincidentally" and "might
not longer work correctly".

Just spell it out that this is a blantant bug and nothing else.

Back then when that code got introduced rb_entry_safe() did not exist at
all so it's even more obvious that this is simply a missing NULL pointer
check, right?

This also requires a Fixes: tag which flags the commit which introduces
this bug.

Thanks,

        tglx