Re: [Xen-devel] [bug report] pvcalls-front: Avoid get_free_pages(GFP_KERNEL) underspinlock

[<wen.yang99@xxxxxxxxxx>]

Hi dan carpenter,
Thank you very much.
This patch will fix the potential null dereference:
diff --git a/drivers/xen/pvcalls-front.c b/drivers/xen/pvcalls-front.c
index 307861f..e56f9a3 100644
--- a/drivers/xen/pvcalls-front.c
+++ b/drivers/xen/pvcalls-front.c
@@ -344,7 +344,7 @@ int pvcalls_front_socket(struct socket *sock)
 static void free_active_ring(struct sock_mapping *map)
 {
        free_pages((unsigned long)map->active.data.in,
-                       map->active.ring->ring_order);
+                       PVCALLS_RING_ORDER);
        free_page((unsigned long)map->active.ring);
 }

We'll test it and send it soon.
Thanks.

Best Wishes,
      Wen


------------------Original Mail------------------
Sender: DanCarpenter <dan.carpenter@xxxxxxxxxx>
To: wen yang10156314;
CC: xen-devel@xxxxxxxxxxxxxxxxxxxx <xen-devel@xxxxxxxxxxxxxxxxxxxx>
Date: 2019/01/13 04:21
Subject: [bug report] pvcalls-front: Avoid get_free_pages(GFP_KERNEL) 
underspinlock
Hello Wen Yang,

The patch 9f51c05dc41a: "pvcalls-front: Avoid
get_free_pages(GFP_KERNEL) under spinlock" from Dec 5, 2018, leads to
the following static checker warning:

drivers/xen/pvcalls-front.c:373 alloc_active_ring()
error: we previously assumed 'map->active.ring' could be null (see line 357)

drivers/xen/pvcalls-front.c
351 static int alloc_active_ring(struct sock_mapping *map)
352 {
353     void *bytes;
354
355     map->active.ring = (struct pvcalls_data_intf *)
356         get_zeroed_page(GFP_KERNEL);
357     if (!map->active.ring)
^^^^^^^^^^^^^^^^^
Check

358         goto out;
359
360     map->active.ring->ring_order = PVCALLS_RING_ORDER;
361     bytes = (void *)__get_free_pages(GFP_KERNEL | __GFP_ZERO,
362                     PVCALLS_RING_ORDER);
363     if (!bytes)
364         goto out;
365
366     map->active.data.in = bytes;
367     map->active.data.out = bytes +
368         XEN_FLEX_RING_SIZE(PVCALLS_RING_ORDER);
369
370     return 0;
371
372 out:
--> 373     free_active_ring(map);
^^^
Unchecked dereference.  This style of error handling tends to have bugs.

https://plus.google.com/u/0/106378716002406849458/posts/1Ud9JbaYnPr

374     return -ENOMEM;
375 }

regards,
dan carpenter

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/xen-devel