Messages in this thread | ![/](/images/icornerl.gif) | | Date | Mon, 13 Mar 2023 14:54:20 +0100 | From | Michal Swiatkowski <> | Subject | Re: [PATCH net v2] 9p/xen : Fix use after free bug in xen_9pfs_front_remove due to race condition |
| |
On Mon, Mar 13, 2023 at 05:00:02PM +0800, Zheng Wang wrote: > 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> > --- > v2: > - fix type error of ring found by kernel test robot > --- > 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..83764431c066 100644 > --- a/net/9p/trans_xen.c > +++ b/net/9p/trans_xen.c > @@ -274,12 +274,17 @@ 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; Move it before int i, j to have RCT.
> > write_lock(&xen_9pfs_lock); > list_del(&priv->list); > write_unlock(&xen_9pfs_lock); > > for (i = 0; i < priv->num_rings; i++) { > + /*cancel work*/ It isn't needed I think, the function cancel_work_sync() tells everything here.
> + ring = &priv->rings[i]; > + cancel_work_sync(&ring->work); > + > if (!priv->rings[i].intf) > break; > if (priv->rings[i].irq > 0) > -- > 2.25.1
| ![\](/images/icornerr.gif) |