[PATCHv6 0/3] media: venus: close() fixes

Sergey Senozhatsky posted 3 patches 1 month ago
drivers/media/platform/qcom/venus/core.c      | 25 +++++++++++++++++++
drivers/media/platform/qcom/venus/core.h      |  2 ++
drivers/media/platform/qcom/venus/vdec.c      | 13 ++--------
drivers/media/platform/qcom/venus/vdec.h      |  1 -
.../media/platform/qcom/venus/vdec_ctrls.c    |  5 ----
drivers/media/platform/qcom/venus/venc.c      | 14 ++---------
drivers/media/platform/qcom/venus/venc.h      |  1 -
.../media/platform/qcom/venus/venc_ctrls.c    |  5 ----
8 files changed, 31 insertions(+), 35 deletions(-)
[PATCHv6 0/3] media: venus: close() fixes
Posted by Sergey Senozhatsky 1 month ago
A couple of fixes for venus driver close() handling
(both enc and dec).

v5->v6:
-- added kfree() backtrace to 0002

Sergey Senozhatsky (3):
  media: venus: fix enc/dec destruction order
  media: venus: sync with threaded IRQ during inst destruction
  media: venus: factor out inst destruction routine

 drivers/media/platform/qcom/venus/core.c      | 25 +++++++++++++++++++
 drivers/media/platform/qcom/venus/core.h      |  2 ++
 drivers/media/platform/qcom/venus/vdec.c      | 13 ++--------
 drivers/media/platform/qcom/venus/vdec.h      |  1 -
 .../media/platform/qcom/venus/vdec_ctrls.c    |  5 ----
 drivers/media/platform/qcom/venus/venc.c      | 14 ++---------
 drivers/media/platform/qcom/venus/venc.h      |  1 -
 .../media/platform/qcom/venus/venc_ctrls.c    |  5 ----
 8 files changed, 31 insertions(+), 35 deletions(-)

-- 
2.47.0.163.g1226f6d8fa-goog
Re: [PATCHv6 0/3] media: venus: close() fixes
Posted by Stanimir Varbanov 2 weeks, 6 days ago
Hi Sergey,

Thank you for the patch!

On 10/25/24 19:56, Sergey Senozhatsky wrote:
> A couple of fixes for venus driver close() handling
> (both enc and dec).
> 
> v5->v6:
> -- added kfree() backtrace to 0002
> 
> Sergey Senozhatsky (3):
>   media: venus: fix enc/dec destruction order
>   media: venus: sync with threaded IRQ during inst destruction
>   media: venus: factor out inst destruction routine

Could you please combine 1/3 and 2/3 commit bodies into 3/3 body and
resend the new 3/3 only. I do not see a reason to apply 1/3 and 2/3.

Also, on what platform this was tested?

~Stan

> 
>  drivers/media/platform/qcom/venus/core.c      | 25 +++++++++++++++++++
>  drivers/media/platform/qcom/venus/core.h      |  2 ++
>  drivers/media/platform/qcom/venus/vdec.c      | 13 ++--------
>  drivers/media/platform/qcom/venus/vdec.h      |  1 -
>  .../media/platform/qcom/venus/vdec_ctrls.c    |  5 ----
>  drivers/media/platform/qcom/venus/venc.c      | 14 ++---------
>  drivers/media/platform/qcom/venus/venc.h      |  1 -
>  .../media/platform/qcom/venus/venc_ctrls.c    |  5 ----
>  8 files changed, 31 insertions(+), 35 deletions(-)
>
Re: [PATCHv6 0/3] media: venus: close() fixes
Posted by Sergey Senozhatsky 2 weeks, 6 days ago
Hi Stanimir,

On (24/11/05 14:04), Stanimir Varbanov wrote:
> On 10/25/24 19:56, Sergey Senozhatsky wrote:
> > A couple of fixes for venus driver close() handling
> > (both enc and dec).
> > 
> > v5->v6:
> > -- added kfree() backtrace to 0002
> > 
> > Sergey Senozhatsky (3):
> >   media: venus: fix enc/dec destruction order
> >   media: venus: sync with threaded IRQ during inst destruction
> >   media: venus: factor out inst destruction routine
> 
> Could you please combine 1/3 and 2/3 commit bodies into 3/3 body and
> resend the new 3/3 only. I do not see a reason to apply 1/3 and 2/3.

So the reason being is that 1/3 fixes a race condition (stale data
in ->fh) and a lockdep splat (wrong destruction order).  2/3 fixes a
completely different race (IRQ vs close) condition and UAF.  And 3/3
is just a refactoring that doesn't fix anything.  Are you sure you
want to squash all 3 of them?  Because they look slightly independent
to me.

> Also, on what platform this was tested?

I ran CTS on one of the strongbad devices.
Re: [PATCHv6 0/3] media: venus: close() fixes
Posted by Sergey Senozhatsky 3 weeks, 3 days ago
On (24/10/26 01:56), Sergey Senozhatsky wrote:
> A couple of fixes for venus driver close() handling
> (both enc and dec).
> 
> v5->v6:
> -- added kfree() backtrace to 0002

linux-media folks, are you fine with the series?