Skip to content

Commit ec21a38

Browse files
committed
Revert "media: dvb-core: Fix use-after-free on race condition at dvb_frontend"
As reported by Thomas Voegtle <[email protected]>, sometimes a DVB card does not initialize properly booting Linux 6.4-rc4. This is not always, maybe in 3 out of 4 attempts. After double-checking, the root cause seems to be related to the UAF fix, which is causing a race issue: [ 26.332149] tda10071 7-0005: found a 'NXP TDA10071' in cold state, will try to load a firmware [ 26.340779] tda10071 7-0005: downloading firmware from file 'dvb-fe-tda10071.fw' [ 989.277402] INFO: task vdr:743 blocked for more than 491 seconds. [ 989.283504] Not tainted 6.4.0-rc5-i5 #249 [ 989.288036] "echo 0 > /proc/sys/kernel/hung_task_timeout_secs" disables this message. [ 989.295860] task:vdr state:D stack:0 pid:743 ppid:711 flags:0x00004002 [ 989.295865] Call Trace: [ 989.295867] <TASK> [ 989.295869] __schedule+0x2ea/0x12d0 [ 989.295877] ? asm_sysvec_apic_timer_interrupt+0x16/0x20 [ 989.295881] schedule+0x57/0xc0 [ 989.295884] schedule_preempt_disabled+0xc/0x20 [ 989.295887] __mutex_lock.isra.16+0x237/0x480 [ 989.295891] ? dvb_get_property.isra.10+0x1bc/0xa50 [ 989.295898] ? dvb_frontend_stop+0x36/0x180 [ 989.338777] dvb_frontend_stop+0x36/0x180 [ 989.338781] dvb_frontend_open+0x2f1/0x470 [ 989.338784] dvb_device_open+0x81/0xf0 [ 989.338804] ? exact_lock+0x20/0x20 [ 989.338808] chrdev_open+0x7f/0x1c0 [ 989.338811] ? generic_permission+0x1a2/0x230 [ 989.338813] ? link_path_walk.part.63+0x340/0x380 [ 989.338815] ? exact_lock+0x20/0x20 [ 989.338817] do_dentry_open+0x18e/0x450 [ 989.374030] path_openat+0xca5/0xe00 [ 989.374031] ? terminate_walk+0xec/0x100 [ 989.374034] ? path_lookupat+0x93/0x140 [ 989.374036] do_filp_open+0xc0/0x140 [ 989.374038] ? __call_rcu_common.constprop.91+0x92/0x240 [ 989.374041] ? __check_object_size+0x147/0x260 [ 989.374043] ? __check_object_size+0x147/0x260 [ 989.374045] ? alloc_fd+0xbb/0x180 [ 989.374048] ? do_sys_openat2+0x243/0x310 [ 989.374050] do_sys_openat2+0x243/0x310 [ 989.374052] do_sys_open+0x52/0x80 [ 989.374055] do_syscall_64+0x5b/0x80 [ 989.421335] ? __task_pid_nr_ns+0x92/0xa0 [ 989.421337] ? syscall_exit_to_user_mode+0x20/0x40 [ 989.421339] ? do_syscall_64+0x67/0x80 [ 989.421341] ? syscall_exit_to_user_mode+0x20/0x40 [ 989.421343] ? do_syscall_64+0x67/0x80 [ 989.421345] entry_SYSCALL_64_after_hwframe+0x63/0xcd [ 989.421348] RIP: 0033:0x7fe895d067e3 [ 989.421349] RSP: 002b:00007fff933c2ba0 EFLAGS: 00000293 ORIG_RAX: 0000000000000101 [ 989.421351] RAX: ffffffffffffffda RBX: 00007fff933c2c10 RCX: 00007fe895d067e3 [ 989.421352] RDX: 0000000000000802 RSI: 00005594acdce160 RDI: 00000000ffffff9c [ 989.421353] RBP: 0000000000000802 R08: 0000000000000000 R09: 0000000000000000 [ 989.421353] R10: 0000000000000000 R11: 0000000000000293 R12: 0000000000000001 [ 989.421354] R13: 00007fff933c2ca0 R14: 00000000ffffffff R15: 00007fff933c2c90 [ 989.421355] </TASK> This reverts commit 6769a0b. Fixes: 6769a0b ("media: dvb-core: Fix use-after-free on race condition at dvb_frontend") Link: https://lore.kernel.org/all/[email protected]/ Signed-off-by: Mauro Carvalho Chehab <[email protected]>
1 parent 4b0a501 commit ec21a38

File tree

2 files changed

+10
-49
lines changed

2 files changed

+10
-49
lines changed

drivers/media/dvb-core/dvb_frontend.c

Lines changed: 9 additions & 44 deletions
Original file line numberDiff line numberDiff line change
@@ -817,26 +817,15 @@ static void dvb_frontend_stop(struct dvb_frontend *fe)
817817

818818
dev_dbg(fe->dvb->device, "%s:\n", __func__);
819819

820-
mutex_lock(&fe->remove_mutex);
821-
822820
if (fe->exit != DVB_FE_DEVICE_REMOVED)
823821
fe->exit = DVB_FE_NORMAL_EXIT;
824822
mb();
825823

826-
if (!fepriv->thread) {
827-
mutex_unlock(&fe->remove_mutex);
824+
if (!fepriv->thread)
828825
return;
829-
}
830826

831827
kthread_stop(fepriv->thread);
832828

833-
mutex_unlock(&fe->remove_mutex);
834-
835-
if (fepriv->dvbdev->users < -1) {
836-
wait_event(fepriv->dvbdev->wait_queue,
837-
fepriv->dvbdev->users == -1);
838-
}
839-
840829
sema_init(&fepriv->sem, 1);
841830
fepriv->state = FESTATE_IDLE;
842831

@@ -2780,22 +2769,17 @@ static int dvb_frontend_open(struct inode *inode, struct file *file)
27802769
struct dvb_adapter *adapter = fe->dvb;
27812770
int ret;
27822771

2783-
mutex_lock(&fe->remove_mutex);
2784-
27852772
dev_dbg(fe->dvb->device, "%s:\n", __func__);
2786-
if (fe->exit == DVB_FE_DEVICE_REMOVED) {
2787-
ret = -ENODEV;
2788-
goto err_remove_mutex;
2789-
}
2773+
if (fe->exit == DVB_FE_DEVICE_REMOVED)
2774+
return -ENODEV;
27902775

27912776
if (adapter->mfe_shared == 2) {
27922777
mutex_lock(&adapter->mfe_lock);
27932778
if ((file->f_flags & O_ACCMODE) != O_RDONLY) {
27942779
if (adapter->mfe_dvbdev &&
27952780
!adapter->mfe_dvbdev->writers) {
27962781
mutex_unlock(&adapter->mfe_lock);
2797-
ret = -EBUSY;
2798-
goto err_remove_mutex;
2782+
return -EBUSY;
27992783
}
28002784
adapter->mfe_dvbdev = dvbdev;
28012785
}
@@ -2818,10 +2802,8 @@ static int dvb_frontend_open(struct inode *inode, struct file *file)
28182802
while (mferetry-- && (mfedev->users != -1 ||
28192803
mfepriv->thread)) {
28202804
if (msleep_interruptible(500)) {
2821-
if (signal_pending(current)) {
2822-
ret = -EINTR;
2823-
goto err_remove_mutex;
2824-
}
2805+
if (signal_pending(current))
2806+
return -EINTR;
28252807
}
28262808
}
28272809

@@ -2833,8 +2815,7 @@ static int dvb_frontend_open(struct inode *inode, struct file *file)
28332815
if (mfedev->users != -1 ||
28342816
mfepriv->thread) {
28352817
mutex_unlock(&adapter->mfe_lock);
2836-
ret = -EBUSY;
2837-
goto err_remove_mutex;
2818+
return -EBUSY;
28382819
}
28392820
adapter->mfe_dvbdev = dvbdev;
28402821
}
@@ -2893,8 +2874,6 @@ static int dvb_frontend_open(struct inode *inode, struct file *file)
28932874

28942875
if (adapter->mfe_shared)
28952876
mutex_unlock(&adapter->mfe_lock);
2896-
2897-
mutex_unlock(&fe->remove_mutex);
28982877
return ret;
28992878

29002879
err3:
@@ -2916,9 +2895,6 @@ static int dvb_frontend_open(struct inode *inode, struct file *file)
29162895
err0:
29172896
if (adapter->mfe_shared)
29182897
mutex_unlock(&adapter->mfe_lock);
2919-
2920-
err_remove_mutex:
2921-
mutex_unlock(&fe->remove_mutex);
29222898
return ret;
29232899
}
29242900

@@ -2929,8 +2905,6 @@ static int dvb_frontend_release(struct inode *inode, struct file *file)
29292905
struct dvb_frontend_private *fepriv = fe->frontend_priv;
29302906
int ret;
29312907

2932-
mutex_lock(&fe->remove_mutex);
2933-
29342908
dev_dbg(fe->dvb->device, "%s:\n", __func__);
29352909

29362910
if ((file->f_flags & O_ACCMODE) != O_RDONLY) {
@@ -2952,18 +2926,10 @@ static int dvb_frontend_release(struct inode *inode, struct file *file)
29522926
}
29532927
mutex_unlock(&fe->dvb->mdev_lock);
29542928
#endif
2929+
if (fe->exit != DVB_FE_NO_EXIT)
2930+
wake_up(&dvbdev->wait_queue);
29552931
if (fe->ops.ts_bus_ctrl)
29562932
fe->ops.ts_bus_ctrl(fe, 0);
2957-
2958-
if (fe->exit != DVB_FE_NO_EXIT) {
2959-
mutex_unlock(&fe->remove_mutex);
2960-
wake_up(&dvbdev->wait_queue);
2961-
} else {
2962-
mutex_unlock(&fe->remove_mutex);
2963-
}
2964-
2965-
} else {
2966-
mutex_unlock(&fe->remove_mutex);
29672933
}
29682934

29692935
dvb_frontend_put(fe);
@@ -3064,7 +3030,6 @@ int dvb_register_frontend(struct dvb_adapter *dvb,
30643030
fepriv = fe->frontend_priv;
30653031

30663032
kref_init(&fe->refcount);
3067-
mutex_init(&fe->remove_mutex);
30683033

30693034
/*
30703035
* After initialization, there need to be two references: one

include/media/dvb_frontend.h

Lines changed: 1 addition & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -686,10 +686,7 @@ struct dtv_frontend_properties {
686686
* @id: Frontend ID
687687
* @exit: Used to inform the DVB core that the frontend
688688
* thread should exit (usually, means that the hardware
689-
* got disconnected).
690-
* @remove_mutex: mutex that avoids a race condition between a callback
691-
* called when the hardware is disconnected and the
692-
* file_operations of dvb_frontend.
689+
* got disconnected.
693690
*/
694691

695692
struct dvb_frontend {
@@ -707,7 +704,6 @@ struct dvb_frontend {
707704
int (*callback)(void *adapter_priv, int component, int cmd, int arg);
708705
int id;
709706
unsigned int exit;
710-
struct mutex remove_mutex;
711707
};
712708

713709
/**

0 commit comments

Comments
 (0)