linux-fbdev.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* Question about patch "fbdev: smscufx: Fix use-after-free in ufx_ops_open()"
@ 2022-10-11 14:13 ChenXiaoSong
  2022-10-11 15:34 ` Hyunwoo Kim
  2022-10-20 20:05 ` Hyunwoo Kim
  0 siblings, 2 replies; 3+ messages in thread
From: ChenXiaoSong @ 2022-10-11 14:13 UTC (permalink / raw)
  To: Hyunwoo Kim
  Cc: steve.glendinning, deller, linux-fbdev, dri-devel, imv4bel,
	zhangxiaoxu (A), yangerkun

Hi Hyunwoo:

[patch "fbdev: smscufx: Fix use-after-free in 
ufx_ops_open()"](https://lore.kernel.org/all/20220925133243.GA383897@ubuntu/T/) 
fix [CVE-2022-41849](https://nvd.nist.gov/vuln/detail/CVE-2022-41849).

If the UAF scenarios is as follows, it seems that [fix path 
v3](https://lore.kernel.org/all/20220925133243.GA383897@ubuntu/T/) will 
not avoid race contidion of krefs:

```
           cpu0                       |        cpu1
-------------------------------------|---------------------------------------------------
  1. open()                           |
     ufx_ops_open()                   |
-------------------------------------|---------------------------------------------------
                                      | 2. ufx_usb_disconnect()
                                      |    dev->virtualized = true;
                                      |    atomic_set()
                                      |    usb_set_intfdata()
                                      |
                                      | 3. if (dev->fb_count == 0)
                                      |    schedule_delayed_work()
                                      |    kref_put()   <- kref count : 1
                                      |    kref_put()   <- kref count : 0
                                      |    ufx_free()
                                      |    kfree(dev);
-------------------------------------|---------------------------------------------------
  4. if (dev->virtualized) <==== UAF  |
```

Did I misunderstand the UAF scenarios ?

^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: Question about patch "fbdev: smscufx: Fix use-after-free in ufx_ops_open()"
  2022-10-11 14:13 Question about patch "fbdev: smscufx: Fix use-after-free in ufx_ops_open()" ChenXiaoSong
@ 2022-10-11 15:34 ` Hyunwoo Kim
  2022-10-20 20:05 ` Hyunwoo Kim
  1 sibling, 0 replies; 3+ messages in thread
From: Hyunwoo Kim @ 2022-10-11 15:34 UTC (permalink / raw)
  To: ChenXiaoSong
  Cc: steve.glendinning, deller, linux-fbdev, dri-devel,
	zhangxiaoxu (A), yangerkun, imv4bel

On Tue, Oct 11, 2022 at 10:13:02PM +0800, ChenXiaoSong wrote:
> Hi Hyunwoo:
> 
> [patch "fbdev: smscufx: Fix use-after-free in ufx_ops_open()"](https://lore.kernel.org/all/20220925133243.GA383897@ubuntu/T/)
> fix [CVE-2022-41849](https://nvd.nist.gov/vuln/detail/CVE-2022-41849).
> 
> If the UAF scenarios is as follows, it seems that [fix path
> v3](https://lore.kernel.org/all/20220925133243.GA383897@ubuntu/T/) will not
> avoid race contidion of krefs:
> 
> ```
>           cpu0                       |        cpu1
> -------------------------------------|---------------------------------------------------
>  1. open()                           |
>     ufx_ops_open()                   |
> -------------------------------------|---------------------------------------------------
>                                      | 2. ufx_usb_disconnect()
>                                      |    dev->virtualized = true;
>                                      |    atomic_set()
>                                      |    usb_set_intfdata()
>                                      |
>                                      | 3. if (dev->fb_count == 0)
>                                      |    schedule_delayed_work()
>                                      |    kref_put()   <- kref count : 1
>                                      |    kref_put()   <- kref count : 0
>                                      |    ufx_free()
>                                      |    kfree(dev);
> -------------------------------------|---------------------------------------------------
>  4. if (dev->virtualized) <==== UAF  |
> ```

You are right. This v3 fix patch may prevent the UAF scenario I first suggested,
but not the one you suggested.

Any good ideas for this?
Could it be solved by adding a global flag variable indicating disconnected?


Regards,
Hyunwoo Kim.

^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: Question about patch "fbdev: smscufx: Fix use-after-free in ufx_ops_open()"
  2022-10-11 14:13 Question about patch "fbdev: smscufx: Fix use-after-free in ufx_ops_open()" ChenXiaoSong
  2022-10-11 15:34 ` Hyunwoo Kim
@ 2022-10-20 20:05 ` Hyunwoo Kim
  1 sibling, 0 replies; 3+ messages in thread
From: Hyunwoo Kim @ 2022-10-20 20:05 UTC (permalink / raw)
  To: ChenXiaoSong
  Cc: steve.glendinning, deller, linux-fbdev, dri-devel,
	zhangxiaoxu (A), yangerkun, imv4bel

Dear,


An additional patch has been submitted for the disconnect->open race condition you reported:
https://lore.kernel.org/linux-fbdev/20221020200113.GA320044@ubuntu/T/#u

Thanks for reporting.


Regards,
Hyunwoo Kim.

^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2022-10-20 20:05 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2022-10-11 14:13 Question about patch "fbdev: smscufx: Fix use-after-free in ufx_ops_open()" ChenXiaoSong
2022-10-11 15:34 ` Hyunwoo Kim
2022-10-20 20:05 ` Hyunwoo Kim

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).