Re: [patch 24/54] mm/mmap.c: do not allow mappings outside of allowed limits

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Sun, Jun 7, 2020 at 9:41 PM Andrew Morton <akpm@xxxxxxxxxxxxxxxxxxxx> wrote:
>
> One can set a lowest possible address in /proc/sys/vm/mmap_min_addr
> and mmap below that bound nevertheless.

Well, /proc/sys/vm/mmap_min_addr actually sets "dac_mmap_min_addr"
directly, and then indirectly sets mmap_min_addr with slightly
different rules.

> It is possible to request a fixed mapping address below mmap_min_addr and
> succeed.  This update adds early checks of mmap_min_addr and mmap_end
> boundaries and fixes the above issue.
>
> Apart from it is wrong I am not aware of any existing issue.

Hmm. I think your patch is generally fine, although a few things worries me:

 - the "mmap_end" check seems wrong. It should be something like

        if (len > mmap_end || addr > mmap_end-len)

   shouldn't it?

 - the limit we _do_ test is that "dac_mmap_min_addr", and we allow
lower limits for CAP_SYS_RAWIO

 - I think this will break vm86 mode on 32-bit x86. Have you tested that?

In other words, I think the reason we don't do that hard check of
mmap_min_addr is exactly that vm86 issue. If we were to force a hard
check, we're now making it impossible to use vm86 mode.

So I'm dropping this patch, because it is not clear that this was
fully thought through.

And if it *was* intentional, and people knew about the vm86 issues,
and the thing about dac_mmap_min_addr and the check in
cap_mmap_addr(), then it should be mentioned in the commit message.

That said, I'd be more than willing to move the "cap_mmap_addr()"
check into the mm layer and make it a whole lot more obvious.

And I'd also be more than willing to really make the difference
between "dac_mmap_min_addr" and "mmap_min_addr" actually meaningful,
and really enforce "mmap_min_addr", because right now it's a
half-baked feature that isn't actually used for anything but hinting
and the grow-down issue.

And that, in turn, is because of those vm86 issues, but also because
we've historically had programs that wanted some legacy behavior and
did a mmap() of a zero-page at the 0 address (because that's how some
old Unix environments worked - I htink legacy parisc or similar).

            Linus




[Index of Archives]     [Linux ARM Kernel]     [Linux ARM]     [Linux Omap]     [Fedora ARM]     [IETF Annouce]     [Bugtraq]     [Linux OMAP]     [Linux MIPS]     [eCos]     [Asterisk Internet PBX]     [Linux API]

  Powered by Linux