Hi,
I just hit an oops when unplugging my usb serial adapter. So naturally,
I tried it again, and found if I use minicom to send BREAK and then
quickly yank the cable, I can reliably cause this oops every single
time.
Originally I noticed the problem on 6.1.7, but tried again on 6.4-rc5
and the exact same problem occurs.
Let me know if you need any more info!
Thanks,
Nick
[ 41.834144] ftdi_sio 1-1:1.0: FTDI USB Serial Device converter detected
[ 41.834178] usb 1-1: Detected FT232R
[ 41.834307] usb 1-1: FTDI USB Serial Device converter now attached to ttyUSB0
[ 49.238361] usb 1-1: USB disconnect, device number 6
[ 49.238592] ftdi_sio ttyUSB0: error from flowcontrol urb
[ 49.238734] ftdi_sio ttyUSB0: FTDI USB Serial Device converter now
disconnected from ttyUSB0
[ 49.238760] ftdi_sio 1-1:1.0: device disconnected
[ 49.274543] BUG: kernel NULL pointer dereference, address: 000000000000000c
[ 49.274550] #PF: supervisor read access in kernel mode
[ 49.274553] #PF: error_code(0x0000) - not-present page
[ 49.274555] PGD 0 P4D 0
[ 49.274560] Oops: 0000 [#1] PREEMPT SMP
[ 49.274564] CPU: 3 PID: 3247 Comm: minicom Not tainted 6.4.0-rc5 #20
[ 49.274568] Hardware name: LENOVO 20CMCTO1WW/20CMCTO1WW, BIOS
N10ET42W (1.21 ) 02/26/2016
[ 49.274572] RIP: 0010:ftdi_break_ctl+0x14/0x7b [ftdi_sio]
[ 49.274584] Code: c0 e8 64 63 6a d3 ba 06 00 00 00 31 f6 48 89 df
5b e9 5c fd ff ff 55 85 f6 89 f5 53 48 8b 9f 58 02 00 00 48 8b 83 48
03 00 00 <44> 8b 40 0c 74 06 66 41 81 c8 00 40 48 8b 13 45 0f b7 c0 b9
40 00
[ 49.274588] RSP: 0018:ffffb016009b3e28 EFLAGS: 00010246
[ 49.274591] RAX: 0000000000000000 RBX: ffffa3f944575800 RCX: 0000000000000000
[ 49.274593] RDX: 0000000000000001 RSI: 0000000000000000 RDI: ffffa3f940d8c400
[ 49.274595] RBP: 0000000000000000 R08: ffffa3fa65d9b840 R09: 0000000000000001
[ 49.274597] R10: 0000000000000000 R11: 0000000000000000 R12: 00000000000000fa
[ 49.274599] R13: ffffa3f940d8c400 R14: 0000000000000000 R15: 0000000000000000
[ 49.274602] FS: 00007f0ffcedc740(0000) GS:ffffa3fa65d80000(0000)
knlGS:0000000000000000
[ 49.274605] CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
[ 49.274607] CR2: 000000000000000c CR3: 000000010c981004 CR4: 00000000003706e0
[ 49.274610] Call Trace:
[ 49.274613] <TASK>
[ 49.274614] ? __die_body+0x15/0x53
[ 49.274620] ? page_fault_oops+0x2e8/0x31c
[ 49.274625] ? psi_group_change+0x237/0x298
[ 49.274631] ? get_sd_balance_interval+0xf/0x39
[ 49.274637] ? newidle_balance+0x25f/0x2c6
[ 49.274641] ? exc_page_fault+0x14b/0x4b8
[ 49.274647] ? asm_exc_page_fault+0x22/0x30
[ 49.274653] ? ftdi_break_ctl+0x14/0x7b [ftdi_sio]
[ 49.274661] serial_break+0x1c/0x1f [usbserial]
[ 49.274672] send_break+0x7f/0xa6
[ 49.274678] tty_ioctl+0x46e/0x6b3
[ 49.274683] ? vfs_write+0x15d/0x188
[ 49.274688] vfs_ioctl+0x16/0x23
[ 49.274692] __do_sys_ioctl+0x52/0x74
[ 49.274698] do_syscall_64+0x7f/0x9f
[ 49.274702] entry_SYSCALL_64_after_hwframe+0x46/0xb0
[ 49.274708] RIP: 0033:0x7f0ffcfdec7b
[ 49.274711] Code: 00 48 89 44 24 18 31 c0 48 8d 44 24 60 c7 04 24
10 00 00 00 48 89 44 24 08 48 8d 44 24 20 48 89 44 24 10 b8 10 00 00
00 0f 05 <89> c2 3d 00 f0 ff ff 77 1c 48 8b 44 24 18 64 48 2b 04 25 28
00 00
[ 49.274713] RSP: 002b:00007ffc18ab3a00 EFLAGS: 00000246 ORIG_RAX:
0000000000000010
[ 49.274717] RAX: ffffffffffffffda RBX: 00005562830d74bc RCX: 00007f0ffcfdec7b
[ 49.274719] RDX: 0000000000000000 RSI: 0000000000005409 RDI: 0000000000000003
[ 49.274721] RBP: 0000556284ef1d60 R08: 00007f0ffd076440 R09: 0000000000000064
[ 49.274723] R10: 00007f0ffcee7450 R11: 0000000000000246 R12: 00005562830e9130
[ 49.274726] R13: 00007ffc18ab3ad0 R14: 00005562830d6ee8 R15: 0000000000000004
[ 49.274728] </TASK>
[ 49.274730] Modules linked in: ftdi_sio usbserial ccm nfs lockd
grace bridge stp llc xt_state iptable_filter iptable_mangle
xt_MASQUERADE xt_mark iptable_nat nf_nat nf_conntrack nf_defrag_ipv6
nf_defrag_ipv4 ip_tables x_tables snd_hda_codec_hdmi squashfs loop
nls_iso8859_1 nls_cp437 vfat fat ext4 crc16 mbcache jbd2 iwlmvm i915
mac80211 snd_ctl_led libarc4 snd_hda_codec_realtek
snd_hda_codec_generic i2c_algo_bit drm_buddy drm_display_helper
uvcvideo iwlwifi drivetemp videobuf2_vmalloc drm_kms_helper
snd_hda_intel coretemp videobuf2_memops uvc snd_intel_dspcfg
videobuf2_v4l2 syscopyarea x86_pkg_temp_thermal sysfillrect
snd_hda_codec thinkpad_acpi sysimgblt cec videodev nvram snd_hda_core
cfg80211 kvm_intel ttm ledtrig_audio snd_pcm videobuf2_common
platform_profile jc42 regmap_i2c kvm irqbypass mc rfkill hwmon thermal
drm intel_gtt ac snd_timer agpgart battery snd video soundcore wmi
evdev efivarfs rtsx_pci_sdmmc mmc_core sha512_ssse3 e1000e rtsx_pci
ptp mfd_core pps_core ipv6 sunrpc
[ 49.274823] CR2: 000000000000000c
[ 49.274825] ---[ end trace 0000000000000000 ]---
[ 49.274827] RIP: 0010:ftdi_break_ctl+0x14/0x7b [ftdi_sio]
[ 49.274835] Code: c0 e8 64 63 6a d3 ba 06 00 00 00 31 f6 48 89 df
5b e9 5c fd ff ff 55 85 f6 89 f5 53 48 8b 9f 58 02 00 00 48 8b 83 48
03 00 00 <44> 8b 40 0c 74 06 66 41 81 c8 00 40 48 8b 13 45 0f b7 c0 b9
40 00
[ 49.274838] RSP: 0018:ffffb016009b3e28 EFLAGS: 00010246
[ 49.274841] RAX: 0000000000000000 RBX: ffffa3f944575800 RCX: 0000000000000000
[ 49.274843] RDX: 0000000000000001 RSI: 0000000000000000 RDI: ffffa3f940d8c400
[ 49.274845] RBP: 0000000000000000 R08: ffffa3fa65d9b840 R09: 0000000000000001
[ 49.274847] R10: 0000000000000000 R11: 0000000000000000 R12: 00000000000000fa
[ 49.274849] R13: ffffa3f940d8c400 R14: 0000000000000000 R15: 0000000000000000
[ 49.274851] FS: 00007f0ffcedc740(0000) GS:ffffa3fa65d80000(0000)
knlGS:0000000000000000
[ 49.274853] CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
[ 49.274855] CR2: 000000000000000c CR3: 000000010c981004 CR4: 00000000003706e0
[ 49.274858] note: minicom[3247] exited with irqs disabled
On Thu, Jun 8, 2023 at 10:37 AM Nick Bowler <[email protected]> wrote:
>
> Hi,
>
> I just hit an oops when unplugging my usb serial adapter. So naturally,
> I tried it again, and found if I use minicom to send BREAK and then
> quickly yank the cable, I can reliably cause this oops every single
> time.
>
> Originally I noticed the problem on 6.1.7, but tried again on 6.4-rc5
> and the exact same problem occurs.
>
> Let me know if you need any more info!
>
> Thanks,
> Nick
>
> [ 41.834144] ftdi_sio 1-1:1.0: FTDI USB Serial Device converter detected
> [ 41.834178] usb 1-1: Detected FT232R
> [ 41.834307] usb 1-1: FTDI USB Serial Device converter now attached to ttyUSB0
> [ 49.238361] usb 1-1: USB disconnect, device number 6
> [ 49.238592] ftdi_sio ttyUSB0: error from flowcontrol urb
> [ 49.238734] ftdi_sio ttyUSB0: FTDI USB Serial Device converter now
> disconnected from ttyUSB0
> [ 49.238760] ftdi_sio 1-1:1.0: device disconnected
According to the log, if I understand correctly, this driver first
disconnects, and then ftdi_break_ctl (operations about sending BREAK)
executes.
Clearly, it could lead to an UAF since priv is freed in the ftdi_port_remove.
I am not sure why there is a NULL pointer dereference since I did not
observe any explicit usb_set_serial_port_data(port, NULL);
The root cause may be due to the race between ftdi_port_remove and
ftdi_break_ctl. The fix can be a flag indicating if the device is
still presented.
Please correct me if you find any problem.
Dongliang Mu
> [ 49.274543] BUG: kernel NULL pointer dereference, address: 000000000000000c
> [ 49.274550] #PF: supervisor read access in kernel mode
> [ 49.274553] #PF: error_code(0x0000) - not-present page
> [ 49.274555] PGD 0 P4D 0
> [ 49.274560] Oops: 0000 [#1] PREEMPT SMP
> [ 49.274564] CPU: 3 PID: 3247 Comm: minicom Not tainted 6.4.0-rc5 #20
> [ 49.274568] Hardware name: LENOVO 20CMCTO1WW/20CMCTO1WW, BIOS
> N10ET42W (1.21 ) 02/26/2016
> [ 49.274572] RIP: 0010:ftdi_break_ctl+0x14/0x7b [ftdi_sio]
> [ 49.274584] Code: c0 e8 64 63 6a d3 ba 06 00 00 00 31 f6 48 89 df
> 5b e9 5c fd ff ff 55 85 f6 89 f5 53 48 8b 9f 58 02 00 00 48 8b 83 48
> 03 00 00 <44> 8b 40 0c 74 06 66 41 81 c8 00 40 48 8b 13 45 0f b7 c0 b9
> 40 00
> [ 49.274588] RSP: 0018:ffffb016009b3e28 EFLAGS: 00010246
> [ 49.274591] RAX: 0000000000000000 RBX: ffffa3f944575800 RCX: 0000000000000000
> [ 49.274593] RDX: 0000000000000001 RSI: 0000000000000000 RDI: ffffa3f940d8c400
> [ 49.274595] RBP: 0000000000000000 R08: ffffa3fa65d9b840 R09: 0000000000000001
> [ 49.274597] R10: 0000000000000000 R11: 0000000000000000 R12: 00000000000000fa
> [ 49.274599] R13: ffffa3f940d8c400 R14: 0000000000000000 R15: 0000000000000000
> [ 49.274602] FS: 00007f0ffcedc740(0000) GS:ffffa3fa65d80000(0000)
> knlGS:0000000000000000
> [ 49.274605] CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
> [ 49.274607] CR2: 000000000000000c CR3: 000000010c981004 CR4: 00000000003706e0
> [ 49.274610] Call Trace:
> [ 49.274613] <TASK>
> [ 49.274614] ? __die_body+0x15/0x53
> [ 49.274620] ? page_fault_oops+0x2e8/0x31c
> [ 49.274625] ? psi_group_change+0x237/0x298
> [ 49.274631] ? get_sd_balance_interval+0xf/0x39
> [ 49.274637] ? newidle_balance+0x25f/0x2c6
> [ 49.274641] ? exc_page_fault+0x14b/0x4b8
> [ 49.274647] ? asm_exc_page_fault+0x22/0x30
> [ 49.274653] ? ftdi_break_ctl+0x14/0x7b [ftdi_sio]
> [ 49.274661] serial_break+0x1c/0x1f [usbserial]
> [ 49.274672] send_break+0x7f/0xa6
> [ 49.274678] tty_ioctl+0x46e/0x6b3
> [ 49.274683] ? vfs_write+0x15d/0x188
> [ 49.274688] vfs_ioctl+0x16/0x23
> [ 49.274692] __do_sys_ioctl+0x52/0x74
> [ 49.274698] do_syscall_64+0x7f/0x9f
> [ 49.274702] entry_SYSCALL_64_after_hwframe+0x46/0xb0
> [ 49.274708] RIP: 0033:0x7f0ffcfdec7b
> [ 49.274711] Code: 00 48 89 44 24 18 31 c0 48 8d 44 24 60 c7 04 24
> 10 00 00 00 48 89 44 24 08 48 8d 44 24 20 48 89 44 24 10 b8 10 00 00
> 00 0f 05 <89> c2 3d 00 f0 ff ff 77 1c 48 8b 44 24 18 64 48 2b 04 25 28
> 00 00
> [ 49.274713] RSP: 002b:00007ffc18ab3a00 EFLAGS: 00000246 ORIG_RAX:
> 0000000000000010
> [ 49.274717] RAX: ffffffffffffffda RBX: 00005562830d74bc RCX: 00007f0ffcfdec7b
> [ 49.274719] RDX: 0000000000000000 RSI: 0000000000005409 RDI: 0000000000000003
> [ 49.274721] RBP: 0000556284ef1d60 R08: 00007f0ffd076440 R09: 0000000000000064
> [ 49.274723] R10: 00007f0ffcee7450 R11: 0000000000000246 R12: 00005562830e9130
> [ 49.274726] R13: 00007ffc18ab3ad0 R14: 00005562830d6ee8 R15: 0000000000000004
> [ 49.274728] </TASK>
> [ 49.274730] Modules linked in: ftdi_sio usbserial ccm nfs lockd
> grace bridge stp llc xt_state iptable_filter iptable_mangle
> xt_MASQUERADE xt_mark iptable_nat nf_nat nf_conntrack nf_defrag_ipv6
> nf_defrag_ipv4 ip_tables x_tables snd_hda_codec_hdmi squashfs loop
> nls_iso8859_1 nls_cp437 vfat fat ext4 crc16 mbcache jbd2 iwlmvm i915
> mac80211 snd_ctl_led libarc4 snd_hda_codec_realtek
> snd_hda_codec_generic i2c_algo_bit drm_buddy drm_display_helper
> uvcvideo iwlwifi drivetemp videobuf2_vmalloc drm_kms_helper
> snd_hda_intel coretemp videobuf2_memops uvc snd_intel_dspcfg
> videobuf2_v4l2 syscopyarea x86_pkg_temp_thermal sysfillrect
> snd_hda_codec thinkpad_acpi sysimgblt cec videodev nvram snd_hda_core
> cfg80211 kvm_intel ttm ledtrig_audio snd_pcm videobuf2_common
> platform_profile jc42 regmap_i2c kvm irqbypass mc rfkill hwmon thermal
> drm intel_gtt ac snd_timer agpgart battery snd video soundcore wmi
> evdev efivarfs rtsx_pci_sdmmc mmc_core sha512_ssse3 e1000e rtsx_pci
> ptp mfd_core pps_core ipv6 sunrpc
> [ 49.274823] CR2: 000000000000000c
> [ 49.274825] ---[ end trace 0000000000000000 ]---
> [ 49.274827] RIP: 0010:ftdi_break_ctl+0x14/0x7b [ftdi_sio]
> [ 49.274835] Code: c0 e8 64 63 6a d3 ba 06 00 00 00 31 f6 48 89 df
> 5b e9 5c fd ff ff 55 85 f6 89 f5 53 48 8b 9f 58 02 00 00 48 8b 83 48
> 03 00 00 <44> 8b 40 0c 74 06 66 41 81 c8 00 40 48 8b 13 45 0f b7 c0 b9
> 40 00
> [ 49.274838] RSP: 0018:ffffb016009b3e28 EFLAGS: 00010246
> [ 49.274841] RAX: 0000000000000000 RBX: ffffa3f944575800 RCX: 0000000000000000
> [ 49.274843] RDX: 0000000000000001 RSI: 0000000000000000 RDI: ffffa3f940d8c400
> [ 49.274845] RBP: 0000000000000000 R08: ffffa3fa65d9b840 R09: 0000000000000001
> [ 49.274847] R10: 0000000000000000 R11: 0000000000000000 R12: 00000000000000fa
> [ 49.274849] R13: ffffa3f940d8c400 R14: 0000000000000000 R15: 0000000000000000
> [ 49.274851] FS: 00007f0ffcedc740(0000) GS:ffffa3fa65d80000(0000)
> knlGS:0000000000000000
> [ 49.274853] CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
> [ 49.274855] CR2: 000000000000000c CR3: 000000010c981004 CR4: 00000000003706e0
> [ 49.274858] note: minicom[3247] exited with irqs disabled
On Wed, Jun 07, 2023 at 10:20:31PM -0400, Nick Bowler wrote:
> I just hit an oops when unplugging my usb serial adapter. So naturally,
> I tried it again, and found if I use minicom to send BREAK and then
> quickly yank the cable, I can reliably cause this oops every single
> time.
Well, don't do that then. ;)
I ran into this a couple of years ago myself but ended up preempted
before I could finish the fix I was working on.
The problem is that the tty layer happily calls back into the driver to
disable the break state after the device is gone.
Something like the below fixes it. I'll revisit this in a couple of
weeks.
Johan
From 14d3b01c02760979ede01d064fb1700d48c3ee68 Mon Sep 17 00:00:00 2001
From: Johan Hovold <[email protected]>
Date: Mon, 18 Oct 2021 09:43:38 +0200
Subject: [PATCH] tty: fix break race
TCSBRK and TCSBRKP can race with hangup and end up calling into a tty
driver for a device that is already gone.
FIXME: expand
Signed-off-by: Johan Hovold <[email protected]>
---
drivers/tty/tty_io.c | 68 ++++++++++++++++++++++++++++++--------------
include/linux/tty.h | 1 +
2 files changed, 47 insertions(+), 22 deletions(-)
diff --git a/drivers/tty/tty_io.c b/drivers/tty/tty_io.c
index c84be40fb8df..85fe52135f4b 100644
--- a/drivers/tty/tty_io.c
+++ b/drivers/tty/tty_io.c
@@ -630,6 +630,8 @@ static void __tty_hangup(struct tty_struct *tty, int exit_session)
tty_ldisc_hangup(tty, cons_filp != NULL);
+ wake_up_interruptible(&tty->break_wait);
+
spin_lock_irq(&tty->ctrl.lock);
clear_bit(TTY_THROTTLED, &tty->flags);
clear_bit(TTY_DO_WRITE_WAKEUP, &tty->flags);
@@ -1757,10 +1759,10 @@ int tty_release(struct inode *inode, struct file *filp)
/*
* Sanity check: if tty->count is going to zero, there shouldn't be
- * any waiters on tty->read_wait or tty->write_wait. We test the
- * wait queues and kick everyone out _before_ actually starting to
- * close. This ensures that we won't block while releasing the tty
- * structure.
+ * any waiters on tty->read_wait, tty->write_wait or tty->break_wait.
+ * We test the wait queues and kick everyone out _before_ actually
+ * starting to close. This ensures that we won't block while
+ * releasing the tty structure.
*
* The test for the o_tty closing is necessary, since the master and
* slave sides may close in any order. If the slave side closes out
@@ -1780,6 +1782,10 @@ int tty_release(struct inode *inode, struct file *filp)
wake_up_poll(&tty->write_wait, EPOLLOUT);
do_sleep++;
}
+ if (waitqueue_active(&tty->break_wait)) {
+ wake_up(&tty->break_wait);
+ do_sleep++;
+ }
}
if (o_tty && o_tty->count <= 1) {
if (waitqueue_active(&o_tty->read_wait)) {
@@ -2461,6 +2467,7 @@ static int tiocgetd(struct tty_struct *tty, int __user *p)
* send_break - performed time break
* @tty: device to break on
* @duration: timeout in mS
+ * @file: file object
*
* Perform a timed break on hardware that lacks its own driver level timed
* break functionality.
@@ -2468,30 +2475,46 @@ static int tiocgetd(struct tty_struct *tty, int __user *p)
* Locking:
* @tty->atomic_write_lock serializes
*/
-static int send_break(struct tty_struct *tty, unsigned int duration)
+static int send_break(struct file *file, struct tty_struct *tty, unsigned int duration)
{
+ DEFINE_WAIT_FUNC(wait, woken_wake_function);
int retval;
if (tty->ops->break_ctl == NULL)
return 0;
if (tty->driver->flags & TTY_DRIVER_HARDWARE_BREAK)
- retval = tty->ops->break_ctl(tty, duration);
- else {
- /* Do the work ourselves */
- if (tty_write_lock(tty, 0) < 0)
- return -EINTR;
- retval = tty->ops->break_ctl(tty, -1);
- if (retval)
- goto out;
- if (!signal_pending(current))
- msleep_interruptible(duration);
- retval = tty->ops->break_ctl(tty, 0);
-out:
- tty_write_unlock(tty);
- if (signal_pending(current))
- retval = -EINTR;
+ return tty->ops->break_ctl(tty, duration);
+
+ if (tty_write_lock(tty, 0) < 0)
+ return -EINTR;
+
+ add_wait_queue(&tty->break_wait, &wait);
+
+ if (signal_pending(current)) {
+ retval = -EINTR;
+ goto out;
+ }
+
+ if (tty_hung_up_p(file)) {
+ retval = -EIO;
+ goto out;
}
+
+ retval = tty->ops->break_ctl(tty, -1);
+ if (retval)
+ goto out;
+
+ wait_woken(&wait, TASK_INTERRUPTIBLE, msecs_to_jiffies(duration) + 1);
+
+ retval = tty->ops->break_ctl(tty, 0);
+out:
+ remove_wait_queue(&tty->break_wait, &wait);
+ tty_write_unlock(tty);
+
+ if (signal_pending(current))
+ retval = -EINTR;
+
return retval;
}
@@ -2739,10 +2762,10 @@ long tty_ioctl(struct file *file, unsigned int cmd, unsigned long arg)
* This is used by the tcdrain() termios function.
*/
if (!arg)
- return send_break(tty, 250);
+ return send_break(file, tty, 250);
return 0;
case TCSBRKP: /* support for POSIX tcsendbreak() */
- return send_break(tty, arg ? arg*100 : 250);
+ return send_break(file, tty, arg ? arg * 100 : 250);
case TIOCMGET:
return tty_tiocmget(tty, p);
@@ -3105,6 +3128,7 @@ struct tty_struct *alloc_tty_struct(struct tty_driver *driver, int idx)
init_ldsem(&tty->ldisc_sem);
init_waitqueue_head(&tty->write_wait);
init_waitqueue_head(&tty->read_wait);
+ init_waitqueue_head(&tty->break_wait);
INIT_WORK(&tty->hangup_work, do_tty_hangup);
mutex_init(&tty->atomic_write_lock);
spin_lock_init(&tty->ctrl.lock);
diff --git a/include/linux/tty.h b/include/linux/tty.h
index e8d5d9997aca..49b0e3b82901 100644
--- a/include/linux/tty.h
+++ b/include/linux/tty.h
@@ -235,6 +235,7 @@ struct tty_struct {
struct fasync_struct *fasync;
wait_queue_head_t write_wait;
wait_queue_head_t read_wait;
+ wait_queue_head_t break_wait;
struct work_struct hangup_work;
void *disc_data;
void *driver_data;
--
2.39.3