Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

The return value of 'sscanf()' should be checked #2

Open
QiAnXinCodeSafe opened this issue Nov 26, 2019 · 0 comments
Open

The return value of 'sscanf()' should be checked #2

QiAnXinCodeSafe opened this issue Nov 26, 2019 · 0 comments

Comments

@QiAnXinCodeSafe
Copy link

sscanf(optarg, "0x%llx", &fact_trl);

sagimnl pushed a commit that referenced this issue Jul 16, 2020
When lockdep is enabled, plugging Thunderbolt dock on Dominik's laptop
triggers following splat:

  ======================================================
  WARNING: possible circular locking dependency detected
  5.3.0-rc6+ #1 Tainted: G                T
  ------------------------------------------------------
  pool-/usr/lib/b/1258 is trying to acquire lock:
  000000005ab0ad43 (pci_rescan_remove_lock){+.+.}, at: authorized_store+0xe8/0x210

  but task is already holding lock:
  00000000bfb796b5 (&tb->lock){+.+.}, at: authorized_store+0x7c/0x210

  which lock already depends on the new lock.

  the existing dependency chain (in reverse order) is:

  -> #1 (&tb->lock){+.+.}:
         __mutex_lock+0xac/0x9a0
         tb_domain_add+0x2d/0x130
         nhi_probe+0x1dd/0x330
         pci_device_probe+0xd2/0x150
         really_probe+0xee/0x280
         driver_probe_device+0x50/0xc0
         bus_for_each_drv+0x84/0xd0
         __device_attach+0xe4/0x150
         pci_bus_add_device+0x4e/0x70
         pci_bus_add_devices+0x2e/0x66
         pci_bus_add_devices+0x59/0x66
         pci_bus_add_devices+0x59/0x66
         enable_slot+0x344/0x450
         acpiphp_check_bridge.part.0+0x119/0x150
         acpiphp_hotplug_notify+0xaa/0x140
         acpi_device_hotplug+0xa2/0x3f0
         acpi_hotplug_work_fn+0x1a/0x30
         process_one_work+0x234/0x580
         worker_thread+0x50/0x3b0
         kthread+0x10a/0x140
         ret_from_fork+0x3a/0x50

  -> #0 (pci_rescan_remove_lock){+.+.}:
         __lock_acquire+0xe54/0x1ac0
         lock_acquire+0xb8/0x1b0
         __mutex_lock+0xac/0x9a0
         authorized_store+0xe8/0x210
         kernfs_fop_write+0x125/0x1b0
         vfs_write+0xc2/0x1d0
         ksys_write+0x6c/0xf0
         do_syscall_64+0x50/0x180
         entry_SYSCALL_64_after_hwframe+0x49/0xbe

  other info that might help us debug this:
   Possible unsafe locking scenario:
         CPU0                    CPU1
         ----                    ----
    lock(&tb->lock);
                                 lock(pci_rescan_remove_lock);
                                 lock(&tb->lock);
    lock(pci_rescan_remove_lock);

   *** DEADLOCK ***
  5 locks held by pool-/usr/lib/b/1258:
   #0: 000000003df1a1ad (&f->f_pos_lock){+.+.}, at: __fdget_pos+0x4d/0x60
   #1: 0000000095a40b02 (sb_writers#6){.+.+}, at: vfs_write+0x185/0x1d0
   #2: 0000000017a7d714 (&of->mutex){+.+.}, at: kernfs_fop_write+0xf2/0x1b0
   #3: 000000004f262981 (kn->count#208){.+.+}, at: kernfs_fop_write+0xfa/0x1b0
   #4: 00000000bfb796b5 (&tb->lock){+.+.}, at: authorized_store+0x7c/0x210

  stack backtrace:
  CPU: 0 PID: 1258 Comm: pool-/usr/lib/b Tainted: G                T 5.3.0-rc6+ #1

On an system using ACPI hotplug the host router gets hotplugged first and then
the firmware starts sending notifications about connected devices so the above
scenario should not happen in reality. However, after taking a second
look at commit a03e828 ("thunderbolt: Serialize PCIe tunnel
creation with PCI rescan") that introduced the locking, I don't think it
is actually correct. It may have cured the symptom but probably the real
root cause was somewhere closer to PCI stack and possibly is already
fixed with recent kernels. I also tried to reproduce the original issue
with the commit reverted but could not.

So to keep lockdep happy and the code bit less complex drop calls to
pci_lock_rescan_remove()/pci_unlock_rescan_remove() in
tb_switch_set_authorized() effectively reverting a03e828.

Link: https://lkml.org/lkml/2019/8/30/513
Fixes: a03e828 ("thunderbolt: Serialize PCIe tunnel creation with PCI rescan")
Reported-by: Dominik Brodowski <[email protected]>
Signed-off-by: Mika Westerberg <[email protected]>
sagimnl pushed a commit that referenced this issue Jul 16, 2020
Original version of g920_get_config() contained two kind of actions:

    1. Device specific communication to query/set some parameters
       which requires active communication channel with the device,
       or, put in other way, for the call to be sandwiched between
       hid_device_io_start() and hid_device_io_stop().

    2. Input subsystem specific FF controller initialization which, in
       order to access a valid 'struct hid_input' via
       'hid->inputs.next', requires claimed hidinput which means be
       executed after the call to hid_hw_start() with connect_mask
       containing HID_CONNECT_HIDINPUT.

Location of g920_get_config() can only fulfill requirements for #1 and
not #2, which might result in following backtrace:

[   88.312258] logitech-hidpp-device 0003:046D:C262.0005: HID++ 4.2 device connected.
[   88.320298] BUG: kernel NULL pointer dereference, address: 0000000000000018
[   88.320304] #PF: supervisor read access in kernel mode
[   88.320307] #PF: error_code(0x0000) - not-present page
[   88.320309] PGD 0 P4D 0
[   88.320315] Oops: 0000 [#1] SMP PTI
[   88.320320] CPU: 1 PID: 3080 Comm: systemd-udevd Not tainted 5.4.0-rc1+ #31
[   88.320322] Hardware name: Apple Inc. MacBookPro11,1/Mac-189A3D4F975D5FFC, BIOS 149.0.0.0.0 09/17/2018
[   88.320334] RIP: 0010:hidpp_probe+0x61f/0x948 [hid_logitech_hidpp]
[   88.320338] Code: 81 00 00 48 89 ef e8 f0 d6 ff ff 41 89 c6 85 c0 75 b5 0f b6 44 24 28 48 8b 5d 00 88 44 24 1e 89 44 24 0c 48 8b 83 18 1c 00 00 <48> 8b 48 18 48 8b 83 10 19 00 00 48 8b 40 40 48 89 0c 24 0f b7 80
[   88.320341] RSP: 0018:ffffb0a6824aba68 EFLAGS: 00010246
[   88.320345] RAX: 0000000000000000 RBX: ffff93a50756e000 RCX: 0000000000010408
[   88.320347] RDX: 0000000000000000 RSI: ffff93a51f0ad0a0 RDI: 000000000002d0a0
[   88.320350] RBP: ffff93a50416da28 R08: ffff93a50416da70 R09: ffff93a50416da70
[   88.320352] R10: 000000148ae9e60c R11: 00000000000f1525 R12: ffff93a50756e000
[   88.320354] R13: ffff93a50756f8d0 R14: 0000000000000000 R15: ffff93a50756fc38
[   88.320358] FS:  00007f8d8c1e0940(0000) GS:ffff93a51f080000(0000) knlGS:0000000000000000
[   88.320361] CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
[   88.320363] CR2: 0000000000000018 CR3: 00000003996d8003 CR4: 00000000001606e0
[   88.320366] Call Trace:
[   88.320377]  ? _cond_resched+0x15/0x30
[   88.320387]  ? create_pinctrl+0x2f/0x3c0
[   88.320393]  ? kernfs_link_sibling+0x94/0xe0
[   88.320398]  ? _cond_resched+0x15/0x30
[   88.320402]  ? kernfs_activate+0x5f/0x80
[   88.320406]  ? kernfs_add_one+0xe2/0x130
[   88.320411]  hid_device_probe+0x106/0x170
[   88.320419]  really_probe+0x147/0x3c0
[   88.320424]  driver_probe_device+0xb6/0x100
[   88.320428]  device_driver_attach+0x53/0x60
[   88.320433]  __driver_attach+0x8a/0x150
[   88.320437]  ? device_driver_attach+0x60/0x60
[   88.320440]  bus_for_each_dev+0x78/0xc0
[   88.320445]  bus_add_driver+0x14d/0x1f0
[   88.320450]  driver_register+0x6c/0xc0
[   88.320453]  ? 0xffffffffc0d67000
[   88.320457]  __hid_register_driver+0x4c/0x80
[   88.320464]  do_one_initcall+0x46/0x1f4
[   88.320469]  ? _cond_resched+0x15/0x30
[   88.320474]  ? kmem_cache_alloc_trace+0x162/0x220
[   88.320481]  ? do_init_module+0x23/0x230
[   88.320486]  do_init_module+0x5c/0x230
[   88.320491]  load_module+0x26e1/0x2990
[   88.320502]  ? ima_post_read_file+0xf0/0x100
[   88.320508]  ? __do_sys_finit_module+0xaa/0x110
[   88.320512]  __do_sys_finit_module+0xaa/0x110
[   88.320520]  do_syscall_64+0x5b/0x180
[   88.320525]  entry_SYSCALL_64_after_hwframe+0x44/0xa9
[   88.320528] RIP: 0033:0x7f8d8d1f01fd
[   88.320532] Code: 00 c3 66 2e 0f 1f 84 00 00 00 00 00 90 f3 0f 1e fa 48 89 f8 48 89 f7 48 89 d6 48 89 ca 4d 89 c2 4d 89 c8 4c 8b 4c 24 08 0f 05 <48> 3d 01 f0 ff ff 73 01 c3 48 8b 0d 5b 8c 0c 00 f7 d8 64 89 01 48
[   88.320535] RSP: 002b:00007ffefa3bb068 EFLAGS: 00000246 ORIG_RAX: 0000000000000139
[   88.320539] RAX: ffffffffffffffda RBX: 000055922040cb40 RCX: 00007f8d8d1f01fd
[   88.320541] RDX: 0000000000000000 RSI: 00007f8d8ce4984d RDI: 0000000000000006
[   88.320543] RBP: 0000000000020000 R08: 0000000000000000 R09: 0000000000000007
[   88.320545] R10: 0000000000000006 R11: 0000000000000246 R12: 00007f8d8ce4984d
[   88.320547] R13: 0000000000000000 R14: 000055922040efc0 R15: 000055922040cb40
[   88.320551] Modules linked in: hid_logitech_hidpp(+) fuse rfcomm ccm xt_CHECKSUM xt_MASQUERADE bridge stp llc nf_nat_tftp nf_conntrack_tftp nf_conntrack_netbios_ns nf_conntrack_broadcast xt_CT ip6t_rpfilter ip6t_REJECT nf_reject_ipv6 xt_conntrack ebtable_nat ip6table_nat ip6table_mangle ip6table_raw ip6table_security iptable_nat nf_nat tun iptable_mangle iptable_raw iptable_security nf_conntrack nf_defrag_ipv6 nf_defrag_ipv4 libcrc32c ip_set nfnetlink ebtable_filter ebtables ip6table_filter ip6_tables cmac bnep sunrpc dm_crypt nls_utf8 hfsplus intel_rapl_msr intel_rapl_common ath9k_htc ath9k_common x86_pkg_temp_thermal intel_powerclamp b43 ath9k_hw coretemp snd_hda_codec_hdmi cordic kvm_intel snd_hda_codec_cirrus mac80211 snd_hda_codec_generic ledtrig_audio kvm snd_hda_intel snd_intel_nhlt irqbypass snd_hda_codec btusb btrtl snd_hda_core ath btbcm ssb snd_hwdep btintel snd_seq crct10dif_pclmul iTCO_wdt snd_seq_device crc32_pclmul bluetooth mmc_core iTCO_vendor_support joydev cfg80211
[   88.320602]  applesmc ghash_clmulni_intel ecdh_generic snd_pcm input_polldev intel_cstate ecc intel_uncore thunderbolt snd_timer i2c_i801 libarc4 rfkill intel_rapl_perf lpc_ich mei_me pcspkr bcm5974 snd bcma mei soundcore acpi_als sbs kfifo_buf sbshc industrialio apple_bl i915 i2c_algo_bit drm_kms_helper drm uas crc32c_intel usb_storage video hid_apple
[   88.320630] CR2: 0000000000000018
[   88.320633] ---[ end trace 933491c8a4fadeb7 ]---
[   88.320642] RIP: 0010:hidpp_probe+0x61f/0x948 [hid_logitech_hidpp]
[   88.320645] Code: 81 00 00 48 89 ef e8 f0 d6 ff ff 41 89 c6 85 c0 75 b5 0f b6 44 24 28 48 8b 5d 00 88 44 24 1e 89 44 24 0c 48 8b 83 18 1c 00 00 <48> 8b 48 18 48 8b 83 10 19 00 00 48 8b 40 40 48 89 0c 24 0f b7 80
[   88.320647] RSP: 0018:ffffb0a6824aba68 EFLAGS: 00010246
[   88.320650] RAX: 0000000000000000 RBX: ffff93a50756e000 RCX: 0000000000010408
[   88.320652] RDX: 0000000000000000 RSI: ffff93a51f0ad0a0 RDI: 000000000002d0a0
[   88.320655] RBP: ffff93a50416da28 R08: ffff93a50416da70 R09: ffff93a50416da70
[   88.320657] R10: 000000148ae9e60c R11: 00000000000f1525 R12: ffff93a50756e000
[   88.320659] R13: ffff93a50756f8d0 R14: 0000000000000000 R15: ffff93a50756fc38
[   88.320662] FS:  00007f8d8c1e0940(0000) GS:ffff93a51f080000(0000) knlGS:0000000000000000
[   88.320664] CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
[   88.320667] CR2: 0000000000000018 CR3: 00000003996d8003 CR4: 00000000001606e0

To solve this issue:

   1. Split g920_get_config() such that all of the device specific
      communication remains a part of the function and input subsystem
      initialization bits go to hidpp_ff_init()

   2. Move call to hidpp_ff_init() from being a part of
      g920_get_config() to be the last step of .probe(), right after a
      call to hid_hw_start() with connect_mask containing
      HID_CONNECT_HIDINPUT.

Fixes: 91cf9a9 ("HID: logitech-hidpp: make .probe usbhid capable")
Signed-off-by: Andrey Smirnov <[email protected]>
Tested-by: Sam Bazley <[email protected]>
Cc: Jiri Kosina <[email protected]>
Cc: Benjamin Tissoires <[email protected]>
Cc: Henrik Rydberg <[email protected]>
Cc: Pierre-Loup A. Griffais <[email protected]>
Cc: Austin Palmer <[email protected]>
Cc: [email protected]
Cc: [email protected]
Cc: [email protected] # 5.2+
Signed-off-by: Benjamin Tissoires <[email protected]>
sagimnl pushed a commit that referenced this issue Jul 16, 2020
…kernel/git/kvmarm/kvmarm into HEAD

KVM/arm fixes for 5.4, take #2

Special PMU edition:

- Fix cycle counter truncation
- Fix cycle counter overflow limit on pure 64bit system
- Allow chained events to be actually functional
- Correct sample period after overflow
sagimnl pushed a commit that referenced this issue Jul 16, 2020
All bonding device has same lockdep key and subclass is initialized with
nest_level.
But actual nest_level value can be changed when a lower device is attached.
And at this moment, the subclass should be updated but it seems to be
unsafe.
So this patch makes bonding use dynamic lockdep key instead of the
subclass.

Test commands:
    ip link add bond0 type bond

    for i in {1..5}
    do
	    let A=$i-1
	    ip link add bond$i type bond
	    ip link set bond$i master bond$A
    done
    ip link set bond5 master bond0

Splat looks like:
[  307.992912] WARNING: possible recursive locking detected
[  307.993656] 5.4.0-rc3+ #96 Tainted: G        W
[  307.994367] --------------------------------------------
[  307.995092] ip/761 is trying to acquire lock:
[  307.995710] ffff8880513aac60 (&(&bond->stats_lock)->rlock#2/2){+.+.}, at: bond_get_stats+0xb8/0x500 [bonding]
[  307.997045]
	       but task is already holding lock:
[  307.997923] ffff88805fcbac60 (&(&bond->stats_lock)->rlock#2/2){+.+.}, at: bond_get_stats+0xb8/0x500 [bonding]
[  307.999215]
	       other info that might help us debug this:
[  308.000251]  Possible unsafe locking scenario:

[  308.001137]        CPU0
[  308.001533]        ----
[  308.001915]   lock(&(&bond->stats_lock)->rlock#2/2);
[  308.002609]   lock(&(&bond->stats_lock)->rlock#2/2);
[  308.003302]
		*** DEADLOCK ***

[  308.004310]  May be due to missing lock nesting notation

[  308.005319] 3 locks held by ip/761:
[  308.005830]  #0: ffffffff9fcc42b0 (rtnl_mutex){+.+.}, at: rtnetlink_rcv_msg+0x466/0x8a0
[  308.006894]  #1: ffff88805fcbac60 (&(&bond->stats_lock)->rlock#2/2){+.+.}, at: bond_get_stats+0xb8/0x500 [bonding]
[  308.008243]  #2: ffffffff9f9219c0 (rcu_read_lock){....}, at: bond_get_stats+0x9f/0x500 [bonding]
[  308.009422]
	       stack backtrace:
[  308.010124] CPU: 0 PID: 761 Comm: ip Tainted: G        W         5.4.0-rc3+ #96
[  308.011097] Hardware name: innotek GmbH VirtualBox/VirtualBox, BIOS VirtualBox 12/01/2006
[  308.012179] Call Trace:
[  308.012601]  dump_stack+0x7c/0xbb
[  308.013089]  __lock_acquire+0x269d/0x3de0
[  308.013669]  ? register_lock_class+0x14d0/0x14d0
[  308.014318]  lock_acquire+0x164/0x3b0
[  308.014858]  ? bond_get_stats+0xb8/0x500 [bonding]
[  308.015520]  _raw_spin_lock_nested+0x2e/0x60
[  308.016129]  ? bond_get_stats+0xb8/0x500 [bonding]
[  308.017215]  bond_get_stats+0xb8/0x500 [bonding]
[  308.018454]  ? bond_arp_rcv+0xf10/0xf10 [bonding]
[  308.019710]  ? rcu_read_lock_held+0x90/0xa0
[  308.020605]  ? rcu_read_lock_sched_held+0xc0/0xc0
[  308.021286]  ? bond_get_stats+0x9f/0x500 [bonding]
[  308.021953]  dev_get_stats+0x1ec/0x270
[  308.022508]  bond_get_stats+0x1d1/0x500 [bonding]

Fixes: d3fff6c ("net: add netdev_lockdep_set_classes() helper")
Signed-off-by: Taehee Yoo <[email protected]>
Signed-off-by: David S. Miller <[email protected]>
sagimnl pushed a commit that referenced this issue Jul 16, 2020
When the extent tree is modified, it should be protected by inode
cluster lock and ip_alloc_sem.

The extent tree is accessed and modified in the
ocfs2_prepare_inode_for_write, but isn't protected by ip_alloc_sem.

The following is a case.  The function ocfs2_fiemap is accessing the
extent tree, which is modified at the same time.

  kernel BUG at fs/ocfs2/extent_map.c:475!
  invalid opcode: 0000 [#1] SMP
  Modules linked in: tun ocfs2 ocfs2_nodemanager configfs ocfs2_stackglue [...]
  CPU: 16 PID: 14047 Comm: o2info Not tainted 4.1.12-124.23.1.el6uek.x86_64 #2
  Hardware name: Oracle Corporation ORACLE SERVER X7-2L/ASM, MB MECH, X7-2L, BIOS 42040600 10/19/2018
  task: ffff88019487e200 ti: ffff88003daa4000 task.ti: ffff88003daa4000
  RIP: ocfs2_get_clusters_nocache.isra.11+0x390/0x550 [ocfs2]
  Call Trace:
    ocfs2_fiemap+0x1e3/0x430 [ocfs2]
    do_vfs_ioctl+0x155/0x510
    SyS_ioctl+0x81/0xa0
    system_call_fastpath+0x18/0xd8
  Code: 18 48 c7 c6 60 7f 65 a0 31 c0 bb e2 ff ff ff 48 8b 4a 40 48 8b 7a 28 48 c7 c2 78 2d 66 a0 e8 38 4f 05 00 e9 28 fe ff ff 0f 1f 00 <0f> 0b 66 0f 1f 44 00 00 bb 86 ff ff ff e9 13 fe ff ff 66 0f 1f
  RIP  ocfs2_get_clusters_nocache.isra.11+0x390/0x550 [ocfs2]
  ---[ end trace c8aa0c8180e869dc ]---
  Kernel panic - not syncing: Fatal exception
  Kernel Offset: disabled

This issue can be reproduced every week in a production environment.

This issue is related to the usage mode.  If others use ocfs2 in this
mode, the kernel will panic frequently.

[[email protected]: coding style fixes]
[Fix new warning due to unused function by removing said function - Linus ]
Link: http://lkml.kernel.org/r/[email protected]
Signed-off-by: Shuning Zhang <[email protected]>
Reviewed-by: Junxiao Bi <[email protected]>
Reviewed-by: Gang He <[email protected]>
Cc: Mark Fasheh <[email protected]>
Cc: Joel Becker <[email protected]>
Cc: Joseph Qi <[email protected]>
Cc: Changwei Ge <[email protected]>
Cc: Jun Piao <[email protected]>
Cc: <[email protected]>
Signed-off-by: Andrew Morton <[email protected]>
Signed-off-by: Linus Torvalds <[email protected]>
sagimnl pushed a commit that referenced this issue Jul 16, 2020
Call napi_disable() twice would cause dead lock. There are three situations
may result in the issue.

1. rtl8152_pre_reset() and set_carrier() are run at the same time.
2. Call rtl8152_set_tunable() after rtl8152_close().
3. Call rtl8152_set_ringparam() after rtl8152_close().

For #1, use the same solution as commit 8481141 ("r8152: Re-order
napi_disable in rtl8152_close"). For #2 and #3, add checking the flag
of IFF_UP and using napi_disable/napi_enable during mutex.

Signed-off-by: Hayes Wang <[email protected]>
Signed-off-by: David S. Miller <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant