* [PATCH 0/2] Fix use-after-free errors when unregistering the i2c_client for the dvb demod
@ 2017-08-02 16:45 Matthias Schwarzott
2017-08-02 16:45 ` [PATCH 1/2] cx23885: Fix use-after-free " Matthias Schwarzott
2017-08-02 16:46 ` [PATCH 2/2] cx231xx: fix " Matthias Schwarzott
0 siblings, 2 replies; 4+ messages in thread
From: Matthias Schwarzott @ 2017-08-02 16:45 UTC (permalink / raw)
To: linux-media; +Cc: mchehab, crope
Hi!
There seem to be a general error in a lot of dvb bride drivers about the order of i2c_unregister_device
and the calls to dvb_unregister_frontend and dvb_frontend_detach.
As soon as the i2c_client for a demod driver is unregistered the memory for the frontend is kfreed.
But the calls to dvb_unregister_frontend and dvb_frontend_detach access it later.
I fixed the error in cx23885 and cx231xx driver as I have access to the hardware.
Further drivers that might show the same bug (but I cannot test):
* em28xx
* ddbride
* saa7164
Regards
Matthias
^ permalink raw reply [flat|nested] 4+ messages in thread
* [PATCH 1/2] cx23885: Fix use-after-free when unregistering the i2c_client for the dvb demod
2017-08-02 16:45 [PATCH 0/2] Fix use-after-free errors when unregistering the i2c_client for the dvb demod Matthias Schwarzott
@ 2017-08-02 16:45 ` Matthias Schwarzott
2017-08-23 10:56 ` Matthias Schwarzott
2017-08-02 16:46 ` [PATCH 2/2] cx231xx: fix " Matthias Schwarzott
1 sibling, 1 reply; 4+ messages in thread
From: Matthias Schwarzott @ 2017-08-02 16:45 UTC (permalink / raw)
To: linux-media; +Cc: mchehab, crope, Matthias Schwarzott
Unregistering the i2c_client of the demod driver destroys the frontend
object.
Calling vb2_dvb_unregister_bus later accesses the frontend (and with the
refcount_t) conversion the refcount_t code complains:
kernel: ------------[ cut here ]------------
kernel: WARNING: CPU: 0 PID: 7883 at lib/refcount.c:128 refcount_sub_and_test+0x70/0x80
kernel: refcount_t: underflow; use-after-free.
kernel: Modules linked in: bluetooth si2165(O) a8293(O) tda10071(O) tea5767(O) tuner(O) cx23885(O-) tda18271(O) videobuf2_dvb(O) videobuf2_dma_sg(O) m88ds3103(O) tveeprom(O) cx2341x(O) v4l2_common(O) dvb_core(O) rc_core(O) videobuf2_memops(O) videobuf2_v4l2(O) ums_realtek videobuf2_core(O) uas videodev(O) media(O) rtl8192cu i2c_mux usb_storage rtl_usb rtl8192c_common rtlwifi snd_hda_codec_hdmi snd_hda_codec_realtek snd_hda_codec_generic snd_hda_intel snd_hda_codec snd_hwdep snd_hda_core x86_pkg_temp_thermal kvm_intel kvm irqbypass
kernel: CPU: 0 PID: 7883 Comm: rmmod Tainted: G W O 4.11.3-gentoo #3
kernel: Hardware name: MEDION E2050 2391/H81H3-EM2, BIOS H81EM2W08.308 08/25/2014
kernel: Call Trace:
kernel: dump_stack+0x4d/0x66
kernel: __warn+0xc6/0xe0
kernel: warn_slowpath_fmt+0x46/0x50
kernel: ? kobject_put+0x2f/0x60
kernel: refcount_sub_and_test+0x70/0x80
kernel: refcount_dec_and_test+0x11/0x20
kernel: dvb_unregister_frontend+0x42/0x60 [dvb_core]
kernel: vb2_dvb_dealloc_frontends+0x9e/0x100 [videobuf2_dvb]
kernel: vb2_dvb_unregister_bus+0xd/0x20 [videobuf2_dvb]
kernel: cx23885_dvb_unregister+0xc3/0x110 [cx23885]
kernel: cx23885_dev_unregister+0xea/0x150 [cx23885]
kernel: cx23885_finidev+0x4f/0x70 [cx23885]
kernel: pci_device_remove+0x34/0xb0
kernel: device_release_driver_internal+0x150/0x200
kernel: driver_detach+0x33/0x70
kernel: bus_remove_driver+0x47/0xa0
kernel: driver_unregister+0x27/0x50
kernel: pci_unregister_driver+0x34/0x90
kernel: cx23885_fini+0x10/0x12 [cx23885]
kernel: SyS_delete_module+0x166/0x220
kernel: ? exit_to_usermode_loop+0x7b/0x80
kernel: entry_SYSCALL_64_fastpath+0x17/0x98
kernel: RIP: 0033:0x7f5901680b07
kernel: RSP: 002b:00007ffdf6cdb028 EFLAGS: 00000206 ORIG_RAX: 00000000000000b0
kernel: RAX: ffffffffffffffda RBX: 0000000000000003 RCX: 00007f5901680b07
kernel: RDX: 000000000000000a RSI: 0000000000000800 RDI: 0000000001500258
kernel: RBP: 00000000015001f0 R08: 0000000000000000 R09: 1999999999999999
kernel: R10: 0000000000000884 R11: 0000000000000206 R12: 00007ffdf6cda010
kernel: R13: 0000000000000000 R14: 00000000015001f0 R15: 00000000014ff010
kernel: ---[ end trace c3a4659b89086061 ]---
Signed-off-by: Matthias Schwarzott <zzam@gentoo.org>
---
drivers/media/pci/cx23885/cx23885-dvb.c | 10 +++++-----
1 file changed, 5 insertions(+), 5 deletions(-)
diff --git a/drivers/media/pci/cx23885/cx23885-dvb.c b/drivers/media/pci/cx23885/cx23885-dvb.c
index 979b66627f60..e795ddeb7fe2 100644
--- a/drivers/media/pci/cx23885/cx23885-dvb.c
+++ b/drivers/media/pci/cx23885/cx23885-dvb.c
@@ -2637,6 +2637,11 @@ int cx23885_dvb_unregister(struct cx23885_tsport *port)
struct vb2_dvb_frontend *fe0;
struct i2c_client *client;
+ fe0 = vb2_dvb_get_frontend(&port->frontends, 1);
+
+ if (fe0 && fe0->dvb.frontend)
+ vb2_dvb_unregister_bus(&port->frontends);
+
/* remove I2C client for CI */
client = port->i2c_client_ci;
if (client) {
@@ -2665,11 +2670,6 @@ int cx23885_dvb_unregister(struct cx23885_tsport *port)
i2c_unregister_device(client);
}
- fe0 = vb2_dvb_get_frontend(&port->frontends, 1);
-
- if (fe0 && fe0->dvb.frontend)
- vb2_dvb_unregister_bus(&port->frontends);
-
switch (port->dev->board) {
case CX23885_BOARD_NETUP_DUAL_DVBS2_CI:
netup_ci_exit(port);
--
2.13.3
^ permalink raw reply related [flat|nested] 4+ messages in thread
* [PATCH 2/2] cx231xx: fix use-after-free when unregistering the i2c_client for the dvb demod
2017-08-02 16:45 [PATCH 0/2] Fix use-after-free errors when unregistering the i2c_client for the dvb demod Matthias Schwarzott
2017-08-02 16:45 ` [PATCH 1/2] cx23885: Fix use-after-free " Matthias Schwarzott
@ 2017-08-02 16:46 ` Matthias Schwarzott
1 sibling, 0 replies; 4+ messages in thread
From: Matthias Schwarzott @ 2017-08-02 16:46 UTC (permalink / raw)
To: linux-media; +Cc: mchehab, crope, Matthias Schwarzott
Calling i2c_unregister_device for a demod driver destroys the frontend object.
Later it is accessed by calling dvb_unregister_frontend and
dvb_frontend_detach.
In some cases this leads to a general protection fault with this
callstack:
dvb_unregister_frontend+0x25/0x50 [dvb_core]
dvb_fini+0xdb/0x160 [cx231xx_dvb]
cx231xx_unregister_extension+0x3d/0xb0 [cx231xx]
cx231xx_dvb_unregister+0x10/0x809 [cx231xx_dvb]
SyS_delete_module+0x18a/0x240
? exit_to_usermode_loop+0x7b/0x80
entry_SYSCALL_64_fastpath+0x17/0x98
Signed-off-by: Matthias Schwarzott <zzam@gentoo.org>
---
drivers/media/usb/cx231xx/cx231xx-dvb.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/drivers/media/usb/cx231xx/cx231xx-dvb.c b/drivers/media/usb/cx231xx/cx231xx-dvb.c
index ee3eeeb600f8..c18bb33e060e 100644
--- a/drivers/media/usb/cx231xx/cx231xx-dvb.c
+++ b/drivers/media/usb/cx231xx/cx231xx-dvb.c
@@ -585,6 +585,9 @@ static void unregister_dvb(struct cx231xx_dvb *dvb)
dvb->demux.dmx.remove_frontend(&dvb->demux.dmx, &dvb->fe_hw);
dvb_dmxdev_release(&dvb->dmxdev);
dvb_dmx_release(&dvb->demux);
+ dvb_unregister_frontend(dvb->frontend);
+ dvb_frontend_detach(dvb->frontend);
+ dvb_unregister_adapter(&dvb->adapter);
/* remove I2C tuner */
client = dvb->i2c_client_tuner;
if (client) {
@@ -597,9 +600,6 @@ static void unregister_dvb(struct cx231xx_dvb *dvb)
module_put(client->dev.driver->owner);
i2c_unregister_device(client);
}
- dvb_unregister_frontend(dvb->frontend);
- dvb_frontend_detach(dvb->frontend);
- dvb_unregister_adapter(&dvb->adapter);
}
static int dvb_init(struct cx231xx *dev)
--
2.13.3
^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PATCH 1/2] cx23885: Fix use-after-free when unregistering the i2c_client for the dvb demod
2017-08-02 16:45 ` [PATCH 1/2] cx23885: Fix use-after-free " Matthias Schwarzott
@ 2017-08-23 10:56 ` Matthias Schwarzott
0 siblings, 0 replies; 4+ messages in thread
From: Matthias Schwarzott @ 2017-08-23 10:56 UTC (permalink / raw)
To: linux-media; +Cc: mchehab, crope
Am 02.08.2017 um 18:45 schrieb Matthias Schwarzott:
> diff --git a/drivers/media/pci/cx23885/cx23885-dvb.c b/drivers/media/pci/cx23885/cx23885-dvb.c
> index 979b66627f60..e795ddeb7fe2 100644
> --- a/drivers/media/pci/cx23885/cx23885-dvb.c
> +++ b/drivers/media/pci/cx23885/cx23885-dvb.c
> @@ -2637,6 +2637,11 @@ int cx23885_dvb_unregister(struct cx23885_tsport *port)
> struct vb2_dvb_frontend *fe0;
> struct i2c_client *client;
>
> + fe0 = vb2_dvb_get_frontend(&port->frontends, 1);
> +
> + if (fe0 && fe0->dvb.frontend)
> + vb2_dvb_unregister_bus(&port->frontends);
> +
> /* remove I2C client for CI */
> client = port->i2c_client_ci;
> if (client) {
> @@ -2665,11 +2670,6 @@ int cx23885_dvb_unregister(struct cx23885_tsport *port)
> i2c_unregister_device(client);
> }
>
> - fe0 = vb2_dvb_get_frontend(&port->frontends, 1);
> -
> - if (fe0 && fe0->dvb.frontend)
> - vb2_dvb_unregister_bus(&port->frontends);
> -
The following code is after i2c_unregister_device.
> switch (port->dev->board) {
> case CX23885_BOARD_NETUP_DUAL_DVBS2_CI:
> netup_ci_exit(port);
>
I wonder if the code above should be moved to before the
i2c_unregister_device block.
Currently these NETUP board drivers do not use "new style/i2c_client
based" frontend drivers. But if in future this switch is extended one
could get in trouble.
Regards
Matthias
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2017-08-23 10:56 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-08-02 16:45 [PATCH 0/2] Fix use-after-free errors when unregistering the i2c_client for the dvb demod Matthias Schwarzott
2017-08-02 16:45 ` [PATCH 1/2] cx23885: Fix use-after-free " Matthias Schwarzott
2017-08-23 10:56 ` Matthias Schwarzott
2017-08-02 16:46 ` [PATCH 2/2] cx231xx: fix " Matthias Schwarzott
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).