linux-kernel - Re: [PATCHv2] mm: Fix warning in move_normal

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: <20200715205508.3rzrkhulruzpy6iv@box>
Date:   Wed, 15 Jul 2020 23:55:08 +0300
From:   "Kirill A. Shutemov" <kirill@...temov.name>
To:     Linus Torvalds <torvalds@...ux-foundation.org>
Cc:     "Kirill A. Shutemov" <kirill.shutemov@...ux.intel.com>,
        Andrew Morton <akpm@...ux-foundation.org>,
        Linux-MM <linux-mm@...ck.org>,
        Linux Kernel Mailing List <linux-kernel@...r.kernel.org>,
        Naresh Kamboju <naresh.kamboju@...aro.org>,
        Joel Fernandes <joel@...lfernandes.org>,
        William Kucharski <william.kucharski@...cle.com>
Subject: Re: [PATCHv2] mm: Fix warning in move_normal_pmd()

On Wed, Jul 15, 2020 at 11:36:59AM -0700, Linus Torvalds wrote:
> On Wed, Jul 15, 2020 at 6:50 AM Kirill A. Shutemov
> <kirill.shutemov@...ux.intel.com> wrote:
> >
> > mremap(2) does not allow source and destination regions to overlap, but
> > shift_arg_pages() calls move_page_tables() directly and in this case the
> > source and destination overlap often. It
> 
> Actually, before we do this patch (which I think is a good idea), I'd
> like Naresh to test the attached patch.
> 
> And Kirill, Joel, mind looking it over too.

I don't understand 'len' calculation in try_to_align_end().

IIUC, it increases 'len' by PMD_SIZE if 'new_addr+len' is not aligned to
PMD_SIZE. It doesn't make sense to me.

Maybe
	*len = roundup_up(*new_addr + *len, PMD_SIZE) - *new_addr;

or something?

BUT

I *think* there's a bigger problem with the patch:

For stack relocation case both VMAs are the same and always(?) the only
VMA around at the time. It means none of ADDR_BEFORE_PREV and
ADDR_AFTER_NEXT are going to stop us.

Consider the following case, before and after try_to_align_start():

		before		after
old_addr:	0x0123000	0x0000000
new_addr:	0x1123000	0x1000000
len:		0x1000000	0x1123000

(4k PAGE_SIZE, 2M PMD_SIZE)

On the first iteration we would attempt to move 0x0-0x200000 to 
0x1000000-0x1200000 and step onto the same WARN(), no? 

-- 
 Kirill A. Shutemov