linux-kernel - Re: [PATCH 1/3] drivers/media/dvb-core/en50221: use kref to manage struct dvb_ca

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-prev] [thread-next>] [day] [month] [year] [list]

Date:	Thu, 16 Jun 2016 10:06:43 -0600
From:	Shuah Khan <shuahkh@....samsung.com>
To:	Max Kellermann <max@...mpel.org>, linux-media@...r.kernel.org,
	mchehab@....samsung.com
Cc:	linux-kernel@...r.kernel.org, Shuah Khan <shuahkh@....samsung.com>
Subject: Re: [PATCH 1/3] drivers/media/dvb-core/en50221: use kref to manage
 struct dvb_ca_private

On 06/15/2016 02:15 PM, Max Kellermann wrote:
> Don't free the object until the file handle has been closed.  Fixes
> use-after-free bug which occurs when I disconnect my DVB-S received
> while VDR is running.

Which file handle? /dev/dvb---

There seems to be a problem in the driver release routine:
dvb_ca_en50221_release() routine:

	kfree(ca->slot_info);
	dvb_unregister_device(ca->dvbdev);
	kfree(ca);

I think this should be since ioctl references slot info

	dvb_unregister_device(ca->dvbdev);
	kfree(ca->slot_info);
	kfree(ca);

I think dvb_ca_en50221_release() and dvb_ca_en50221_io_do_ioctl()
should serialize access to ca. dvb_ca_en50221_io_do_ioctl() holds
the ioctl_mutex, however, dvb_ca_en50221_release() could happen while
ioctl is in progress. Maybe you can try fixing those first.

As I mentioned in my review on your 3/3 patch, adding a kref here
adds more refcounted objects to the mix. You want to avoid that.

thanks,
-- Shuah