linux-kernel - Re: [linux-next:master] [mm] 47325a5c88: WARNING:at_mm/slub.c:#free_large

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: <75bc4bf4-afe0-a1e0-36bb-1fad7ecee210@google.com>
Date: Wed, 10 Jul 2024 12:14:55 -0700 (PDT)
From: Hugh Dickins <hughd@...gle.com>
To: Usama Arif <usamaarif642@...il.com>
cc: Hugh Dickins <hughd@...gle.com>, kernel test robot <oliver.sang@...el.com>, 
    Andrew Morton <akpm@...ux-foundation.org>, 
    Johannes Weiner <hannes@...xchg.org>, oe-lkp@...ts.linux.dev, 
    lkp@...el.com, Linux Memory Management List <linux-mm@...ck.org>, 
    Chengming Zhou <chengming.zhou@...ux.dev>, 
    Yosry Ahmed <yosryahmed@...gle.com>, Nhat Pham <nphamcs@...il.com>, 
    David Hildenbrand <david@...hat.com>, "Huang, Ying" <ying.huang@...el.com>, 
    Matthew Wilcox <willy@...radead.org>, 
    Shakeel Butt <shakeel.butt@...ux.dev>, Andi Kleen <ak@...ux.intel.com>, 
    linux-kernel@...r.kernel.org, ltp@...ts.linux.it
Subject: Re: [linux-next:master] [mm] 47325a5c88:
 WARNING:at_mm/slub.c:#free_large_kmalloc

On Wed, 10 Jul 2024, Usama Arif wrote:
> On 10/07/2024 21:49, Hugh Dickins wrote:
> > It's a long time since I was active hereabouts, but the bot report
> > and your flurry of updates make me think that you should step back,
> > slow down, and look more carefully at the precedents here.
> > 
> > IIRC, the main problem is that parts of the swap_info_struct can
> > still be in use from before while you're wanting to set up new values.
> > Observe how alloc_swap_info() may return a fresh or an old allocation.
> > Observe how enable_swap_info() is called after getting swapon_mutex
> > late in swapon(), once past all possiblities of error.
> > 
> > I expect that your new zeromap needs to be taking the same care as is
> > taken with swap_map and cluster_info: to be safe, follow their example
> > in both swapon() and swapoff().
> > 
> > Hugh
> 
> Thanks, yeah sent too many in quick succession :). Will be more careful next time.
> 
> Both the 2nd and 3rd version are careful to solve the problem of using old allocation
> which you described.
> 
> The 2nd one takes care of it in the same way as swap_map.

It didn't look like that: it set "p->zeromap = zeromap" as soon as
zeromap had been allocated; whereas swap_map is only installed later
via enable_swap_info().

> 
> But I believe its unnecessary to do all that change in the 2nd version, when you can just
> set it to NULL after kvfree, which is a much smaller change and takes care of reusing old
> allocation equally well.

It's possible that there's a good reason why zeromap does not need the
same care as swap_map; and it's possible that things have changed down
the years and swap_map itself doesn't even need all that care.

But you haven't persuaded me: I repeat, step back, slow down,
think carefully about why the existing sequence is as it is
(and please don't respond without doing so).

Hugh