linux-kernel - Re: [PATCH] mm: slub: reinitialize random sequence cache on slab object update

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: <da7f86610add1ea78234dfc88178472e@codeaurora.org>
Date:   Thu, 05 Mar 2020 11:18:55 +0530
From:   vjitta@...eaurora.org
To:     Vlastimil Babka <vbabka@...e.cz>
Cc:     cl@...ux.com, penberg@...nel.org, rientjes@...gle.com,
        iamjoonsoo.kim@....com, akpm@...ux-foundation.org,
        linux-mm@...ck.org, linux-kernel@...r.kernel.org,
        vinmenon@...eaurora.org, kernel-team@...roid.com
Subject: Re: [PATCH] mm: slub: reinitialize random sequence cache on slab
 object update

On 2020-02-27 22:23, Vlastimil Babka wrote:
> On 1/30/20 12:17 PM, vjitta@...eaurora.org wrote:
>> From: Vijayanand Jitta <vjitta@...eaurora.org>
>> 
>> Random sequence cache is precomputed during slab object creation
>> based up on the object size and no of objects per slab. These could
>> be changed when flags like SLAB_STORE_USER, SLAB_POISON are updated
>> from sysfs. So when shuffle_freelist is called during slab_alloc it
>> uses updated object count to access the precomputed random sequence
>> cache. This could result in incorrect access of the random sequence
>> cache which could further result in slab corruption. Fix this by
>> reinitializing the random sequence cache up on slab object update.
>> 
>> A sample panic trace when write to slab_store_user was attempted.
> 
> A more complete oops report would have been better, e.g. if anyone was 
> googling
> it, to find this patch.
> 
> Also I was checking where else calculate_sizes() is called and found
> order_store(). So if somebody changes (especially increases) the order,
> shouldn't the reinitialization also be done?

Yes, reinitialization must be done here aswell , will update the patch.

> 
> This is even more nasty as it doesn't seem to require that no objects 
> exist.
> Also there is no synchronization against concurrent allocations/frees? 
> Gasp.

Since, random sequence cache is only used to update the freelist in 
shuffle_freelist
which is done only when a new slab is created incase if objects 
allocations are
done without a need of new slab creation they will use the existing 
freelist which
should be fine as object size doesn't change after order_store() and 
incase if a new
slab is created we will get the updated freelist. so in both cases i 
think it should
be fine.