linux-kernel - Re: [PATCH V2 3/4] posix-timers: Initialise timer->it_signal in posix_timer

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] [thread-next>] [day] [month] [year] [list]

Message-ID: <87bjuwiu11.ffs@tglx>
Date: Thu, 20 Feb 2025 15:13:14 +0100
From: Thomas Gleixner <tglx@...utronix.de>
To: Eric Dumazet <edumazet@...gle.com>
Cc: Anna-Maria Behnsen <anna-maria@...utronix.de>, Frederic Weisbecker
 <frederic@...nel.org>, linux-kernel <linux-kernel@...r.kernel.org>,
 Benjamin Segall <bsegall@...gle.com>, Eric Dumazet
 <eric.dumazet@...il.com>
Subject: Re: [PATCH V2 3/4] posix-timers: Initialise timer->it_signal in
 posix_timer_add()

On Thu, Feb 20 2025 at 09:44, Eric Dumazet wrote:
> On Thu, Feb 20, 2025 at 9:19 AM Thomas Gleixner <tglx@...utronix.de> wrote:
>> > This fixes a potential race, in the unlikely case a thread
>> > was preempted long enough that other threads created more than
>> > 2^31 itimers.
>>
>> and then what happens?
>
> Two threads might get the same timer_id given back.

I know that, but how will someone who reads that changelog without the
knowledge and background information know?

That's the whole point of change logs to explain it for the uninformed
reader, no?

>> >
>> >       spin_lock_irq(&current->sighand->siglock);
>> > -     /* This makes the timer valid in the hash table */
>> > +     /* This makes the timer valid in the hash table, clearing low order bit. */
>>
>> Clearing the low order bit of what? This is a full write and not a clear
>> low order bit operation.
>>
>
> Prior value was (sig | 1L)
>
> New value is (sig)
>
> -> low order bit is cleared.

Right I know, but again it's not obvious without figuring out from some
other place what the logic behind this is.

Thanks,

        tglx