[bug report] x86/xen: open code alloc_vm_area in arch_gnttab_valloc

* [bug report] x86/xen: open code alloc_vm_area in arch_gnttab_valloc
@ 2020-09-24  8:50 Dan Carpenter
  2020-09-24 10:10 ` Christoph Hellwig
  0 siblings, 1 reply; 2+ messages in thread
From: Dan Carpenter @ 2020-09-24  8:50 UTC (permalink / raw)
  To: hch; +Cc: xen-devel, Andrew Morton, linux-mm

Hello Christoph Hellwig,

The patch a0e7ac6b4907: "x86/xen: open code alloc_vm_area in
arch_gnttab_valloc" from Sep 23, 2020, leads to the following static
checker warning:

	arch/x86/xen/grant-table.c:110 arch_gnttab_valloc()
	warn: did you mean to pass the address of 'area->ptes'

arch/x86/xen/grant-table.c
    93  static int gnttab_apply(pte_t *pte, unsigned long addr, void *data)
    94  {
    95          pte_t ***p = data;
    96  
    97          **p = pte;
    98          (*p)++;
    99          return 0;
   100  }
   101  
   102  static int arch_gnttab_valloc(struct gnttab_vm_area *area, unsigned nr_frames)
   103  {
   104          area->ptes = kmalloc_array(nr_frames, sizeof(*area->ptes), GFP_KERNEL);

area->ptes is allocated here.

   105          if (area->ptes == NULL)
   106                  return -ENOMEM;
   107          area->area = get_vm_area(PAGE_SIZE * nr_frames, VM_IOREMAP);
   108          if (!area->area)
   109                  goto out_free_ptes;
   110          if (apply_to_page_range(&init_mm, (unsigned long)area->area->addr,
   111                          PAGE_SIZE * nr_frames, gnttab_apply, &area->ptes))
                                                                     ^^^^^^^^^^^
This increments area->ptes.  In the original code, there was a stack
variable which was changed and the area->ptes pointer wasn't modified.

   112                  goto out_free_vm_area;
   113          return 0;
   114  out_free_vm_area:
   115          free_vm_area(area->area);
   116  out_free_ptes:
   117          kfree(area->ptes);
                ^^^^^^^^^^^^^^^^^
This frees a different pointer from what was allocated.

   118          return -ENOMEM;
   119  }

regards,
dan carpenter

^ permalink raw reply	[flat|nested] 2+ messages in thread