linux-kernel - Re: A potential race

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [thread-next>] [day] [month] [year] [list]

Date:	Fri, 1 Jul 2016 17:53:09 +0200
From:	Hans Verkuil <hverkuil@...all.nl>
To:	Pavel Andrianov <andrianov@...ras.ru>,
	Mauro Carvalho Chehab <mchehab@...nel.org>,
	Vladis Dronov <vdronov@...hat.com>,
	Insu Yun <wuninsu@...il.com>, Oliver Neukum <oneukum@...e.com>,
	linux-media@...r.kernel.org, linux-kernel@...r.kernel.org,
	Vaishali Thakkar <vaishali.thakkar@...cle.com>,
	ldv-project@...uxtesting.org
Subject: Re: A potential race

On 07/01/2016 04:39 PM, Pavel Andrianov wrote:
>  Hi!
> 
> There is a potential race condition between usbvision_v4l2_close
> <http://lxr.free-electrons.com/source/drivers/media/usb/usbvision/usbvision-video.c#L403> and usbvision_disconnect
> <http://lxr.free-electrons.com/source/drivers/media/usb/usbvision/usbvision-video.c#L1569>. The possible scenario may be the following.
> usbvision_disconnect <http://lxr.free-electrons.com/source/drivers/media/usb/usbvision/usbvision-video.c#L1569> starts execution, assigns
> usbvision->remove_pending = 1 <http://lxr.free-electrons.com/source/drivers/media/usb/usbvision/usbvision-video.c#L1587>, and is interrupted
> (rescheduled) after mutex_unlock <http://lxr.free-electrons.com/source/drivers/media/usb/usbvision/usbvision-video.c#L1592>. After that
> usbvision_v4l2_close <http://lxr.free-electrons.com/source/drivers/media/usb/usbvision/usbvision-video.c#L403> is executed, decrease
> usbvision->user-- <http://lxr.free-electrons.com/source/drivers/media/usb/usbvision/usbvision-video.c#L419>, checks
> usbvision->remove_pending <http://lxr.free-electrons.com/source/drivers/media/usb/usbvision/usbvision-video.c#L422>, executes
> usbvision_release <http://lxr.free-electrons.com/ident?i=usbvision_release> and finishes. Then usbvision_disconnect
> <http://lxr.free-electrons.com/source/drivers/media/usb/usbvision/usbvision-video.c#L1569> continues its execution. It checks
> usbversion->user <http://lxr.free-electrons.com/source/drivers/media/usb/usbvision/usbvision-video.c#L1594> (it is already 0) and also
> execute usbvision_release <http://lxr.free-electrons.com/ident?i=usbvision_release>. Thus, release is executed twice. The same situation may
> occur if usbvision_v4l2_close <http://lxr.free-electrons.com/source/drivers/media/usb/usbvision/usbvision-video.c#L403> is interrupted by
> usbvision_disconnect <http://lxr.free-electrons.com/source/drivers/media/usb/usbvision/usbvision-video.c#L1569>. Moreover, the same problem
> is in usbvision_radio_close <http://lxr.free-electrons.com/source/drivers/media/usb/usbvision/usbvision-video.c#L1135>. In all these cases
> the check before call usbvision_release <http://lxr.free-electrons.com/ident?i=usbvision_release> under mutex_lock protection does not solve
> the problem, because  there may occur an open() after the check and the race takes place again. The question is: why the usbvision_release
> <http://lxr.free-electrons.com/ident?i=usbvision_release> is called from close() (usbvision_v4l2_close
> <http://lxr.free-electrons.com/source/drivers/media/usb/usbvision/usbvision-video.c#L403> and usbvision_radio_close
> <http://lxr.free-electrons.com/source/drivers/media/usb/usbvision/usbvision-video.c#L1135>)? Usually release functions are called from
> disconnect.

Please don't use html mail, mailinglists will silently reject this.

The usbvision driver is old and unloved and known to be very bad code. It needs a huge amount of work to make all this work correctly.

I don't see anyone picking this up...

Regards,

	Hans