linux-kernel - Re: [RESEND PATCH v1] proc: use untagged_addr() for pagemap

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] [day] [month] [year] [list]

Message-ID: <1606447013.8845.5.camel@mtkswgap22>
Date:   Fri, 27 Nov 2020 11:16:53 +0800
From:   Miles Chen <miles.chen@...iatek.com>
To:     Catalin Marinas <catalin.marinas@....com>
CC:     Alexey Dobriyan <adobriyan@...il.com>,
        Andrew Morton <akpm@...ux-foundation.org>,
        Linux Kernel Mailing List <linux-kernel@...r.kernel.org>,
        <linux-fsdevel@...r.kernel.org>,
        <linux-mediatek@...ts.infradead.org>, <wsd_upstream@...iatek.com>,
        <andreyknvl@...gle.com>
Subject: Re: [RESEND PATCH v1] proc: use untagged_addr() for pagemap_read
 addresses

On Thu, 2020-11-26 at 11:10 +0000, Catalin Marinas wrote:
> Hi Miles,
> 
> Could you please cc me and Andrey Konovalov on future versions of this
> patch (if any)?
> 
> On Mon, 23 Nov 2020 at 08:47, Miles Chen <miles.chen@...iatek.com> wrote:
> > When we try to visit the pagemap of a tagged userspace pointer, we find
> > that the start_vaddr is not correct because of the tag.
> > To fix it, we should untag the usespace pointers in pagemap_read().
> >
> > I tested with 5.10-rc4 and the issue remains.
> >
> > My test code is baed on [1]:
> >
> > A userspace pointer which has been tagged by 0xb4: 0xb400007662f541c8
> >
> > === userspace program ===
> >
> > uint64 OsLayer::VirtualToPhysical(void *vaddr) {
> >         uint64 frame, paddr, pfnmask, pagemask;
> >         int pagesize = sysconf(_SC_PAGESIZE);
> >         off64_t off = ((uintptr_t)vaddr) / pagesize * 8; // off = 0xb400007662f541c8 / pagesize * 8 = 0x5a00003b317aa0
> 
> Arguably, that's a user-space bug since tagged file offsets were never
> supported. In this case it's not even a tag at bit 56 as per the arm64
> tagged address ABI but rather down to bit 47. You could say that the
> problem is caused by the C library (malloc()) or whoever created the
> tagged vaddr and passed it to this function. It's not a kernel
> regression as we've never supported it.

thanks for the explaination.
> 
> Now, pagemap is a special case where the offset is usually not
> generated as a classic file offset but rather derived by shifting a
> user virtual address. I guess we can make a concession for pagemap
> (only) and allow such offset with the tag at bit (56 - PAGE_SHIFT +
> 3).
> 
> Please fix the patch as per Eric's suggestion on avoiding the
> overflow. You should also add a Cc: stable v5.4- as that's when we
> enabled the tagged address ABI on arm64 and when it's more likely for
> the C library/malloc() to start generating such pointers.

Got it, thanks for your reviewing and suggestion. I will follow Eric's
suggestion and submit patch v2 and cc stable v5.4-

Miles
> 
> If the problem is only limited to this test, I'd rather fix the user
> but I can't tell how widespread the /proc/pid/pagemap usage is.
> 
> Thanks.
>