linux-kernel - Re: [PATCH v2 1/2] maple_tree: Disable mas_wr_append() when other readers are possible

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: <87v8cv22jh.fsf@mail.lhotse>
Date:   Thu, 31 Aug 2023 15:39:46 +1000
From:   Michael Ellerman <mpe@...erman.id.au>
To:     Geert Uytterhoeven <geert@...ux-m68k.org>,
        "Liam R. Howlett" <Liam.Howlett@...cle.com>
Cc:     Andrew Morton <akpm@...ux-foundation.org>,
        maple-tree@...ts.infradead.org, linux-mm@...ck.org,
        linux-kernel@...r.kernel.org, stable@...r.kernel.org,
        linux-renesas-soc@...r.kernel.org
Subject: Re: [PATCH v2 1/2] maple_tree: Disable mas_wr_append() when other
 readers are possible

Geert Uytterhoeven <geert@...ux-m68k.org> writes:
>  	Hi Liam,
>
> On Fri, 18 Aug 2023, Liam R. Howlett wrote:
>> The current implementation of append may cause duplicate data and/or
>> incorrect ranges to be returned to a reader during an update.  Although
>> this has not been reported or seen, disable the append write operation
>> while the tree is in rcu mode out of an abundance of caution.
>>
>> During the analysis of the mas_next_slot() the following was
>> artificially created by separating the writer and reader code:
>>
>> Writer:                                 reader:
>> mas_wr_append
>>    set end pivot
>>    updates end metata
>>    Detects write to last slot
>>    last slot write is to start of slot
>>    store current contents in slot
>>    overwrite old end pivot
>>                                        mas_next_slot():
>>                                                read end metadata
>>                                                read old end pivot
>>                                                return with incorrect range
>>    store new value
>>
>> Alternatively:
>>
>> Writer:                                 reader:
>> mas_wr_append
>>    set end pivot
>>    updates end metata
>>    Detects write to last slot
>>    last lost write to end of slot
>>    store value
>>                                        mas_next_slot():
>>                                                read end metadata
>>                                                read old end pivot
>>                                                read new end pivot
>>                                                return with incorrect range
>>    set old end pivot
>>
>> There may be other accesses that are not safe since we are now updating
>> both metadata and pointers, so disabling append if there could be rcu
>> readers is the safest action.
>>
>> Fixes: 54a611b60590 ("Maple Tree: add new data structure")
>> Cc: stable@...r.kernel.org
>> Signed-off-by: Liam R. Howlett <Liam.Howlett@...cle.com>
>
> Thanks for your patch, which is now commit cfeb6ae8bcb96ccf
> ("maple_tree: disable mas_wr_append() when other readers are
> possible") in v6.5, and is being backported to stable.
>
> On Renesas RZ/A1 and RZ/A2 (single-core Cortex-A9), this causes the
> following warning:
>
>       clocksource: timer@...3b000: mask: 0xffffffff max_cycles: 0xffffffff, max_idle_ns: 28958491609 ns
>       sched_clock: 32 bits at 66MHz, resolution 15ns, wraps every 32537631224ns
>       /soc/timer@...3b000: used for clocksource
>       /soc/timer@...3c000: used for clock events
>      +------------[ cut here ]------------
>      +WARNING: CPU: 0 PID: 0 at init/main.c:992 start_kernel+0x2f0/0x480
>      +Interrupts were enabled early
...
>
> I do not see this issue on any other platform
> (arm/arm64/risc-v/mips/sh/m68k), several of them use the same
> RCU configuration.

There's something similar on pmac32 / mac99.

> Do you have a clue?

It seems something in the maple tree code is setting TIF_NEED_RESCHED,
and that causes a subsequent call to cond_resched() to call schedule()
and enable interrupts.

On pmac32 enabling CONFIG_DEBUG_ATOMIC_SLEEP fixes/hides the problem.
But I don't see why.

cheers