Re: Bug in stv680

[Alan Cox <alan@redhat.com>]

On Wed, Jun 25, 2008 at 03:51:33PM -0400, David Ellingsworth wrote:
>      1. Application 1 opens the device. stv680->user transitions from 0->1
>      2. Application 2 opens the device. stv680->user transitions from 1->1
>      3. Application 2 closes the device. stv680->user transitions from 1->0
>      4. Device is physically disconnected. stv680 is freed.
>      5. Application 1 closes the device.
> 
> The crash could happen at step 4 in stv680_read, stv680_mmap, or
> stv680_do_ioctl, or at step 5 in stv_close depending on how
> Application 1 is using the device.

Yes I think you are right

> The apparent fix for this, is that stv680->user should be incremented
> whenever the device is opened, and decremented when it is closed.
> Likewise, a lock should be used to guarantee exclusive access to
> stv680->user to avoid a race condition between stv_open and stv_close.
> This should correct the case where the structure is freed by
> stv_disconnect while it is still open and in use.

Currently open/close use the big kernel lock so the locking part is ok. The
'proper' kernel way to do this is to use krefs which basically implement
the locking you describe but using atomic_inc/dec and atomic decrement and
compare with zero operations from the various architecture layers.


Something like

struct stv680 *stv_get(struct stv680 *v)
{
	kref_get(&v->kref);
}

void stv_put(struct stv680 *)
{
	kref_put(&v->kref, stv_free_thingies);
}


One camera driver that does use krefs properly (they are fairly new so
a lot of drivers don't yet adopt them) is the stk-webcam driver which uses
them for open/close as you describe.


--
video4linux-list mailing list
Unsubscribe mailto:video4linux-list-request@redhat.com?subject=unsubscribe
https://www.redhat.com/mailman/listinfo/video4linux-list