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]
Date:   Mon, 14 Nov 2022 08:40:13 -0800
From:   Dave Hansen <dave.hansen@...el.com>
To:     Michael Kelley <mikelley@...rosoft.com>, hpa@...or.com,
        kys@...rosoft.com, haiyangz@...rosoft.com, wei.liu@...nel.org,
        decui@...rosoft.com, luto@...nel.org, peterz@...radead.org,
        davem@...emloft.net, edumazet@...gle.com, kuba@...nel.org,
        pabeni@...hat.com, lpieralisi@...nel.org, robh@...nel.org,
        kw@...ux.com, bhelgaas@...gle.com, arnd@...db.de,
        hch@...radead.org, m.szyprowski@...sung.com, robin.murphy@....com,
        thomas.lendacky@....com, brijesh.singh@....com, tglx@...utronix.de,
        mingo@...hat.com, bp@...en8.de, dave.hansen@...ux.intel.com,
        Tianyu.Lan@...rosoft.com, kirill.shutemov@...ux.intel.com,
        sathyanarayanan.kuppuswamy@...ux.intel.com, ak@...ux.intel.com,
        isaku.yamahata@...el.com, dan.j.williams@...el.com,
        jane.chu@...cle.com, seanjc@...gle.com, tony.luck@...el.com,
        x86@...nel.org, linux-kernel@...r.kernel.org,
        linux-hyperv@...r.kernel.org, netdev@...r.kernel.org,
        linux-pci@...r.kernel.org, linux-arch@...r.kernel.org,
        iommu@...ts.linux.dev
Subject: Re: [PATCH v2 01/12] x86/ioremap: Fix page aligned size calculation
 in __ioremap_caller()

On 11/10/22 22:21, Michael Kelley wrote:
> --- a/arch/x86/mm/ioremap.c
> +++ b/arch/x86/mm/ioremap.c
> @@ -218,7 +218,7 @@ static void __ioremap_check_mem(resource_size_t addr, unsigned long size,
>  	 */
>  	offset = phys_addr & ~PAGE_MASK;
>  	phys_addr &= PHYSICAL_PAGE_MASK;
> -	size = PAGE_ALIGN(last_addr+1) - phys_addr;
> +	size = (PAGE_ALIGN(last_addr+1) & PHYSICAL_PAGE_MASK) - phys_addr;

Michael, thanks for the explanation in your other reply.  First and
foremost, I *totally* missed the reason for this patch.  I was thinking
about issues that could pop up from the _lower_ bits being masked off.

Granted, your changelog _did_ say "upper bits", so shame on me.  But, it
would be great to put some more background in the changelog to make it a
bit harder for silly reviewers to miss such things.

I'd also like to propose something that I think is more straightforward:

        /*
         * Mappings have to be page-aligned
         */
        offset = phys_addr & ~PAGE_MASK;
        phys_addr &= PAGE_MASK;
        size = PAGE_ALIGN(last_addr+1) - phys_addr;

	/*
	 * Mask out any bits not parts of the actual physical
	 * address, like memory encryption bits.
	 */
	phys_addr &= PHYSICAL_PAGE_MASK;

Because, first of all, that "Mappings have to be page-aligned" thing is
(now) doing more than page-aligning things.  Second, the moment you mask
out the metadata bits, the 'size' calculation gets harder.  Doing it in
two phases (page alignment followed by metadata bit masking) breaks up
the two logical operations.


Powered by blists - more mailing lists