search

rockowitz / ddcutil

Control monitor settings using DDC/CI and USB
http://www.ddcutil.com
GNU General Public License v2.0
954 stars 39 forks source link

stack overflow in query_proc_modules_for_video leading to segfault #354

Closed nekopsykose closed 8 months ago

nekopsykose commented 10 months ago

when building with clang -flto (non-lto doesn't crash, weirdly), the following crash can be seen in ddcutil environment --verbose: 

Scanning /proc/modules for driver environment...
   Found other loaded module:      i2c_piix4
   Found other loaded module:      i2c_dev
   Found video driver module:      amdgpu
   Found other loaded module:      drm_buddy
   Found other loaded module:      drm_suballoc_helper
   Loaded video module depends on: amdgpu,
   Found other loaded module:      drm_ttm_helper
   Found other loaded module:      i2c_algo_bit
   Found other loaded module:      drm_display_helper
   Found other loaded module:      drm_kms_helper
   Loaded drm module depends on:   amdgpu,drm_buddy,drm_suballoc_helper,drm_ttm_helper,ttm,drm_display_helper,drm_kms_helper,gpu_sched,amdxcp,
zsh: segmentation fault (core dumped)  ./src/ddcutil environment --verbose

using asan gives something a bit more useful, but the stacktrace itself is probably not completely useful

==4594==ERROR: AddressSanitizer: stack-buffer-overflow on address 0x7fffedd77840 at pc 0x5555558b596d bp 0x7fffffffd720 sp 0x7fffffffceb0
WRITE of size 34 at 0x7fffedd77840 thread T0
    #0 0x5555558b596c in scanf_common(void*, int, bool, char const*, __va_list_tag*) crtbegin.c
    #1 0x5555558b626b in sscanf (/tmp/mytemp.kOKBjh/src/ddcutil+0x36226b) (BuildId: d93c61db81f91395)
    #2 0x55555598d3e6 in query_proc_modules_for_video /tmp/mytemp.kOKBjh/src/app_sysenv/query_sysenv_procfs.c:55:25
    #3 0x555555970355 in query_sysenv /tmp/mytemp.kOKBjh/src/app_sysenv/query_sysenv.c:879:10
    #4 0x55555595b6a3 in main /tmp/mytemp.kOKBjh/src/app_ddcutil/main.c:978:7

Address 0x7fffedd77840 is located in stack of thread T0 at offset 64 in frame
    #0 0x55555598cf37 in query_proc_modules_for_video /tmp/mytemp.kOKBjh/src/app_sysenv/query_sysenv_procfs.c:30

  This frame has 6 object(s):
    [32, 64) 'mod_name' (line 48)
    [96, 100) 'mod_size' (line 49) <== Memory access at offset 64 partially underflows this variable
    [112, 116) 'mod_instance_ct' (line 50)
    [128, 628) 'mod_dependencies' (line 51)
    [704, 714) 'mod_load_state' (line 52)
    [736, 766) 'mod_addr' (line 53)
HINT: this may be a false positive if your program uses some custom stack unwind mechanism, swapcontext or vfork
      (longjmp and C++ exceptions *are* supported)
SUMMARY: AddressSanitizer: stack-buffer-overflow crtbegin.c in scanf_common(void*, int, bool, char const*, __va_list_tag*)
Shadow bytes around the buggy address:
  0x7fffedd77580: f5 f5 f5 f5 f5 f5 f5 f5 f5 f5 f5 f5 f5 f5 f5 f5
  0x7fffedd77600: f5 f5 f5 f5 f5 f5 f5 f5 f5 f5 f5 f5 f5 f5 f5 f5
  0x7fffedd77680: f5 f5 f5 f5 f5 f5 f5 f5 f5 f5 f5 f5 f5 f5 f5 f5
  0x7fffedd77700: f5 f5 f5 f5 f5 f5 f5 f5 f5 f5 f5 f5 f5 f5 f5 f5
  0x7fffedd77780: f5 f5 f5 f5 f5 f5 f5 f5 f5 f5 f5 f5 f5 f5 f5 f5
=>0x7fffedd77800: f1 f1 f1 f1 00 00 00 00[f2]f2 f2 f2 04 f2 04 f2
  0x7fffedd77880: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x7fffedd77900: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x7fffedd77980: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x7fffedd77a00: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 04 f2
  0x7fffedd77a80: f2 f2 f2 f2 f2 f2 f2 f2 00 02 f2 f2 00 00 00 06
Shadow byte legend (one shadow byte represents 8 application bytes):
  Addressable:           00
  Partially addressable: 01 02 03 04 05 06 07 
  Heap left redzone:       fa
  Freed heap region:       fd
  Stack left redzone:      f1
  Stack mid redzone:       f2
  Stack right redzone:     f3
  Stack after return:      f5
  Stack use after scope:   f8
  Global redzone:          f9
  Global init order:       f6
  Poisoned by user:        f7
  Container overflow:      fc
  Array cookie:            ac
  Intra object redzone:    bb
  ASan internal:           fe
  Left alloca redzone:     ca
  Right alloca redzone:    cb
==4594==ABORTING

this is weird because looking at curline in the previous iterations (maybe i didn't look far back enough, and it's weird the sanitizers don't instantly report the overflow but only later when it crashes, unless i'm misreading it), the contents don't seem to overflow the sscanf format string. nevertheless, specifying a size for it to match the declaration:

diff --git a/src/app_sysenv/query_sysenv_procfs.c b/src/app_sysenv/query_sysenv_procfs.c
index f8cdd64e..04cd65f6 100644
--- a/src/app_sysenv/query_sysenv_procfs.c
+++ b/src/app_sysenv/query_sysenv_procfs.c
@@ -45,13 +45,13 @@ int query_proc_modules_for_video() {
       int ndx = 0;
       for (ndx=0; ndx<garray->len; ndx++) {
          char * curline = g_ptr_array_index(garray, ndx);
-         char mod_name[32];
-         int  mod_size;
-         int  mod_instance_ct;
-         char mod_dependencies[500];
-         char mod_load_state[10];     // one of: Live Loading Unloading
-         char mod_addr[30];
-         int piece_ct = sscanf(curline, "%s %d %d %s %s %s",
+         char mod_name[32] = {};
+         int  mod_size = 0;
+         int  mod_instance_ct = 0;
+         char mod_dependencies[500] = {};
+         char mod_load_state[10] = {};     // one of: Live Loading Unloading
+         char mod_addr[30] = {};
+         int piece_ct = sscanf(curline, "%31s %d %d %499s %9s %29s",
                                mod_name,
                                &mod_size,
                                &mod_instance_ct,

fixes this crash entirely. perhaps the sscanf should pass the size of the params (either like this, or some more automated way) so it doesn't overflow?

rockowitz commented 10 months ago

What is the value of /proc/modules being parsed?

nekopsykose commented 10 months ago

a cat looks like

$ cat /proc/modules
hidp 32768 0 - Live 0x0000000000000000
rfcomm 86016 7 - Live 0x0000000000000000
snd_seq_dummy 12288 0 - Live 0x0000000000000000
snd_hrtimer 12288 1 - Live 0x0000000000000000
tls 139264 0 - Live 0x0000000000000000
overlay 176128 2 - Live 0x0000000000000000
cmac 12288 3 - Live 0x0000000000000000
zram 32768 2 - Live 0x0000000000000000
algif_hash 12288 1 - Live 0x0000000000000000
algif_skcipher 12288 1 - Live 0x0000000000000000
af_alg 32768 6 algif_hash,algif_skcipher, Live 0x0000000000000000
bnep 32768 2 - Live 0x0000000000000000
cfg80211 1167360 0 - Live 0x0000000000000000
8021q 45056 0 - Live 0x0000000000000000
mrp 16384 1 8021q, Live 0x0000000000000000
garp 12288 1 8021q, Live 0x0000000000000000
stp 12288 1 garp, Live 0x0000000000000000
llc 12288 2 garp,stp, Live 0x0000000000000000
wireguard 110592 0 - Live 0x0000000000000000
libchacha20poly1305 12288 1 wireguard, Live 0x0000000000000000
poly1305_x86_64 24576 1 libchacha20poly1305, Live 0x0000000000000000
chacha_x86_64 28672 1 libchacha20poly1305, Live 0x0000000000000000
curve25519_x86_64 90112 1 wireguard, Live 0x0000000000000000
libcurve25519_generic 49152 2 wireguard,curve25519_x86_64, Live 0x0000000000000000
libchacha 12288 1 chacha_x86_64, Live 0x0000000000000000
ip6_udp_tunnel 12288 1 wireguard, Live 0x0000000000000000
udp_tunnel 20480 1 wireguard, Live 0x0000000000000000
binfmt_misc 24576 1 - Live 0x0000000000000000
xfs 2154496 1 - Live 0x0000000000000000
nls_iso8859_1 12288 1 - Live 0x0000000000000000
nls_cp437 16384 1 - Live 0x0000000000000000
vfat 20480 1 - Live 0x0000000000000000
fat 90112 1 vfat, Live 0x0000000000000000
intel_rapl_msr 16384 0 - Live 0x0000000000000000
intel_rapl_common 32768 1 intel_rapl_msr, Live 0x0000000000000000
edac_mce_amd 36864 0 - Live 0x0000000000000000
kvm_amd 180224 0 - Live 0x0000000000000000
snd_hda_codec_realtek 176128 1 - Live 0x0000000000000000
snd_hda_codec_generic 102400 1 snd_hda_codec_realtek, Live 0x0000000000000000
ledtrig_audio 12288 1 snd_hda_codec_generic, Live 0x0000000000000000
kvm 1179648 1 kvm_amd, Live 0x0000000000000000
irqbypass 12288 1 kvm, Live 0x0000000000000000
crct10dif_pclmul 12288 1 - Live 0x0000000000000000
crc32_pclmul 12288 0 - Live 0x0000000000000000
polyval_clmulni 12288 0 - Live 0x0000000000000000
polyval_generic 12288 1 polyval_clmulni, Live 0x0000000000000000
snd_hda_codec_hdmi 86016 1 - Live 0x0000000000000000
gf128mul 16384 1 polyval_generic, Live 0x0000000000000000
sd_mod 77824 1 - Live 0x0000000000000000
ghash_clmulni_intel 12288 0 - Live 0x0000000000000000
sha512_ssse3 49152 0 - Live 0x0000000000000000
snd_hda_intel 49152 2 - Live 0x0000000000000000
aesni_intel 360448 4 - Live 0x0000000000000000
snd_intel_dspcfg 28672 1 snd_hda_intel, Live 0x0000000000000000
crypto_simd 12288 1 aesni_intel, Live 0x0000000000000000
cryptd 24576 3 ghash_clmulni_intel,crypto_simd, Live 0x0000000000000000
r8169 126976 0 - Live 0x0000000000000000
snd_intel_sdw_acpi 16384 1 snd_intel_dspcfg, Live 0x0000000000000000
btusb 77824 0 - Live 0x0000000000000000
ahci 49152 1 - Live 0x0000000000000000
rapl 16384 0 - Live 0x0000000000000000
snd_hda_codec 180224 4 snd_hda_codec_realtek,snd_hda_codec_generic,snd_hda_codec_hdmi,snd_hda_intel, Live 0x0000000000000000
ccp 131072 1 kvm_amd, Live 0x0000000000000000
sp5100_tco 16384 0 - Live 0x0000000000000000
wmi_bmof 12288 0 - Live 0x0000000000000000
libahci 53248 1 ahci, Live 0x0000000000000000
btbcm 20480 1 btusb, Live 0x0000000000000000
btrtl 28672 1 btusb, Live 0x0000000000000000
rng_core 16384 1 ccp, Live 0x0000000000000000
i2c_piix4 24576 0 - Live 0x0000000000000000
pcspkr 12288 0 - Live 0x0000000000000000
snd_hda_core 114688 5 snd_hda_codec_realtek,snd_hda_codec_generic,snd_hda_codec_hdmi,snd_hda_intel,snd_hda_codec, Live 0x0000000000000000
k10temp 12288 0 - Live 0x0000000000000000
realtek 36864 1 - Live 0x0000000000000000
libata 397312 2 ahci,libahci, Live 0x0000000000000000
snd_hwdep 16384 1 snd_hda_codec, Live 0x0000000000000000
btmtk 12288 1 btusb, Live 0x0000000000000000
tiny_power_button 12288 0 - Live 0x0000000000000000
snd_pcm 172032 4 snd_hda_codec_hdmi,snd_hda_intel,snd_hda_codec,snd_hda_core, Live 0x0000000000000000
btintel 49152 1 btusb, Live 0x0000000000000000
scsi_mod 237568 2 sd_mod,libata, Live 0x0000000000000000
button 20480 0 - Live 0x0000000000000000
scsi_common 12288 3 sd_mod,libata,scsi_mod, Live 0x0000000000000000
i2c_dev 20480 0 - Live 0x0000000000000000
snd_seq 114688 7 snd_seq_dummy, Live 0x0000000000000000
snd_seq_device 12288 1 snd_seq, Live 0x0000000000000000
snd_timer 45056 3 snd_hrtimer,snd_pcm,snd_seq, Live 0x0000000000000000
snd 118784 16 snd_hda_codec_realtek,snd_hda_codec_generic,snd_hda_codec_hdmi,snd_hda_intel,snd_hda_codec,snd_hwdep,snd_pcm,snd_seq,snd_seq_device,snd_timer, Live 0x0000000000000000
soundcore 12288 1 snd, Live 0x0000000000000000
vhost_vsock 24576 0 - Live 0x0000000000000000
vmw_vsock_virtio_transport_common 45056 1 vhost_vsock, Live 0x0000000000000000
vsock 53248 2 vhost_vsock,vmw_vsock_virtio_transport_common, Live 0x0000000000000000
vhost_net 36864 0 - Live 0x0000000000000000
vhost 57344 2 vhost_vsock,vhost_net, Live 0x0000000000000000
vhost_iotlb 12288 1 vhost, Live 0x0000000000000000
tap 32768 1 vhost_net, Live 0x0000000000000000
hci_vhci 24576 0 - Live 0x0000000000000000
bluetooth 970752 44 hidp,rfcomm,bnep,btusb,btbcm,btrtl,btmtk,btintel,hci_vhci, Live 0x0000000000000000
ecdh_generic 12288 2 bluetooth, Live 0x0000000000000000
rfkill 36864 3 cfg80211,bluetooth, Live 0x0000000000000000
ecc 53248 1 ecdh_generic, Live 0x0000000000000000
crc16 12288 1 bluetooth, Live 0x0000000000000000
vfio_iommu_type1 45056 0 - Live 0x0000000000000000
vfio 49152 1 vfio_iommu_type1, Live 0x0000000000000000
iommufd 81920 1 vfio, Live 0x0000000000000000
uhid 16384 0 - Live 0x0000000000000000
dm_mod 184320 0 - Live 0x0000000000000000
uinput 20480 1 - Live 0x0000000000000000
userio 12288 0 - Live 0x0000000000000000
ppp_generic 49152 0 - Live 0x0000000000000000
slhc 12288 1 ppp_generic, Live 0x0000000000000000
tun 65536 1 vhost_net, Live 0x0000000000000000
loop 32768 0 - Live 0x0000000000000000
nvram 12288 0 - Live 0x0000000000000000
cuse 16384 0 - Live 0x0000000000000000
fuse 200704 3 cuse, Live 0x0000000000000000
btrfs 1912832 1 - Live 0x0000000000000000
raid6_pq 118784 1 btrfs, Live 0x0000000000000000
xor 20480 1 btrfs, Live 0x0000000000000000
libcrc32c 12288 2 xfs,btrfs, Live 0x0000000000000000
crc32c_generic 12288 0 - Live 0x0000000000000000
xxhash_generic 12288 1 - Live 0x0000000000000000
joydev 24576 0 - Live 0x0000000000000000
evdev 24576 26 - Live 0x0000000000000000
input_leds 12288 0 - Live 0x0000000000000000
mac_hid 12288 0 - Live 0x0000000000000000
hid_generic 12288 0 - Live 0x0000000000000000
usbkbd 12288 0 - Live 0x0000000000000000
usbmouse 12288 0 - Live 0x0000000000000000
usbhid 65536 0 - Live 0x0000000000000000
hid 151552 4 hidp,uhid,hid_generic,usbhid, Live 0x0000000000000000
amdgpu 10854400 41 - Live 0x0000000000000000
iommu_v2 20480 1 amdgpu, Live 0x0000000000000000
drm_buddy 16384 1 amdgpu, Live 0x0000000000000000
drm_suballoc_helper 12288 1 amdgpu, Live 0x0000000000000000
video 69632 1 amdgpu, Live 0x0000000000000000
drm_ttm_helper 12288 1 amdgpu, Live 0x0000000000000000
ttm 90112 2 amdgpu,drm_ttm_helper, Live 0x0000000000000000
agpgart 36864 1 ttm, Live 0x0000000000000000
i2c_algo_bit 16384 1 amdgpu, Live 0x0000000000000000
drm_display_helper 163840 1 amdgpu, Live 0x0000000000000000
cec 73728 1 drm_display_helper, Live 0x0000000000000000
rc_core 61440 1 cec, Live 0x0000000000000000
xhci_pci 20480 0 - Live 0x0000000000000000
xhci_pci_renesas 20480 1 xhci_pci, Live 0x0000000000000000
drm_kms_helper 188416 4 amdgpu,drm_display_helper, Live 0x0000000000000000
xhci_hcd 303104 1 xhci_pci, Live 0x0000000000000000
gpu_sched 49152 1 amdgpu, Live 0x0000000000000000
amdxcp 12288 1 amdgpu, Live 0x0000000000000000
drm 606208 26 amdgpu,drm_buddy,drm_suballoc_helper,drm_ttm_helper,ttm,drm_display_helper,drm_kms_helper,gpu_sched,amdxcp, Live 0x0000000000000000
usbcore 335872 6 btusb,usbkbd,usbmouse,usbhid,xhci_pci,xhci_hcd, Live 0x0000000000000000
crc32c_intel 16384 1 - Live 0x0000000000000000
usb_common 12288 2 xhci_hcd,usbcore, Live 0x0000000000000000
wmi 32768 2 wmi_bmof,video, Live 0x0000000000000000
gpio_amdpt 16384 0 - Live 0x0000000000000000
gpio_generic 20480 1 gpio_amdpt, Live 0x0000000000000000
rockowitz commented 10 months ago

I'm unable to reproduce your stack overflow. What are your full compiler options? Please submit the output of a ddcutil environment --verbose command that succeeds. The sscanf() documentation makes no mention of having to initialize the variables that will be set, and logically I don't see any reason for it. I suspect there's some over-aggressive optimization going on.

rockowitz commented 8 months ago

Unclear why the initialization in the patch should solve the problem, but there's no harm in doing it. Patch applied in commit 5f9d97f6d530dfe1616b8983077092acb7a3981f.

nekopsykose commented 8 months ago

thanks!