linux-kernel - Re: [PATCH] mm: Update do_vmi_align

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: <20230630164135.725ewvttype5tt6x@revolver>
Date:   Fri, 30 Jun 2023 12:41:35 -0400
From:   "Liam R. Howlett" <Liam.Howlett@...cle.com>
To:     Linus Torvalds <torvalds@...ux-foundation.org>
Cc:     Matthew Wilcox <willy@...radead.org>, linux-mm@...ck.org,
        linux-kernel@...r.kernel.org,
        Andrew Morton <akpm@...ux-foundation.org>
Subject: Re: [PATCH] mm: Update do_vmi_align_munmap() return semantics

* Linus Torvalds <torvalds@...ux-foundation.org> [230630 12:19]:
> On Fri, 30 Jun 2023 at 09:06, Liam R. Howlett <Liam.Howlett@...cle.com> wrote:
> >
> > Update do_vmi_align_munmap() to return 0 for success.  Clean up the
> > callers and comments to always expect the lock downgrade to be honored
> > on the success path.  The error path will always leave the lock
> > untouched.
> 
> Thanks for doing this, but with this cleanup, it becomes clear that
> some of the callers that asked for a downgrade didn't actually want
> that at all...

...

> 
> I didn't look at what all the indirect callers here were doing, but it
> really looked to me like *most* callers wanted the lock dropped
> entirely at the end.
> 
> In fact, looking at that patch, it looks like *all* of the callers
> that asked for downgrading actually really wanted the lock dropped
> entirely.
> 
> But I may well be missing some context.  So take this not as a NAK,
> but as a "you looked at all this code, could it perhaps be simplified
> a bit more still?"
> 

Sure, I noticed that but didn't want to change too much, at least in one
commit.  I'll double check and will keep cleaning here.

I'm not sure how many error scenarios need the lock at all on return
either.

I hesitate to ask considering how much trouble I've caused with the
32bit map flag, but I also wonder about the stack guard now that the
write lock is taken for stack expansion?

Thanks,
Liam