linux-kernel - Re: [PATCH v3 4/4] fs/sysv: Replace kmap() with kmap_local

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 for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <63cb9ce63db7e_c68ad29473@iweiny-mobl.notmuch>
Date:   Sat, 21 Jan 2023 00:05:58 -0800
From:   Ira Weiny <ira.weiny@...el.com>
To:     Helge Deller <deller@....de>, Al Viro <viro@...iv.linux.org.uk>,
        "Matthew Wilcox" <willy@...radead.org>
CC:     "Fabio M. De Francesco" <fmdefrancesco@...il.com>,
        Christoph Hellwig <hch@...radead.org>,
        <linux-kernel@...r.kernel.org>, <linux-fsdevel@...r.kernel.org>,
        Ira Weiny <ira.weiny@...el.com>, <linux-parisc@...r.kernel.org>
Subject: Re: [PATCH v3 4/4] fs/sysv: Replace kmap() with kmap_local_page()

Helge Deller wrote:
> On 1/20/23 06:56, Al Viro wrote:
> > On Fri, Jan 20, 2023 at 05:07:48AM +0000, Al Viro wrote:
> >> On Fri, Jan 20, 2023 at 04:54:51AM +0000, Matthew Wilcox wrote:
> >>
> >>>> Sure, but... there's also this:
> >>>>
> >>>> static inline void __kunmap_local(const void *addr)
> >>>> {
> >>>> #ifdef ARCH_HAS_FLUSH_ON_KUNMAP
> >>>>          kunmap_flush_on_unmap(addr);
> >>>> #endif
> >>>> }
> >>>>
> >>>> Are you sure that the guts of that thing will be happy with address that is not
> >>>> page-aligned?  I've looked there at some point, got scared of parisc (IIRC)
> >>>> MMU details and decided not to rely upon that...
> >>>
> >>> Ugh, PA-RISC (the only implementor) definitely will flush the wrong
> >>> addresses.  I think we should do this, as having bugs that only manifest
> >>> on one not-well-tested architecture seems Bad.
> >>>
> >>>   static inline void __kunmap_local(const void *addr)
> >>>   {
> >>>   #ifdef ARCH_HAS_FLUSH_ON_KUNMAP
> >>> -       kunmap_flush_on_unmap(addr);
> >>> +       kunmap_flush_on_unmap(PAGE_ALIGN_DOWN(addr));
> >>>   #endif
> >>>   }
> >>
> >> PTR_ALIGN_DOWN(addr, PAGE_SIZE), perhaps?
> >
> > 	Anyway, that's a question to parisc folks; I _think_ pdtlb
> > quietly ignores the lower bits of address, so that part seems
> > to be safe, but I wouldn't bet upon that.
> 
> No, on PA2.0 (64bit) CPUs the lower bits of the address of pdtlb
> encodes the amount of memory (page size) to be flushed, see:
> http://ftp.parisc-linux.org/docs/arch/parisc2.0.pdf (page 7-106)
> So, the proposed page alignment with e.g. PTR_ALIGN_DON() is needed.
> 
> Helge

I'm not sure I completely understand.

First, arn't PAGE_ALIGN_DOWN(addr) and PTR_ALIGN_DOWN(addr, PAGE_SIZE) the
same?

align.h
#define PTR_ALIGN_DOWN(p, a)    ((typeof(p))ALIGN_DOWN((unsigned long)(p), (a)))

mm.h:
#define PAGE_ALIGN_DOWN(addr) ALIGN_DOWN(addr, PAGE_SIZE)

Did parisc redefine it somewhere I'm not seeing?

Second, if the lower bits encode the amount of memory to be flushed is it
required to return the original value returned from page_address()?

Ira