Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Date:   Sat, 18 Jun 2022 03:22:20 +0300
From:   "Kirill A. Shutemov" <kirill@shutemov.name>
To:     Yuntao Wang <ytcoode@gmail.com>
Cc:     Dave Hansen <dave.hansen@linux.intel.com>,
        Andy Lutomirski <luto@kernel.org>,
        Peter Zijlstra <peterz@infradead.org>,
        Thomas Gleixner <tglx@linutronix.de>,
        Ingo Molnar <mingo@redhat.com>, Borislav Petkov <bp@alien8.de>,
        "H. Peter Anvin" <hpa@zytor.com>, Baoquan He <bhe@redhat.com>,
        x86@kernel.org, linux-kernel@vger.kernel.org,
        stable@vger.kernel.org
Subject: Re: [PATCH] x86/mm: Fix possible index overflow when creating page
 table mapping
Message-ID: <20220618002220.panwk4zwnigayboy@box.shutemov.name>
References: <20220616135510.1784995-1-ytcoode@gmail.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <20220616135510.1784995-1-ytcoode@gmail.com>
Precedence: bulk

On Thu, Jun 16, 2022 at 09:55:10PM +0800, Yuntao Wang wrote:
> There are two issues in phys_p4d_init():
> 
> - The __kernel_physical_mapping_init() does not do boundary-checking for
>   paddr_end and passes it directly to phys_p4d_init(), phys_p4d_init() does
>   not do bounds checking either, so if the physical memory to be mapped is
>   large enough, 'p4d_page + p4d_index(vaddr)' will wrap around to the
>   beginning entry of the P4D table and its data will be overwritten.
> 
> - The for loop body will be executed only when 'vaddr < vaddr_end'
>   evaluates to true, but if that condition is true, 'paddr >= paddr_end'
>   will evaluate to false, thus the 'if (paddr >= paddr_end) {}' block will
>   never be executed and become dead code.
> 
> To fix these issues, use 'i < PTRS_PER_P4D' instead of 'vaddr < vaddr_end'
> as the for loop condition, this also make it more consistent with the logic
> of the phys_{pud,pmt,pte}_init() functions.

Hm. I don't see why you changed phys_p4d_init(), but not
__kernel_physical_mapping_init(). It does exactly the same thing, just
pgd_index() is hidden a bit deeper than p4d_index().

-- 
 Kirill A. Shutemov