[PATCH mm] mm: hugetlb: Add checks for NULL for vma returned from find_vma. find_vma may return NULL, that's why its return value is usually checked for NULL.

[PATCH mm] mm: hugetlb: Add checks for NULL for vma returned from find_vma. find_vma may return NULL, that's why its return value is usually checked for NULL.

[Esina Ekaterina]

Found by Astra Linux on behalf of Linux Verification Center
(linuxtesting.org) with SVACE.

Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2")
Signed-off-by: Esina Ekaterina <eesina@astralinux.ru>
---
 mm/hugetlb.c | 32 ++++++++++++++++++--------------
 1 file changed, 18 insertions(+), 14 deletions(-)

diff --git a/mm/hugetlb.c b/mm/hugetlb.c
index 3499b3803384..2162bfcf8f46 100644
--- a/mm/hugetlb.c
+++ b/mm/hugetlb.c
@@ -5394,9 +5394,6 @@ void adjust_range_if_pmd_sharing_possible(struct vm_area_struct *vma,
 pte_t *huge_pmd_share(struct mm_struct *mm, unsigned long addr, pud_t *pud)
 {
 	struct vm_area_struct *vma = find_vma(mm, addr);
-	struct address_space *mapping = vma->vm_file->f_mapping;
-	pgoff_t idx = ((addr - vma->vm_start) >> PAGE_SHIFT) +
-			vma->vm_pgoff;
 	struct vm_area_struct *svma;
 	unsigned long saddr;
 	pte_t *spte = NULL;
@@ -5406,18 +5403,25 @@ pte_t *huge_pmd_share(struct mm_struct *mm, unsigned long addr, pud_t *pud)
 	if (!vma_shareable(vma, addr))
 		return (pte_t *)pmd_alloc(mm, pud, addr);
 
-	i_mmap_assert_locked(mapping);
-	vma_interval_tree_foreach(svma, &mapping->i_mmap, idx, idx) {
-		if (svma == vma)
-			continue;
+	if (vma && vma->vm_file && vm->vm_file->f_mapping
+		&& vma->vm_start && vma->vm->vm_pgoff) {
+		struct address_space *mapping = vma->vm_file->f_mapping;
+		pgoff_t idx = ((addr - vma->vm_start) >> PAGE_SHIFT) +
+			vma->vm_pgoff;
 
-		saddr = page_table_shareable(svma, vma, addr, idx);
-		if (saddr) {
-			spte = huge_pte_offset(svma->vm_mm, saddr,
-					       vma_mmu_pagesize(svma));
-			if (spte) {
-				get_page(virt_to_page(spte));
-				break;
+		i_mmap_assert_locked(mapping);
+		vma_interval_tree_foreach(svma, &mapping->i_mmap, idx, idx) {
+			if (svma == vma)
+				continue;
+
+			saddr = page_table_shareable(svma, vma, addr, idx);
+			if (saddr) {
+				spte = huge_pte_offset(svma->vm_mm, saddr,
+							vma_mmu_pagesize(svma));
+				if (spte) {
+					get_page(virt_to_page(spte));
+					break;
+				}
 			}
 		}
 	}
-- 
2.39.0

Re: [PATCH mm] mm: hugetlb: Add checks for NULL for vma returned from find_vma. find_vma may return NULL, that's why its return value is usually checked for NULL.

[David Hildenbrand]

On 16.01.23 11:05, Esina Ekaterina wrote:
> Found by Astra Linux on behalf of Linux Verification Center
> (linuxtesting.org) with SVACE.

Please trim the subject and move details to the description.

What are the symptoms? How exactly does this BUG manifest? What is this 
patch fixing?


-- 
Thanks,

David / dhildenb

Re: [PATCH mm] mm: hugetlb: Add checks for NULL for vma returned from find_vma. find_vma may return NULL, that's why its return value is usually checked for NULL.

[David Hildenbrand]

On 16.01.23 16:10, Екатерина Есина wrote:
> Sorry, I've noticed that I'd sent the patch without description and sent 
> version 2.
> Function find_vma may return NULL, that's why its return value is 
> usually checked for NULL. In this case vma and its fields also should be 
> checked before dereferencing to avoid NULL pointer dereference

Thanks, but again

What are the symptoms? How exactly does this BUG manifest?

For example, does the kernel crash, and if so, how can it be triggered?

-- 
Thanks,

David / dhildenb

Re: [PATCH mm] mm: hugetlb: Add checks for NULL for vma returned from find_vma. find_vma may return NULL, that's why its return value is usually checked for NULL.

[Matthew Wilcox]

On Mon, Jan 16, 2023 at 04:12:48PM +0100, David Hildenbrand wrote:
> On 16.01.23 16:10, Екатерина Есина wrote:
> > Sorry, I've noticed that I'd sent the patch without description and sent
> > version 2.
> > Function find_vma may return NULL, that's why its return value is
> > usually checked for NULL. In this case vma and its fields also should be
> > checked before dereferencing to avoid NULL pointer dereference
> 
> Thanks, but again
> 
> What are the symptoms? How exactly does this BUG manifest?
> 
> For example, does the kernel crash, and if so, how can it be triggered?

SVACE is a static analyser.  It's not very good, but the people who run
it are extremely persistent about submitting patches based solely on the
output of the tool and doing no human checks of their own.  Probably best
to just ignore them, or NACK them.