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 <[email protected]>
---
net/9p/trans_xen.c | 5 +++++
1 file changed, 5 insertions(+)
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);
--
2.25.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/[email protected]/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 <[email protected]>
| Link: https://lore.kernel.org/oe-kbuild-all/[email protected]/
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
--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests