Bug in bessel.c

[dj0abr]

first thank you for this outstanding library, I did a couple of projects and all worked very well.

I have found a bug in the iir filter library and was able to track it down to the bessel function:

Please see iirdes.c, line 552, function call: bessel_azpkf(_n,za,pa,&ka); variable pa is an array with length _n

Then see bessel.c, line 73: in bessel_azpkf you make a call to fpoly_bessel_roots(_n+1,_pa); note the _n+1, but pa has only length _n.

This results in a memory overflow, which often remains undetected, but can lead to unexpected program crashes from time to time.

If I change the definition of the array pa[_n] to pa[_n+1] then everything is ok, that proves that the error is here.

Something similar happens with butterfly (butter_azpkf function), you should check if k>_n in the for loop already, currently it is checked only at the end assert(k==_n); which may be too late. I never had crashes with elliptical filter, but maybe there is something similar, I think its worth to check it out.

Temporary solution: until you have fixed this issue we can solve it by changing the definition of pa: iirdes.c, line 504: change float complex pa[_n]; into float complex pa[_n * 2]; This take more memory, but I never had any more crashes after this change.

since iir filters are used in many other functions this may be a solution for many other unclear crashes.

Kurt

[jgaeddert]

Ah! Good catch. I'll check this out soon.

[fsheikh]

Just another data point, very nicely caught by clang's address sanitizer.

==20169==ERROR: AddressSanitizer: dynamic-stack-buffer-overflow on address 0x7ffd33d485c8 at pc 0x00000053ff03 bp 0x7ffd33d47ab0 sp 0x7ffd33d47aa8
WRITE of size 4 at 0x7ffd33d485c8 thread T0
    #0 0x53ff02 in fpoly_bessel_roots_orchard /home/fsheikh/liquid-dsp/src/filter/src/bessel.c:193:23
    #1 0x53dafa in fpoly_bessel_roots /home/fsheikh/liquid-dsp/src/filter/src/bessel.c:127:5
    #2 0x53d2e0 in bessel_azpkf /home/fsheikh/liquid-dsp/src/filter/src/bessel.c:73:5
    #3 0x5041b3 in liquid_iirdes /home/fsheikh/liquid-dsp/src/filter/src/iirdes.c:552:9
    #4 0x4f6c95 in main /home/fsheikh/liquid-dsp/examples/iirdes_example.c:153:5
    #5 0x7f8c1541fb96 in __libc_start_main /build/glibc-2ORdQG/glibc-2.27/csu/../csu/libc-start.c:310
    #6 0x41b039 in _start (/home/fsheikh/liquid-dsp/examples/iirdes_example+0x41b039)

Address 0x7ffd33d485c8 is located in stack of thread T0 at offset 744 in frame
    #0 0x53d18f in bessel_azpkf /home/fsheikh/liquid-dsp/src/filter/src/bessel.c:71

  This frame has 1 object(s):
    [32, 40) 'coerce' <== Memory access at offset 744 overflows this variable
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: dynamic-stack-buffer-overflow /home/fsheikh/liquid-dsp/src/filter/src/bessel.c:193:23 in fpoly_bessel_roots_orchard
Shadow bytes around the buggy address:
  0x1000267a1060: 00 f3 f3 f3 00 00 00 00 00 00 00 00 00 00 00 00
  0x1000267a1070: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x1000267a1080: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x1000267a1090: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x1000267a10a0: ca ca ca ca 00 00 00 00 00 cb cb cb cb cb cb cb
=>0x1000267a10b0: ca ca ca ca 00 00 00 00 00[cb]cb cb cb cb cb cb
  0x1000267a10c0: f1 f1 f1 f1 00 f2 f2 f2 00 f2 f2 f2 f8 f2 f2 f2
  0x1000267a10d0: 00 f2 f2 f2 00 f2 f2 f2 00 f3 f3 f3 00 00 00 00
  0x1000267a10e0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x1000267a10f0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x1000267a1100: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
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
  Shadow gap:              cc
==20169==ABORTING

[dj0abr]

just a question, has this problem been addressed already?

[jgaeddert]

@andreasbombe fixed this; sorry for taking so long to get back to it