Re: [PATCH] i3c: master: svc: Fix use after free vulnerability in svc_i3c_master Driver Due to Race Condition

From: Miquel Raynal <miquel.raynal@bootlin.com>
To: Frank Li <Frank.li@nxp.com>
Cc: Kaixin Wang <kxwang23@m.fudan.edu.cn>,
	21210240012@m.fudan.edu.cn, 21302010073@m.fudan.edu.cn,
	conor.culhane@silvaco.com, alexandre.belloni@bootlin.com,
	linux-i3c@lists.infradead.org, linux-kernel@vger.kernel.org
Subject: Re: [PATCH] i3c: master: svc: Fix use after free vulnerability in svc_i3c_master Driver Due to Race Condition
Date: Thu, 12 Sep 2024 08:57:09 +0200	[thread overview]
Message-ID: <20240912085709.6ec0a289@xps-13> (raw)
In-Reply-To: <ZuG2SbsHEU5BU9mX@lizhi-Precision-Tower-5810>

Hi,

Frank.li@nxp.com wrote on Wed, 11 Sep 2024 11:24:57 -0400:

> On Wed, Sep 11, 2024 at 11:01:35PM +0800, Kaixin Wang wrote:
> > In the svc_i3c_master_probe function, &master->hj_work is bound with
> > svc_i3c_master_hj_work, &master->ibi_work is bound with
> > svc_i3c_master_ibi_work. And svc_i3c_master_ibi_work  can start the
> > hj_work, svc_i3c_master_irq_handler can start the ibi_work.
> >
> > If we remove the module which will call svc_i3c_master_remove to
> > make cleanup, it will free master->base through i3c_master_unregister
> > while the work mentioned above will be used. The sequence of operations
> > that may lead to a UAF bug is as follows:
> >
> > CPU0                                         CPU1
> >
> >                                     | svc_i3c_master_hj_work
> > svc_i3c_master_remove               |
> > i3c_master_unregister(&master->base)|
> > device_unregister(&master->dev)     |
> > device_release                      |
> > //free master->base                 |
> >                                     | i3c_master_do_daa(&master->base)
> >                                     | //use master->base
> >
> > Fix it by ensuring that the work is canceled before proceeding with the
> > cleanup in svc_i3c_master_remove.
> >
> > Signed-off-by: Kaixin Wang <kxwang23@m.fudan.edu.cn>
> > ---  
> 
> Please add fixes tag and cc stable.

Yes indeed. Otherwise looks good to me once this fixed.

Reviewed-by: Miquel Raynal <miquel.raynal@bootlin.com>

Thanks,
Miquèl

-- 
linux-i3c mailing list
linux-i3c@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-i3c