regressions.lists.linux.dev archive mirror
 help / color / mirror / Atom feed
* Re: Sometimes DVB broken with commit 6769a0b7ee0c3b
       [not found] <da5382ad-09d6-20ac-0d53-611594b30861@lio96.de>
@ 2023-05-31 13:31 ` Linux regression tracking #adding (Thorsten Leemhuis)
  2023-06-05  9:38 ` Linux regression tracking (Thorsten Leemhuis)
  1 sibling, 0 replies; 7+ messages in thread
From: Linux regression tracking #adding (Thorsten Leemhuis) @ 2023-05-31 13:31 UTC (permalink / raw)
  To: Thomas Voegtle, linux-kernel, Hyunwoo Kim, Mauro Carvalho Chehab
  Cc: Linux kernel regressions list

[CCing the regression list, as it should be in the loop for regressions:
https://docs.kernel.org/admin-guide/reporting-regressions.html]

[TLDR: I'm adding this report to the list of tracked Linux kernel
regressions; the text you find below is based on a few templates
paragraphs you might have encountered already in similar form.
See link in footer if these mails annoy you.]

On 30.05.23 13:12, Thomas Voegtle wrote:
> 
> 
> I have the problem that sometimes my DVB card does not initialize
> properly booting Linux 6.4-rc4.
> This is not always, maybe in 3 out of 4 attempts.
> When this happens somehow you don't see anything special in dmesg, but
> the card just doesn't work.
> 
> Reverting this helps:
> commit 6769a0b7ee0c3b31e1b22c3fadff2bfb642de23f
> [...]

Thanks for the report. To be sure the issue doesn't fall through the
cracks unnoticed, I'm adding it to regzbot, the Linux kernel regression
tracking bot:

#regzbot ^introduced 6769a0b7ee0c3b31e1b22c3fadff2bfb642d
#regzbot title media: dvb-core: DVB card sometimes does not initialize
#regzbot ignore-activity

This isn't a regression? This issue or a fix for it are already
discussed somewhere else? It was fixed already? You want to clarify when
the regression started to happen? Or point out I got the title or
something else totally wrong? Then just reply and tell me -- ideally
while also telling regzbot about it, as explained by the page listed in
the footer of this mail.

Developers: When fixing the issue, remember to add 'Link:' tags pointing
to the report (the parent of this mail). See page linked in footer for
details.

Ciao, Thorsten (wearing his 'the Linux kernel's regression tracker' hat)
--
Everything you wanna know about Linux kernel regression tracking:
https://linux-regtracking.leemhuis.info/about/#tldr
That page also explains what to do if mails like this annoy you.

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

* Re: Sometimes DVB broken with commit 6769a0b7ee0c3b
       [not found] <da5382ad-09d6-20ac-0d53-611594b30861@lio96.de>
  2023-05-31 13:31 ` Sometimes DVB broken with commit 6769a0b7ee0c3b Linux regression tracking #adding (Thorsten Leemhuis)
@ 2023-06-05  9:38 ` Linux regression tracking (Thorsten Leemhuis)
  2023-06-05 10:37   ` Mauro Carvalho Chehab
  1 sibling, 1 reply; 7+ messages in thread
From: Linux regression tracking (Thorsten Leemhuis) @ 2023-06-05  9:38 UTC (permalink / raw)
  To: Hyunwoo Kim, Mauro Carvalho Chehab
  Cc: Linux Media Mailing List, Linux kernel regressions list,
	Thomas Voegtle, linux-kernel

Hi, Thorsten here, the Linux kernel's regression tracker.

On 30.05.23 13:12, Thomas Voegtle wrote:
> 
> I have the problem that sometimes my DVB card does not initialize
> properly booting Linux 6.4-rc4.
> This is not always, maybe in 3 out of 4 attempts.
> When this happens somehow you don't see anything special in dmesg, but
> the card just doesn't work.
> 
> Reverting this helps:
> commit 6769a0b7ee0c3b31e1b22c3fadff2bfb642de23f
> Author: Hyunwoo Kim <imv4bel@gmail.com>
> Date:   Thu Nov 17 04:59:22 2022 +0000
> 
>     media: dvb-core: Fix use-after-free on race condition at dvb_frontend
> 
> 
> I have:
> 03:00.0 Multimedia video controller [0400]: Conexant Systems, Inc.
> CX23887/8
> PCIe Broadcast Audio and Video Decoder with 3D Comb [14f1:8880] (rev 04)
>         Subsystem: Hauppauge computer works Inc. Device [0070:c138]
>         Kernel driver in use: cx23885

Hmmm, that was posted last Tuesday and received not a single reply. :-/

Hyunwoo Kim: could you please look at it, as it's a regression caused by
a commit of yours (one that would be good to solve before 6.4 is
finalized!)? And in case you are unable to do so let us know?

But FWIW:

Mauro: I wonder if this is something you or someone else has to look
into, as Hyunwoo Kim posted a few times per months to Linux lists, but
according  to a quick search on lore hasn't posted anything since ~two
months now. :-/

Ciao, Thorsten (wearing his 'the Linux kernel's regression tracker' hat)
--
Everything you wanna know about Linux kernel regression tracking:
https://linux-regtracking.leemhuis.info/about/#tldr
If I did something stupid, please tell me, as explained on that page.

#regzbot poke

> zcat /proc/config.gz | grep ^CONFIG_DVB
> CONFIG_DVB_CORE=y
> CONFIG_DVB_MMAP=y
> CONFIG_DVB_NET=y
> CONFIG_DVB_MAX_ADAPTERS=8
> CONFIG_DVB_DYNAMIC_MINORS=y
> CONFIG_DVB_USB=y
> CONFIG_DVB_USB_TTUSB2=y
> CONFIG_DVB_M88DS3103=y
> CONFIG_DVB_STB0899=y
> CONFIG_DVB_STB6100=y
> CONFIG_DVB_STV090x=y
> CONFIG_DVB_DRXK=y
> CONFIG_DVB_SI2165=y
> CONFIG_DVB_CX24116=y
> CONFIG_DVB_CX24117=y
> CONFIG_DVB_DS3000=y
> CONFIG_DVB_STV0299=y
> CONFIG_DVB_STV0900=y
> CONFIG_DVB_STV6110=y
> CONFIG_DVB_TDA10071=y
> CONFIG_DVB_TDA10086=y
> CONFIG_DVB_TDA8261=y
> CONFIG_DVB_TDA826X=y
> CONFIG_DVB_TS2020=y
> CONFIG_DVB_TUA6100=y
> CONFIG_DVB_DIB7000P=y
> CONFIG_DVB_SI2168=y
> CONFIG_DVB_STV0367=y
> CONFIG_DVB_TDA10048=y
> CONFIG_DVB_TDA1004X=y
> CONFIG_DVB_ZL10353=y
> CONFIG_DVB_TDA10021=y
> CONFIG_DVB_TDA10023=y
> CONFIG_DVB_LGDT330X=y
> CONFIG_DVB_S5H1409=y
> CONFIG_DVB_S5H1411=y
> CONFIG_DVB_MB86A20S=y
> CONFIG_DVB_PLL=y
> CONFIG_DVB_TUNER_DIB0070=y
> CONFIG_DVB_A8293=y
> CONFIG_DVB_LNBP21=y
> 
> 

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

* Re: Sometimes DVB broken with commit 6769a0b7ee0c3b
  2023-06-05  9:38 ` Linux regression tracking (Thorsten Leemhuis)
@ 2023-06-05 10:37   ` Mauro Carvalho Chehab
  2023-06-05 10:44     ` Thorsten Leemhuis
  0 siblings, 1 reply; 7+ messages in thread
From: Mauro Carvalho Chehab @ 2023-06-05 10:37 UTC (permalink / raw)
  To: Linux regression tracking (Thorsten Leemhuis)
  Cc: Linux regressions mailing list, Hyunwoo Kim,
	Linux Media Mailing List, Thomas Voegtle, linux-kernel

Em Mon, 5 Jun 2023 11:38:49 +0200
"Linux regression tracking (Thorsten Leemhuis)" <regressions@leemhuis.info> escreveu:

> Hi, Thorsten here, the Linux kernel's regression tracker.
> 
> On 30.05.23 13:12, Thomas Voegtle wrote:
> > 
> > I have the problem that sometimes my DVB card does not initialize
> > properly booting Linux 6.4-rc4.
> > This is not always, maybe in 3 out of 4 attempts.
> > When this happens somehow you don't see anything special in dmesg, but
> > the card just doesn't work.
> > 
> > Reverting this helps:
> > commit 6769a0b7ee0c3b31e1b22c3fadff2bfb642de23f
> > Author: Hyunwoo Kim <imv4bel@gmail.com>
> > Date:   Thu Nov 17 04:59:22 2022 +0000
> > 
> >     media: dvb-core: Fix use-after-free on race condition at dvb_frontend
> > 
> > 
> > I have:
> > 03:00.0 Multimedia video controller [0400]: Conexant Systems, Inc.
> > CX23887/8
> > PCIe Broadcast Audio and Video Decoder with 3D Comb [14f1:8880] (rev 04)
> >         Subsystem: Hauppauge computer works Inc. Device [0070:c138]
> >         Kernel driver in use: cx23885  
> 
> Hmmm, that was posted last Tuesday and received not a single reply. :-/
> 
> Hyunwoo Kim: could you please look at it, as it's a regression caused by
> a commit of yours (one that would be good to solve before 6.4 is
> finalized!)? And in case you are unable to do so let us know?
> 
> But FWIW:
> 
> Mauro: I wonder if this is something you or someone else has to look
> into, as Hyunwoo Kim posted a few times per months to Linux lists, but
> according  to a quick search on lore hasn't posted anything since ~two
> months now. :-/

Yeah, I was slow applying this one, as I was afraid of it to cause
troubles. The DVB frontend state machine is complex, and uses a
semaphore to update its state. There was some past attempts of
addressing some lifetime issues there that we ended needing to revert
or not being applied, as the fix caused more harm than good.

The way DVB tuning works is that it uses a zigzag mechanism to
tune to a frequency. It actually tries to tune at several different
frequencies, like:

	f
	f + delta
	f - delta
	f + 2 * delta
	f - 2 * delta
	...

the DVB core supports 3 different types of zigzag approaches:
	- hardware-based - no need to do any special implementation;
	- software-based - Kernel will try the above tunes in in a
	  zig-zag way;
	- custom-based - the hardware has some helpers to speedup
	  and improve the tuning logic.

If a signal is lost, the device will re-run the zigzag logic.

It is not trivial to test all possible conditions there, and some
boards may have multiple frontend devices for different types of
TV transmission (cable, satellite, air).

As I don't have a DVB signal generator anymore, my tests were limited to 
devices I have it handy that are compatible with DVB/T. 

In any case, we need more details about the problem, as CX23887 is just
the media streaming PCIe bridge device. It tells nothing about what
frontend is attached to the bridge.

Regards,
Mauro

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

* Re: Sometimes DVB broken with commit 6769a0b7ee0c3b
  2023-06-05 10:37   ` Mauro Carvalho Chehab
@ 2023-06-05 10:44     ` Thorsten Leemhuis
  2023-06-05 18:00       ` Mauro Carvalho Chehab
  0 siblings, 1 reply; 7+ messages in thread
From: Thorsten Leemhuis @ 2023-06-05 10:44 UTC (permalink / raw)
  To: Mauro Carvalho Chehab
  Cc: Linux regressions mailing list, Hyunwoo Kim,
	Linux Media Mailing List, Thomas Voegtle, linux-kernel

On 05.06.23 12:37, Mauro Carvalho Chehab wrote:
> Em Mon, 5 Jun 2023 11:38:49 +0200
> "Linux regression tracking (Thorsten Leemhuis)" <regressions@leemhuis.info> escreveu:
> 
>> Hi, Thorsten here, the Linux kernel's regression tracker.
>>
>> On 30.05.23 13:12, Thomas Voegtle wrote:
>>>
>>> I have the problem that sometimes my DVB card does not initialize
>>> properly booting Linux 6.4-rc4.
>>> This is not always, maybe in 3 out of 4 attempts.
>>> When this happens somehow you don't see anything special in dmesg, but
>>> the card just doesn't work.
>>>
>>> Reverting this helps:
>>> commit 6769a0b7ee0c3b31e1b22c3fadff2bfb642de23f
>>> Author: Hyunwoo Kim <imv4bel@gmail.com>
>>> Date:   Thu Nov 17 04:59:22 2022 +0000
>>>
>>>     media: dvb-core: Fix use-after-free on race condition at dvb_frontend
>>>
>>>
>>> I have:
>>> 03:00.0 Multimedia video controller [0400]: Conexant Systems, Inc.
>>> CX23887/8
>>> PCIe Broadcast Audio and Video Decoder with 3D Comb [14f1:8880] (rev 04)
>>>         Subsystem: Hauppauge computer works Inc. Device [0070:c138]
>>>         Kernel driver in use: cx23885  
>>
>> Hmmm, that was posted last Tuesday and received not a single reply. :-/
>>
>> Hyunwoo Kim: could you please look at it, as it's a regression caused by
>> a commit of yours (one that would be good to solve before 6.4 is
>> finalized!)? And in case you are unable to do so let us know?
>>
>> But FWIW:
>>
>> Mauro: I wonder if this is something you or someone else has to look
>> into, as Hyunwoo Kim posted a few times per months to Linux lists, but
>> according  to a quick search on lore hasn't posted anything since ~two
>> months now. :-/
> 
> Yeah, I was slow applying this one, as I was afraid of it to cause
> troubles. The DVB frontend state machine is complex, and uses a
> semaphore to update its state. There was some past attempts of
> addressing some lifetime issues there that we ended needing to revert
> or not being applied, as the fix caused more harm than good.
> [...]

Thx for the update. That's unfortunate, but how it is sometimes. Which
leads to a follow-up question: is reverting the culprit temporarily an
option? Or did those old use-after-free problems became known to be a
problem we can't live with anymore for another few months?

Ciao, Thorsten (wearing his 'the Linux kernel's regression tracker' hat)
--
Everything you wanna know about Linux kernel regression tracking:
https://linux-regtracking.leemhuis.info/about/#tldr
If I did something stupid, please tell me, as explained on that page.

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

* Re: Sometimes DVB broken with commit 6769a0b7ee0c3b
  2023-06-05 10:44     ` Thorsten Leemhuis
@ 2023-06-05 18:00       ` Mauro Carvalho Chehab
  2023-06-12 15:10         ` Linux regression tracking (Thorsten Leemhuis)
  0 siblings, 1 reply; 7+ messages in thread
From: Mauro Carvalho Chehab @ 2023-06-05 18:00 UTC (permalink / raw)
  To: Thorsten Leemhuis
  Cc: Linux regressions mailing list, Hyunwoo Kim,
	Linux Media Mailing List, Thomas Voegtle, linux-kernel

Em Mon, 5 Jun 2023 12:44:43 +0200
Thorsten Leemhuis <regressions@leemhuis.info> escreveu:

> On 05.06.23 12:37, Mauro Carvalho Chehab wrote:
> > Em Mon, 5 Jun 2023 11:38:49 +0200
> > "Linux regression tracking (Thorsten Leemhuis)" <regressions@leemhuis.info> escreveu:
> >   
> >> Hi, Thorsten here, the Linux kernel's regression tracker.
> >>
> >> On 30.05.23 13:12, Thomas Voegtle wrote:  
> >>>
> >>> I have the problem that sometimes my DVB card does not initialize
> >>> properly booting Linux 6.4-rc4.
> >>> This is not always, maybe in 3 out of 4 attempts.
> >>> When this happens somehow you don't see anything special in dmesg, but
> >>> the card just doesn't work.
> >>>
> >>> Reverting this helps:
> >>> commit 6769a0b7ee0c3b31e1b22c3fadff2bfb642de23f
> >>> Author: Hyunwoo Kim <imv4bel@gmail.com>
> >>> Date:   Thu Nov 17 04:59:22 2022 +0000
> >>>
> >>>     media: dvb-core: Fix use-after-free on race condition at dvb_frontend
> >>>
> >>>
> >>> I have:
> >>> 03:00.0 Multimedia video controller [0400]: Conexant Systems, Inc.
> >>> CX23887/8
> >>> PCIe Broadcast Audio and Video Decoder with 3D Comb [14f1:8880] (rev 04)
> >>>         Subsystem: Hauppauge computer works Inc. Device [0070:c138]
> >>>         Kernel driver in use: cx23885    
> >>
> >> Hmmm, that was posted last Tuesday and received not a single reply. :-/
> >>
> >> Hyunwoo Kim: could you please look at it, as it's a regression caused by
> >> a commit of yours (one that would be good to solve before 6.4 is
> >> finalized!)? And in case you are unable to do so let us know?
> >>
> >> But FWIW:
> >>
> >> Mauro: I wonder if this is something you or someone else has to look
> >> into, as Hyunwoo Kim posted a few times per months to Linux lists, but
> >> according  to a quick search on lore hasn't posted anything since ~two
> >> months now. :-/  
> > 
> > Yeah, I was slow applying this one, as I was afraid of it to cause
> > troubles. The DVB frontend state machine is complex, and uses a
> > semaphore to update its state. There was some past attempts of
> > addressing some lifetime issues there that we ended needing to revert
> > or not being applied, as the fix caused more harm than good.
> > [...]  
> 
> Thx for the update. That's unfortunate, but how it is sometimes. Which
> leads to a follow-up question: is reverting the culprit temporarily an
> option? Or did those old use-after-free problems became known to be a
> problem we can't live with anymore for another few months?

Reverting the patch seems to be the way to proceed. Then, work on another
way to address UAF. 

I'm not aware of dvb users complaining about troubles due to UAF, although 
it seems that there's now a CVE for it. So, maybe someone complained against
a distro Kernel, which caused the CVE to be opened.

So, while it is nice to have the lifetime issues fixed, last time I checked,
the USB dvb-usb/dvb-usb-v2 have some logic that usually prevents it to cause 
real issues during device removal, and unbinding DVB PCIe devices is 
something that users don't do in practice.

Regards,
Mauro

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

* Re: Sometimes DVB broken with commit 6769a0b7ee0c3b
  2023-06-05 18:00       ` Mauro Carvalho Chehab
@ 2023-06-12 15:10         ` Linux regression tracking (Thorsten Leemhuis)
  2023-06-14 22:20           ` Mauro Carvalho Chehab
  0 siblings, 1 reply; 7+ messages in thread
From: Linux regression tracking (Thorsten Leemhuis) @ 2023-06-12 15:10 UTC (permalink / raw)
  To: Mauro Carvalho Chehab
  Cc: Linux regressions mailing list, Hyunwoo Kim,
	Linux Media Mailing List, Thomas Voegtle, linux-kernel

On 05.06.23 20:00, Mauro Carvalho Chehab wrote:
> Em Mon, 5 Jun 2023 12:44:43 +0200
> Thorsten Leemhuis <regressions@leemhuis.info> escreveu:
>> On 05.06.23 12:37, Mauro Carvalho Chehab wrote:
>>> Em Mon, 5 Jun 2023 11:38:49 +0200
>>> "Linux regression tracking (Thorsten Leemhuis)" <regressions@leemhuis.info> escreveu:
>>>>
>>>> On 30.05.23 13:12, Thomas Voegtle wrote:  
>>>>>
>>>>> I have the problem that sometimes my DVB card does not initialize
>>>>> properly booting Linux 6.4-rc4.
>>>>> This is not always, maybe in 3 out of 4 attempts.
>>>>> When this happens somehow you don't see anything special in dmesg, but
>>>>> the card just doesn't work.
>>>>>
>>>>> Reverting this helps:
>>>>> commit 6769a0b7ee0c3b31e1b22c3fadff2bfb642de23f
>>>>[...]
>>>> Mauro: I wonder if this is something you or someone else has to look
>>>> into, as Hyunwoo Kim posted a few times per months to Linux lists, but
>>>> according  to a quick search on lore hasn't posted anything since ~two
>>>> months now. :-/  
>>>
>>> Yeah, I was slow applying this one, as I was afraid of it to cause
>>> troubles. The DVB frontend state machine is complex, and uses a
>>> semaphore to update its state. There was some past attempts of
>>> addressing some lifetime issues there that we ended needing to revert
>>> or not being applied, as the fix caused more harm than good.
>>> [...]  
>>
>> Thx for the update. That's unfortunate, but how it is sometimes. Which
>> leads to a follow-up question: is reverting the culprit temporarily an
>> option? Or did those old use-after-free problems became known to be a
>> problem we can't live with anymore for another few months?
> 
> Reverting the patch seems to be the way to proceed. Then, work on another
> way to address UAF. 
> 
> I'm not aware of dvb users complaining about troubles due to UAF, although 
> it seems that there's now a CVE for it. So, maybe someone complained against
> a distro Kernel, which caused the CVE to be opened.
> 
> So, while it is nice to have the lifetime issues fixed, last time I checked,
> the USB dvb-usb/dvb-usb-v2 have some logic that usually prevents it to cause 
> real issues during device removal, and unbinding DVB PCIe devices is 
> something that users don't do in practice.

Thx for the explanation and handling this. I noticed you posted a
revert, but it misses a fixes tag for the reverted commit and a Link: or
Closes: tag to the report. I think Linus would very much welcome at
least one of the latter in a situation like this (see [1] and [2]). I
would, too, as then regzbot would have noticed the patch posting. But
whatever, no big deal, let me tell regzbot about the latest progress
manually:

#regzbot monitor:
https://lore.kernel.org/all/20230609082238.3671398-1-mchehab@kernel.org/
#regzbot fix: Revert "media: dvb-core: Fix use-after-free on race
condition at dvb_frontend"

Ciao, Thorsten (wearing his 'the Linux kernel's regression tracker' hat)

[1] for details, see:
https://lore.kernel.org/all/CAHk-=wjMmSZzMJ3Xnskdg4+GGz=5p5p+GSYyFBTh0f-DgvdBWg@mail.gmail.com/
https://lore.kernel.org/all/CAHk-=wgs38ZrfPvy=nOwVkVzjpM3VFU1zobP37Fwd_h9iAD5JQ@mail.gmail.com/
https://lore.kernel.org/all/CAHk-=wjxzafG-=J8oT30s7upn4RhBs6TX-uVFZ5rME+L5_DoJA@mail.gmail.com/

[2] see Documentation/process/submitting-patches.rst
(http://docs.kernel.org/process/submitting-patches.html) and
Documentation/process/5.Posting.rst
(https://docs.kernel.org/process/5.Posting.html)

--
Everything you wanna know about Linux kernel regression tracking:
https://linux-regtracking.leemhuis.info/about/#tldr
If I did something stupid, please tell me, as explained on that page.

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

* Re: Sometimes DVB broken with commit 6769a0b7ee0c3b
  2023-06-12 15:10         ` Linux regression tracking (Thorsten Leemhuis)
@ 2023-06-14 22:20           ` Mauro Carvalho Chehab
  0 siblings, 0 replies; 7+ messages in thread
From: Mauro Carvalho Chehab @ 2023-06-14 22:20 UTC (permalink / raw)
  To: Linux regression tracking (Thorsten Leemhuis)
  Cc: Linux regressions mailing list, Hyunwoo Kim,
	Linux Media Mailing List, Thomas Voegtle, linux-kernel

Em Mon, 12 Jun 2023 17:10:06 +0200
"Linux regression tracking (Thorsten Leemhuis)" <regressions@leemhuis.info> escreveu:

> On 05.06.23 20:00, Mauro Carvalho Chehab wrote:
> > Em Mon, 5 Jun 2023 12:44:43 +0200
> > Thorsten Leemhuis <regressions@leemhuis.info> escreveu:  
> >> On 05.06.23 12:37, Mauro Carvalho Chehab wrote:  
> >>> Em Mon, 5 Jun 2023 11:38:49 +0200
> >>> "Linux regression tracking (Thorsten Leemhuis)" <regressions@leemhuis.info> escreveu:  
> >>>>
> >>>> On 30.05.23 13:12, Thomas Voegtle wrote:    
> >>>>>
> >>>>> I have the problem that sometimes my DVB card does not initialize
> >>>>> properly booting Linux 6.4-rc4.
> >>>>> This is not always, maybe in 3 out of 4 attempts.
> >>>>> When this happens somehow you don't see anything special in dmesg, but
> >>>>> the card just doesn't work.
> >>>>>
> >>>>> Reverting this helps:
> >>>>> commit 6769a0b7ee0c3b31e1b22c3fadff2bfb642de23f  
> >>>>[...]
> >>>> Mauro: I wonder if this is something you or someone else has to look
> >>>> into, as Hyunwoo Kim posted a few times per months to Linux lists, but
> >>>> according  to a quick search on lore hasn't posted anything since ~two
> >>>> months now. :-/    
> >>>
> >>> Yeah, I was slow applying this one, as I was afraid of it to cause
> >>> troubles. The DVB frontend state machine is complex, and uses a
> >>> semaphore to update its state. There was some past attempts of
> >>> addressing some lifetime issues there that we ended needing to revert
> >>> or not being applied, as the fix caused more harm than good.
> >>> [...]    
> >>
> >> Thx for the update. That's unfortunate, but how it is sometimes. Which
> >> leads to a follow-up question: is reverting the culprit temporarily an
> >> option? Or did those old use-after-free problems became known to be a
> >> problem we can't live with anymore for another few months?  
> > 
> > Reverting the patch seems to be the way to proceed. Then, work on another
> > way to address UAF. 
> > 
> > I'm not aware of dvb users complaining about troubles due to UAF, although 
> > it seems that there's now a CVE for it. So, maybe someone complained against
> > a distro Kernel, which caused the CVE to be opened.
> > 
> > So, while it is nice to have the lifetime issues fixed, last time I checked,
> > the USB dvb-usb/dvb-usb-v2 have some logic that usually prevents it to cause 
> > real issues during device removal, and unbinding DVB PCIe devices is 
> > something that users don't do in practice.  
> 
> Thx for the explanation and handling this. I noticed you posted a
> revert, but it misses a fixes tag for the reverted commit and a Link: or
> Closes: tag to the report. 

I added a Fixed and a link (see patch enclosed).

If everything merges fine on linux-next, I should be sending a PR
upstream likely tomorrow.

Regards,
Mauro

[PATCH] Revert "media: dvb-core: Fix use-after-free on race condition at dvb_frontend"

As reported by Thomas Voegtle <tv@lio96.de>, sometimes a DVB card does
not initialize properly booting Linux 6.4-rc4. This is not always, maybe
in 3 out of 4 attempts.

After double-checking, the root cause seems to be related to the
UAF fix, which is causing a race issue:

[   26.332149] tda10071 7-0005: found a 'NXP TDA10071' in cold state, will try to load a firmware
[   26.340779] tda10071 7-0005: downloading firmware from file 'dvb-fe-tda10071.fw'
[  989.277402] INFO: task vdr:743 blocked for more than 491 seconds.
[  989.283504]       Not tainted 6.4.0-rc5-i5 #249
[  989.288036] "echo 0 > /proc/sys/kernel/hung_task_timeout_secs" disables this message.
[  989.295860] task:vdr             state:D stack:0     pid:743   ppid:711    flags:0x00004002
[  989.295865] Call Trace:
[  989.295867]  <TASK>
[  989.295869]  __schedule+0x2ea/0x12d0
[  989.295877]  ? asm_sysvec_apic_timer_interrupt+0x16/0x20
[  989.295881]  schedule+0x57/0xc0
[  989.295884]  schedule_preempt_disabled+0xc/0x20
[  989.295887]  __mutex_lock.isra.16+0x237/0x480
[  989.295891]  ? dvb_get_property.isra.10+0x1bc/0xa50
[  989.295898]  ? dvb_frontend_stop+0x36/0x180
[  989.338777]  dvb_frontend_stop+0x36/0x180
[  989.338781]  dvb_frontend_open+0x2f1/0x470
[  989.338784]  dvb_device_open+0x81/0xf0
[  989.338804]  ? exact_lock+0x20/0x20
[  989.338808]  chrdev_open+0x7f/0x1c0
[  989.338811]  ? generic_permission+0x1a2/0x230
[  989.338813]  ? link_path_walk.part.63+0x340/0x380
[  989.338815]  ? exact_lock+0x20/0x20
[  989.338817]  do_dentry_open+0x18e/0x450
[  989.374030]  path_openat+0xca5/0xe00
[  989.374031]  ? terminate_walk+0xec/0x100
[  989.374034]  ? path_lookupat+0x93/0x140
[  989.374036]  do_filp_open+0xc0/0x140
[  989.374038]  ? __call_rcu_common.constprop.91+0x92/0x240
[  989.374041]  ? __check_object_size+0x147/0x260
[  989.374043]  ? __check_object_size+0x147/0x260
[  989.374045]  ? alloc_fd+0xbb/0x180
[  989.374048]  ? do_sys_openat2+0x243/0x310
[  989.374050]  do_sys_openat2+0x243/0x310
[  989.374052]  do_sys_open+0x52/0x80
[  989.374055]  do_syscall_64+0x5b/0x80
[  989.421335]  ? __task_pid_nr_ns+0x92/0xa0
[  989.421337]  ? syscall_exit_to_user_mode+0x20/0x40
[  989.421339]  ? do_syscall_64+0x67/0x80
[  989.421341]  ? syscall_exit_to_user_mode+0x20/0x40
[  989.421343]  ? do_syscall_64+0x67/0x80
[  989.421345]  entry_SYSCALL_64_after_hwframe+0x63/0xcd
[  989.421348] RIP: 0033:0x7fe895d067e3
[  989.421349] RSP: 002b:00007fff933c2ba0 EFLAGS: 00000293 ORIG_RAX: 0000000000000101
[  989.421351] RAX: ffffffffffffffda RBX: 00007fff933c2c10 RCX: 00007fe895d067e3
[  989.421352] RDX: 0000000000000802 RSI: 00005594acdce160 RDI: 00000000ffffff9c
[  989.421353] RBP: 0000000000000802 R08: 0000000000000000 R09: 0000000000000000
[  989.421353] R10: 0000000000000000 R11: 0000000000000293 R12: 0000000000000001
[  989.421354] R13: 00007fff933c2ca0 R14: 00000000ffffffff R15: 00007fff933c2c90
[  989.421355]  </TASK>

This reverts commit 6769a0b7ee0c3b31e1b22c3fadff2bfb642de23f.

Fixes: 6769a0b7ee0c ("media: dvb-core: Fix use-after-free on race condition at dvb_frontend")
Link: https://lore.kernel.org/all/da5382ad-09d6-20ac-0d53-611594b30861@lio96.de/
Signed-off-by: Mauro Carvalho Chehab <mchehab@kernel.org>

diff --git a/drivers/media/dvb-core/dvb_frontend.c b/drivers/media/dvb-core/dvb_frontend.c
index bc6950a5740f..9293b058ab99 100644
--- a/drivers/media/dvb-core/dvb_frontend.c
+++ b/drivers/media/dvb-core/dvb_frontend.c
@@ -817,26 +817,15 @@ static void dvb_frontend_stop(struct dvb_frontend *fe)
 
 	dev_dbg(fe->dvb->device, "%s:\n", __func__);
 
-	mutex_lock(&fe->remove_mutex);
-
 	if (fe->exit != DVB_FE_DEVICE_REMOVED)
 		fe->exit = DVB_FE_NORMAL_EXIT;
 	mb();
 
-	if (!fepriv->thread) {
-		mutex_unlock(&fe->remove_mutex);
+	if (!fepriv->thread)
 		return;
-	}
 
 	kthread_stop(fepriv->thread);
 
-	mutex_unlock(&fe->remove_mutex);
-
-	if (fepriv->dvbdev->users < -1) {
-		wait_event(fepriv->dvbdev->wait_queue,
-			   fepriv->dvbdev->users == -1);
-	}
-
 	sema_init(&fepriv->sem, 1);
 	fepriv->state = FESTATE_IDLE;
 
@@ -2780,13 +2769,9 @@ static int dvb_frontend_open(struct inode *inode, struct file *file)
 	struct dvb_adapter *adapter = fe->dvb;
 	int ret;
 
-	mutex_lock(&fe->remove_mutex);
-
 	dev_dbg(fe->dvb->device, "%s:\n", __func__);
-	if (fe->exit == DVB_FE_DEVICE_REMOVED) {
-		ret = -ENODEV;
-		goto err_remove_mutex;
-	}
+	if (fe->exit == DVB_FE_DEVICE_REMOVED)
+		return -ENODEV;
 
 	if (adapter->mfe_shared == 2) {
 		mutex_lock(&adapter->mfe_lock);
@@ -2794,8 +2779,7 @@ static int dvb_frontend_open(struct inode *inode, struct file *file)
 			if (adapter->mfe_dvbdev &&
 			    !adapter->mfe_dvbdev->writers) {
 				mutex_unlock(&adapter->mfe_lock);
-				ret = -EBUSY;
-				goto err_remove_mutex;
+				return -EBUSY;
 			}
 			adapter->mfe_dvbdev = dvbdev;
 		}
@@ -2818,10 +2802,8 @@ static int dvb_frontend_open(struct inode *inode, struct file *file)
 			while (mferetry-- && (mfedev->users != -1 ||
 					      mfepriv->thread)) {
 				if (msleep_interruptible(500)) {
-					if (signal_pending(current)) {
-						ret = -EINTR;
-						goto err_remove_mutex;
-					}
+					if (signal_pending(current))
+						return -EINTR;
 				}
 			}
 
@@ -2833,8 +2815,7 @@ static int dvb_frontend_open(struct inode *inode, struct file *file)
 				if (mfedev->users != -1 ||
 				    mfepriv->thread) {
 					mutex_unlock(&adapter->mfe_lock);
-					ret = -EBUSY;
-					goto err_remove_mutex;
+					return -EBUSY;
 				}
 				adapter->mfe_dvbdev = dvbdev;
 			}
@@ -2893,8 +2874,6 @@ static int dvb_frontend_open(struct inode *inode, struct file *file)
 
 	if (adapter->mfe_shared)
 		mutex_unlock(&adapter->mfe_lock);
-
-	mutex_unlock(&fe->remove_mutex);
 	return ret;
 
 err3:
@@ -2916,9 +2895,6 @@ static int dvb_frontend_open(struct inode *inode, struct file *file)
 err0:
 	if (adapter->mfe_shared)
 		mutex_unlock(&adapter->mfe_lock);
-
-err_remove_mutex:
-	mutex_unlock(&fe->remove_mutex);
 	return ret;
 }
 
@@ -2929,8 +2905,6 @@ static int dvb_frontend_release(struct inode *inode, struct file *file)
 	struct dvb_frontend_private *fepriv = fe->frontend_priv;
 	int ret;
 
-	mutex_lock(&fe->remove_mutex);
-
 	dev_dbg(fe->dvb->device, "%s:\n", __func__);
 
 	if ((file->f_flags & O_ACCMODE) != O_RDONLY) {
@@ -2952,18 +2926,10 @@ static int dvb_frontend_release(struct inode *inode, struct file *file)
 		}
 		mutex_unlock(&fe->dvb->mdev_lock);
 #endif
+		if (fe->exit != DVB_FE_NO_EXIT)
+			wake_up(&dvbdev->wait_queue);
 		if (fe->ops.ts_bus_ctrl)
 			fe->ops.ts_bus_ctrl(fe, 0);
-
-		if (fe->exit != DVB_FE_NO_EXIT) {
-			mutex_unlock(&fe->remove_mutex);
-			wake_up(&dvbdev->wait_queue);
-		} else {
-			mutex_unlock(&fe->remove_mutex);
-		}
-
-	} else {
-		mutex_unlock(&fe->remove_mutex);
 	}
 
 	dvb_frontend_put(fe);
@@ -3064,7 +3030,6 @@ int dvb_register_frontend(struct dvb_adapter *dvb,
 	fepriv = fe->frontend_priv;
 
 	kref_init(&fe->refcount);
-	mutex_init(&fe->remove_mutex);
 
 	/*
 	 * After initialization, there need to be two references: one
diff --git a/include/media/dvb_frontend.h b/include/media/dvb_frontend.h
index 367d5381217b..e7c44870f20d 100644
--- a/include/media/dvb_frontend.h
+++ b/include/media/dvb_frontend.h
@@ -686,10 +686,7 @@ struct dtv_frontend_properties {
  * @id:			Frontend ID
  * @exit:		Used to inform the DVB core that the frontend
  *			thread should exit (usually, means that the hardware
- *			got disconnected).
- * @remove_mutex:	mutex that avoids a race condition between a callback
- *			called when the hardware is disconnected and the
- *			file_operations of dvb_frontend.
+ *			got disconnected.
  */
 
 struct dvb_frontend {
@@ -707,7 +704,6 @@ struct dvb_frontend {
 	int (*callback)(void *adapter_priv, int component, int cmd, int arg);
 	int id;
 	unsigned int exit;
-	struct mutex remove_mutex;
 };
 
 /**





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

end of thread, other threads:[~2023-06-14 22:20 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
     [not found] <da5382ad-09d6-20ac-0d53-611594b30861@lio96.de>
2023-05-31 13:31 ` Sometimes DVB broken with commit 6769a0b7ee0c3b Linux regression tracking #adding (Thorsten Leemhuis)
2023-06-05  9:38 ` Linux regression tracking (Thorsten Leemhuis)
2023-06-05 10:37   ` Mauro Carvalho Chehab
2023-06-05 10:44     ` Thorsten Leemhuis
2023-06-05 18:00       ` Mauro Carvalho Chehab
2023-06-12 15:10         ` Linux regression tracking (Thorsten Leemhuis)
2023-06-14 22:20           ` Mauro Carvalho Chehab

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).