diff mbox series

[net] 9p/xen : Fix use after free bug in xen_9pfs_front_remove due to race condition

Message ID 20230313041303.3158458-1-zyytlz.wz@163.com
State New
Headers
Series [net] 9p/xen : Fix use after free bug in xen_9pfs_front_remove due to race condition |

Commit Message

Zheng Wang March 13, 2023, 4:13 a.m. UTC 
  In xen_9pfs_front_probe, it calls xen_9pfs_front_alloc_dataring
to init priv->rings and bound &ring->work with p9_xen_response.

When it calls xen_9pfs_front_event_handler to handle IRQ requests,
it will finally call schedule_work to start the work.

When we call xen_9pfs_front_remove to remove the driver, there
may be a sequence as follows:

Fix it by finishing the work before cleanup in xen_9pfs_front_free.

Note that, this bug is found by static analysis, which might be
false positive.

CPU0                  CPU1

                     |p9_xen_response
xen_9pfs_front_remove|
  xen_9pfs_front_free|
kfree(priv)          |
//free priv          |
                     |p9_tag_lookup
                     |//use priv->client

Fixes: 71ebd71921e4 ("xen/9pfs: connect to the backend")
Signed-off-by: Zheng Wang <zyytlz.wz@163.com>
---
 net/9p/trans_xen.c | 5 +++++
 1 file changed, 5 insertions(+)

Comments

kernel test robot March 13, 2023, 7:21 a.m. UTC | #1 
Hi Zheng,

Thank you for the patch! Yet something to improve:

[auto build test ERROR on net/master]

url:    https://github.com/intel-lab-lkp/linux/commits/Zheng-Wang/9p-xen-Fix-use-after-free-bug-in-xen_9pfs_front_remove-due-to-race-condition/20230313-121534
patch link:    https://lore.kernel.org/r/20230313041303.3158458-1-zyytlz.wz%40163.com
patch subject: [PATCH net] 9p/xen : Fix use after free bug in xen_9pfs_front_remove due to race condition
config: arm-randconfig-r046-20230313 (https://download.01.org/0day-ci/archive/20230313/202303131508.drILo2xA-lkp@intel.com/config)
compiler: arm-linux-gnueabi-gcc (GCC) 12.1.0
reproduce (this is a W=1 build):
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # https://github.com/intel-lab-lkp/linux/commit/bf2159b54bb14c42221106b8681c471d005e7345
        git remote add linux-review https://github.com/intel-lab-lkp/linux
        git fetch --no-tags linux-review Zheng-Wang/9p-xen-Fix-use-after-free-bug-in-xen_9pfs_front_remove-due-to-race-condition/20230313-121534
        git checkout bf2159b54bb14c42221106b8681c471d005e7345
        # save the config file
        mkdir build_dir && cp config build_dir/.config
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-12.1.0 make.cross W=1 O=build_dir ARCH=arm olddefconfig
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-12.1.0 make.cross W=1 O=build_dir ARCH=arm SHELL=/bin/bash net/9p/

If you fix the issue, kindly add following tag where applicable
| Reported-by: kernel test robot <lkp@intel.com>
| Link: https://lore.kernel.org/oe-kbuild-all/202303131508.drILo2xA-lkp@intel.com/

All errors (new ones prefixed by >>):

   net/9p/trans_xen.c: In function 'xen_9pfs_front_free':
>> net/9p/trans_xen.c:284:24: error: incompatible types when assigning to type 'struct xen_9pfs_dataring *' from type 'struct xen_9pfs_dataring'
     284 |                 ring = priv->rings[i];
         |                        ^~~~


vim +284 net/9p/trans_xen.c

   273	
   274	static void xen_9pfs_front_free(struct xen_9pfs_front_priv *priv)
   275	{
   276		int i, j;
   277		struct xen_9pfs_dataring *ring = NULL;
   278	
   279		write_lock(&xen_9pfs_lock);
   280		list_del(&priv->list);
   281		write_unlock(&xen_9pfs_lock);
   282	
   283		for (i = 0; i < priv->num_rings; i++) {
 > 284			ring = priv->rings[i];
   285			if (!priv->rings[i].intf)
   286				break;
   287			if (priv->rings[i].irq > 0)
   288				unbind_from_irqhandler(priv->rings[i].irq, priv->dev);
   289	
   290			cancel_work_sync(&ring->work);
   291	
   292			if (priv->rings[i].data.in) {
   293				for (j = 0;
   294				     j < (1 << priv->rings[i].intf->ring_order);
   295				     j++) {
   296					grant_ref_t ref;
   297	
   298					ref = priv->rings[i].intf->ref[j];
   299					gnttab_end_foreign_access(ref, NULL);
   300				}
   301				free_pages_exact(priv->rings[i].data.in,
   302					   1UL << (priv->rings[i].intf->ring_order +
   303						   XEN_PAGE_SHIFT));
   304			}
   305			gnttab_end_foreign_access(priv->rings[i].ref, NULL);
   306			free_page((unsigned long)priv->rings[i].intf);
   307		}
   308		kfree(priv->rings);
   309		kfree(priv->tag);
   310		kfree(priv);
   311	}
   312
diff mbox series

Patch

diff --git a/net/9p/trans_xen.c b/net/9p/trans_xen.c
index c64050e839ac..60adb3aadd63 100644
--- a/net/9p/trans_xen.c
+++ b/net/9p/trans_xen.c
@@ -274,16 +274,21 @@  static const struct xenbus_device_id xen_9pfs_front_ids[] = {
 static void xen_9pfs_front_free(struct xen_9pfs_front_priv *priv)
 {
 	int i, j;
+	struct xen_9pfs_dataring *ring = NULL;
 
 	write_lock(&xen_9pfs_lock);
 	list_del(&priv->list);
 	write_unlock(&xen_9pfs_lock);
 
 	for (i = 0; i < priv->num_rings; i++) {
+		ring = priv->rings[i];
 		if (!priv->rings[i].intf)
 			break;
 		if (priv->rings[i].irq > 0)
 			unbind_from_irqhandler(priv->rings[i].irq, priv->dev);
+
+		cancel_work_sync(&ring->work);
+
 		if (priv->rings[i].data.in) {
 			for (j = 0;
 			     j < (1 << priv->rings[i].intf->ring_order);