Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
Pre-existing issues: - [Critical] Lockless page table walk in `atomic_pte_lookup` reads page table entries multiple times, causing TOCTOU vulnerabilities (UAF, Wild Read, Privilege Escalation). - [Critical] atomic_pte_lookup maps the base of an anonymous THP instead of the requested offset, causing silent data corruption. -- --- Patch [9]: [PATCH RFC 09/11] misc/sgi-gru: use ptep_get() for page-table reads --- commit 24fd6151a0088920cebe4cf33a4840e02a2f4e62 Author: Muhammad Usama Anjum <[email protected]> misc/sgi-gru: use ptep_get() for page-table reads A leaf PMD is being read through ptep_get() by treating the PMD address as PTE-sized table storage. ptep_get() now accepts hw_pte_t *, so update the cast accordingly. > diff --git a/drivers/misc/sgi-gru/grufault.c b/drivers/misc/sgi-gru/grufault.c > index 3557d78ee47a2..ff89d34ad2aa4 100644 > --- a/drivers/misc/sgi-gru/grufault.c > +++ b/drivers/misc/sgi-gru/grufault.c > @@ -228,10 +228,10 @@ static int atomic_pte_lookup(struct vm_area_struct > *vma, unsigned long vaddr, > goto err; > #ifdef CONFIG_X86_64 > if (unlikely(pmd_leaf(*pmdp))) > - pte = ptep_get((pte_t *)pmdp); > + pte = ptep_get((hw_pte_t *)pmdp); > else > #endif > - pte = *pte_offset_kernel(pmdp, vaddr); > + pte = ptep_get(pte_offset_kernel(pmdp, vaddr)); [Severity: Critical] This is a pre-existing issue, but does this lockless page table walk have time-of-check to time-of-use vulnerabilities? When atomic_pte_lookup() evaluates pmd_leaf(*pmdp) and then rereads *pmdp via ptep_get(), if a THP split occurs concurrently (which only takes the PMD lock and can run concurrently with mmap_read_lock()), the second read could see a PMD pointing to a PTE table. The function would then treat this non-leaf PMD as a leaf PTE, extracting the PFN of the PTE table itself. Could this cause the GRU hardware TLB to be programmed to grant user-space read/write access to its own kernel page tables? Similarly, if a concurrent MADV_DONTNEED (which runs under mmap_read_lock()) clears the PMD between pmd_none() and pte_offset_kernel(), could pte_offset_kernel() compute a direct-map address of PFN 0 (PAGE_OFFSET), resulting in a wild read of physical memory to use as a PTE? [Severity: Critical] This is another pre-existing issue, but does atomic_pte_lookup() map the base of an anonymous THP instead of the requested offset, potentially causing silent data corruption? Further down in this function, the code does: *paddr = pte_pfn(pte) << PAGE_SHIFT; #ifdef CONFIG_HUGETLB_PAGE *pageshift = is_vm_hugetlb_page(vma) ? HPAGE_SHIFT : PAGE_SHIFT; #else *pageshift = PAGE_SHIFT; #endif When processing a TLB miss for a virtual address mapped by an anonymous THP, pmd_leaf(*pmdp) is true. The function extracts the base PFN of the 2MB page from the PMD. However, because is_vm_hugetlb_page(vma) evaluates to false for anonymous THPs (it only checks for hugetlbfs VMAs), *pageshift is set to PAGE_SHIFT (4KB). Because the base PFN of the THP is extracted without adding the offset for vaddr, and a 4KB page size is used, does this cause reads and writes to any 4KB chunk within an anonymous THP to incorrectly access the first 4KB chunk of that THP? > > if (unlikely(!pte_present(pte) || > (write && (!pte_write(pte) || !pte_dirty(pte))))) -- Sashiko AI review ยท https://sashiko.dev/#/patchset/[email protected]?part=9
