[bug report] pvcalls-front: Avoid get_free_pages(GFP_KERNEL) under spinlock

[bug report] pvcalls-front: Avoid get_free_pages(GFP_KERNEL) under spinlock

[Dan Carpenter]

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@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

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

[wen.yang99]

[-- Attachment #1.1: Type: text/plain, Size: 2118 bytes --]

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@oracle.com>
To: wen yang10156314;
CC: xen-devel@lists.xenproject.org <xen-devel@lists.xenproject.org>
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

[-- Attachment #2: Type: text/plain, Size: 157 bytes --]

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