Skip to content

Commit 1f862a6

Browse files
MaxKellermannmchehab
authored andcommitted
[media] dvb_frontend: move kref to struct dvb_frontend
This commit amends my old commit fe35637 ("[media] dvb_frontend: eliminate blocking wait in dvb_unregister_frontend()"), which added kref to struct dvb_frontend_private. It turned out that there are several use-after-free bugs left, which affect the struct dvb_frontend. Protecting it with kref also protects struct dvb_frontend_private, so we can simply move it. This is how the use-after-free looks like in KASAN: BUG: KASAN: use-after-free in string+0x60/0xb1 at addr ffff880033bd9fc0 Read of size 1 by task kworker/0:2/617 CPU: 0 PID: 617 Comm: kworker/0:2 Not tainted 4.8.0-rc1-hosting+ #60 Hardware name: Bochs Bochs, BIOS Bochs 01/01/2011 Workqueue: usb_hub_wq hub_event 0000000000000000 ffff880033757218 ffffffff81394e50 ffff880033bd9fd0 ffff880035c03b00 ffff880033757240 ffffffff811f271d ffff880033bd9fc0 1ffff1000677b3f8 ffffed000677b3f8 ffff8800337572b8 ffffffff811f2afe Call Trace: [...] [<ffffffff813a2d2f>] vsnprintf+0x39d/0x7e9 [<ffffffff813993f9>] add_uevent_var+0x10f/0x1dc [<ffffffff814fe5ca>] rc_dev_uevent+0x55/0x6f [<ffffffff814438f8>] dev_uevent+0x2e1/0x316 [<ffffffff81399744>] kobject_uevent_env+0x27e/0x701 [<ffffffff81399bd2>] kobject_uevent+0xb/0xd [<ffffffff81443445>] device_del+0x322/0x383 [<ffffffff81500c0c>] rc_unregister_device+0x98/0xc3 [<ffffffff81508fb4>] dvb_usb_remote_exit+0x7a/0x90 [<ffffffff81506157>] dvb_usb_exit+0x1d/0xe5 [<ffffffff81506e90>] dvb_usb_device_exit+0x69/0x7d [<ffffffff8150a181>] pctv452e_usb_disconnect+0x7b/0x80 [...] Object at ffff880033bd9fc0, in cache kmalloc-16 size: 16 Allocated: [...] Freed: PID = 617 [...] [<ffffffff811f034c>] kfree+0xd9/0x166 [<ffffffff814fe513>] ir_free_table+0x2f/0x51 [<ffffffff81500bc1>] rc_unregister_device+0x4d/0xc3 [<ffffffff81508fb4>] dvb_usb_remote_exit+0x7a/0x90 [<ffffffff81506157>] dvb_usb_exit+0x1d/0xe5 [<ffffffff81506e90>] dvb_usb_device_exit+0x69/0x7d [<ffffffff8150a181>] pctv452e_usb_disconnect+0x7b/0x80 Another one: BUG: KASAN: use-after-free in do_sys_poll+0x336/0x6b8 at addr ffff88003563fcc0 Read of size 8 by task tuner on fronte/1042 CPU: 1 PID: 1042 Comm: tuner on fronte Tainted: G B 4.8.0-rc1-hosting+ #60 Hardware name: Bochs Bochs, BIOS Bochs 01/01/2011 0000000000000000 ffff88003353f910 ffffffff81394e50 ffff88003563fd80 ffff880035c03200 ffff88003353f938 ffffffff811f271d ffff88003563fc80 1ffff10006ac7f98 ffffed0006ac7f98 ffff88003353f9b0 ffffffff811f2afe Call Trace: [...] [<ffffffff812289b3>] do_sys_poll+0x336/0x6b8 [...] [<ffffffff81228ed9>] SyS_poll+0xa9/0x194 [...] Object at ffff88003563fc80, in cache kmalloc-256 size: 256 Allocated: [...] Freed: PID = 617 [...] [<ffffffff811f034c>] kfree+0xd9/0x166 [<ffffffff814eb60d>] dvb_unregister_device+0xd6/0xe5 [<ffffffff814fa4ed>] dvb_unregister_frontend+0x4b/0x66 [<ffffffff8150810b>] dvb_usb_adapter_frontend_exit+0x69/0xac [<ffffffff8150617d>] dvb_usb_exit+0x43/0xe5 [<ffffffff81506e90>] dvb_usb_device_exit+0x69/0x7d [<ffffffff8150a181>] pctv452e_usb_disconnect+0x7b/0x80 Signed-off-by: Max Kellermann <[email protected]> Signed-off-by: Mauro Carvalho Chehab <[email protected]>
1 parent f686c14 commit 1f862a6

File tree

2 files changed

+31
-16
lines changed

2 files changed

+31
-16
lines changed

drivers/media/dvb-core/dvb_frontend.c

Lines changed: 30 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -104,8 +104,6 @@ MODULE_PARM_DESC(dvb_mfe_wait_time, "Wait up to <mfe_wait_time> seconds on open(
104104
static DEFINE_MUTEX(frontend_mutex);
105105

106106
struct dvb_frontend_private {
107-
struct kref refcount;
108-
109107
/* thread/frontend values */
110108
struct dvb_device *dvbdev;
111109
struct dvb_frontend_parameters parameters_out;
@@ -143,21 +141,30 @@ struct dvb_frontend_private {
143141
#endif
144142
};
145143

146-
static void dvb_frontend_private_free(struct kref *ref)
144+
static void dvb_frontend_invoke_release(struct dvb_frontend *fe,
145+
void (*release)(struct dvb_frontend *fe));
146+
147+
static void dvb_frontend_free(struct kref *ref)
147148
{
148-
struct dvb_frontend_private *fepriv =
149-
container_of(ref, struct dvb_frontend_private, refcount);
149+
struct dvb_frontend *fe =
150+
container_of(ref, struct dvb_frontend, refcount);
151+
struct dvb_frontend_private *fepriv = fe->frontend_priv;
152+
153+
dvb_free_device(fepriv->dvbdev);
154+
155+
dvb_frontend_invoke_release(fe, fe->ops.release);
156+
150157
kfree(fepriv);
151158
}
152159

153-
static void dvb_frontend_private_put(struct dvb_frontend_private *fepriv)
160+
static void dvb_frontend_put(struct dvb_frontend *fe)
154161
{
155-
kref_put(&fepriv->refcount, dvb_frontend_private_free);
162+
kref_put(&fe->refcount, dvb_frontend_free);
156163
}
157164

158-
static void dvb_frontend_private_get(struct dvb_frontend_private *fepriv)
165+
static void dvb_frontend_get(struct dvb_frontend *fe)
159166
{
160-
kref_get(&fepriv->refcount);
167+
kref_get(&fe->refcount);
161168
}
162169

163170
static void dvb_frontend_wakeup(struct dvb_frontend *fe);
@@ -2555,7 +2562,7 @@ static int dvb_frontend_open(struct inode *inode, struct file *file)
25552562
fepriv->events.eventr = fepriv->events.eventw = 0;
25562563
}
25572564

2558-
dvb_frontend_private_get(fepriv);
2565+
dvb_frontend_get(fe);
25592566

25602567
if (adapter->mfe_shared)
25612568
mutex_unlock (&adapter->mfe_lock);
@@ -2605,7 +2612,7 @@ static int dvb_frontend_release(struct inode *inode, struct file *file)
26052612
fe->ops.ts_bus_ctrl(fe, 0);
26062613
}
26072614

2608-
dvb_frontend_private_put(fepriv);
2615+
dvb_frontend_put(fe);
26092616

26102617
return ret;
26112618
}
@@ -2695,7 +2702,14 @@ int dvb_register_frontend(struct dvb_adapter* dvb,
26952702
}
26962703
fepriv = fe->frontend_priv;
26972704

2698-
kref_init(&fepriv->refcount);
2705+
kref_init(&fe->refcount);
2706+
2707+
/*
2708+
* After initialization, there need to be two references: one
2709+
* for dvb_unregister_frontend(), and another one for
2710+
* dvb_frontend_detach().
2711+
*/
2712+
dvb_frontend_get(fe);
26992713

27002714
sema_init(&fepriv->sem, 1);
27012715
init_waitqueue_head (&fepriv->wait_queue);
@@ -2730,12 +2744,12 @@ int dvb_unregister_frontend(struct dvb_frontend* fe)
27302744
dev_dbg(fe->dvb->device, "%s:\n", __func__);
27312745

27322746
mutex_lock(&frontend_mutex);
2733-
dvb_frontend_stop (fe);
2734-
dvb_unregister_device (fepriv->dvbdev);
2747+
dvb_frontend_stop(fe);
2748+
dvb_remove_device(fepriv->dvbdev);
27352749

27362750
/* fe is invalid now */
27372751
mutex_unlock(&frontend_mutex);
2738-
dvb_frontend_private_put(fepriv);
2752+
dvb_frontend_put(fe);
27392753
return 0;
27402754
}
27412755
EXPORT_SYMBOL(dvb_unregister_frontend);
@@ -2757,6 +2771,6 @@ void dvb_frontend_detach(struct dvb_frontend* fe)
27572771
dvb_frontend_invoke_release(fe, fe->ops.tuner_ops.release);
27582772
dvb_frontend_invoke_release(fe, fe->ops.analog_ops.release);
27592773
dvb_frontend_invoke_release(fe, fe->ops.detach);
2760-
dvb_frontend_invoke_release(fe, fe->ops.release);
2774+
dvb_frontend_put(fe);
27612775
}
27622776
EXPORT_SYMBOL(dvb_frontend_detach);

drivers/media/dvb-core/dvb_frontend.h

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -667,6 +667,7 @@ struct dtv_frontend_properties {
667667
*/
668668

669669
struct dvb_frontend {
670+
struct kref refcount;
670671
struct dvb_frontend_ops ops;
671672
struct dvb_adapter *dvb;
672673
void *demodulator_priv;

0 commit comments

Comments
 (0)