On Tue, 26 Nov 2024 11:21:14 +0100,
Martin Pieuchot <mpi@grenadille.net> wrote:
> 
> On 25/11/24(Mon) 20:58, Kirill A. Korinsky wrote:
> > tech@,
> > 
> > uvideo supports two modes of webcam reading, bulk and non-bulk. In bulk
> > mode it wraps by locks usbd_transfer to prevent call usbd_close_pipe on
> > used pipe, and in case of non-bulk, no lock here.
> 
> What do you mean by locks?
> 
> > When close had happened, it may lead to crash if non-bulk decided to
> > re-setup usbd_transfer in parallel thread.
> 
> I don't understand what you say.  To me the issue seem to come from the
> fact that uvideo_vs_close() is called twice, possibly because the first
> one sleeps.
> 
> > Here is the thread https://marc.info/?t=173228582400007&r=1&w=2 where I
> > encountered this crash, and this patch fixes it.
> 
> The diff below prevent the crash by clearing pointers before going to
> sleep.  Which means the "second" close won't trigger a user-after-free.
> 
> I'm not sure the order of operations below is correct.  usbd_ref_wait()
> is possibly where the thread being killed is waiting.  You can confirm
> that by killing ffplay and looking on which channel it hangs.
> 
> Then comes the question, why does the bulk thread doesn't stop?  Is it
> stuck?  Where is it sleeping?
>

Seems my wording wasn't good. I mean that I use usbd_ref_wait as lock to
block the second call of close.

I also was wrong that it goes to non-bulk code path, and you're right. The
code is blocked at usbd_transfer inside uvideo_vs_start_bulk_thread.

When I add a very short timeout (1s), I can't crash system because anymore,
but for longer timeout (10s) I still can do it.

-- 
wbr, Kirill