drivers/i3c/master/svc-i3c-master.c | 1 + 1 file changed, 1 insertion(+)
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>
---
drivers/i3c/master/svc-i3c-master.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/drivers/i3c/master/svc-i3c-master.c b/drivers/i3c/master/svc-i3c-master.c
index 0a68fd1b81d4..e084ba648b4a 100644
--- a/drivers/i3c/master/svc-i3c-master.c
+++ b/drivers/i3c/master/svc-i3c-master.c
@@ -1775,6 +1775,7 @@ static void svc_i3c_master_remove(struct platform_device *pdev)
{
struct svc_i3c_master *master = platform_get_drvdata(pdev);
+ cancel_work_sync(&master->hj_work);
i3c_master_unregister(&master->base);
pm_runtime_dont_use_autosuspend(&pdev->dev);
--
2.25.1
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.
Frank
> drivers/i3c/master/svc-i3c-master.c | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/drivers/i3c/master/svc-i3c-master.c b/drivers/i3c/master/svc-i3c-master.c
> index 0a68fd1b81d4..e084ba648b4a 100644
> --- a/drivers/i3c/master/svc-i3c-master.c
> +++ b/drivers/i3c/master/svc-i3c-master.c
> @@ -1775,6 +1775,7 @@ static void svc_i3c_master_remove(struct platform_device *pdev)
> {
> struct svc_i3c_master *master = platform_get_drvdata(pdev);
>
> + cancel_work_sync(&master->hj_work);
> i3c_master_unregister(&master->base);
>
> pm_runtime_dont_use_autosuspend(&pdev->dev);
> --
> 2.25.1
>
在 2024-09-11 23:24:57，"Frank Li" <Frank.li@nxp.com> 写道：
>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.
>
>Frank
>
I will add them in the next version of the patch.
Best regards,
Kaixin Wang
>
>> drivers/i3c/master/svc-i3c-master.c | 1 +
>> 1 file changed, 1 insertion(+)
>>
>> diff --git a/drivers/i3c/master/svc-i3c-master.c b/drivers/i3c/master/svc-i3c-master.c
>> index 0a68fd1b81d4..e084ba648b4a 100644
>> --- a/drivers/i3c/master/svc-i3c-master.c
>> +++ b/drivers/i3c/master/svc-i3c-master.c
>> @@ -1775,6 +1775,7 @@ static void svc_i3c_master_remove(struct platform_device *pdev)
>> {
>> struct svc_i3c_master *master = platform_get_drvdata(pdev);
>>
>> + cancel_work_sync(&master->hj_work);
>> i3c_master_unregister(&master->base);
>>
>> pm_runtime_dont_use_autosuspend(&pdev->dev);
>> --
>> 2.25.1
>>
>
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
在 2024-09-12 14:57:09，"Miquel Raynal" <miquel.raynal@bootlin.com> 写道： >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 > Thanks for the review! Best regards, Kaixin Wang
© 2016 - 2024 Red Hat, Inc.