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

SIGSEGV with memcpy() in rx_meter_c::work() (src/dsp/rx_meter.cpp:73) on FreeBSD #1275

Closed
martymac opened this issue Aug 31, 2023 · 16 comments
Closed
Labels

Comments

@martymac
Copy link
Contributor

Hello,

Gqrx crashes on FreeBSD 13.2 (amd64, with Gnuradio 3.8.4). I get the following backtrace :

Thread 14 received signal SIGSEGV, Segmentation fault.
Address not mapped to object.
[Switching to LWP 105501 of process 12505]
memcpy () at /usr/src/lib/libc/amd64/string/memmove.S:306
warning: Source file is more recent than executable.
306             MEMMOVE erms=0 overlap=1 begin=MEMMOVE_BEGIN end=MEMMOVE_END
(gdb) bt full
#0  memcpy () at /usr/src/lib/libc/amd64/string/memmove.S:306
No locals.
#1  0x00000000003446c5 in rx_meter_c::work (this=0x804bf2c00, noutput_items=512, input_items=std::vector of length 1 = {...}, output_items=std::vector of length 0) at /usr/ports/comms/gqrx/work/gqrx-2.16/src/dsp/rx_meter.cpp:73
        lock = {__m_ = @0x804bf2e40}
        in = 0x80df7b710
        items_to_copy = 512
#2  0x0000000801fe9c59 in gr::sync_block::general_work(int, std::__1::vector<int, std::__1::allocator<int> >&, std::__1::vector<void const*, std::__1::allocator<void const*> >&, std::__1::vector<void*, std::__1::allocator<void*> >&) ()
   from /usr/local/lib/libgnuradio-runtime.so.3.10.7

The problem occurs with an up-to-date master branch and is triggered with a rtl-sdr dongle when starting DSP with any of the 3 WFM modes.

It seems to be related to the use of GNU Radio buffers. I tried to use a fixed buffer size in src/dsp/rx_meter.cpp (inspired by commit 8d03fd1) and it fixed the problem.

I basically increased the number of items when calling gr::make_buffer() that way:

--- src/dsp/rx_meter.cpp.orig
+++ src/dsp/rx_meter.cpp
@@ -26,6 +26,7 @@
 #include <dsp/rx_meter.h>
 #include <iostream>
 
+#define MAX_METER_SIZE (1024 * 1024 * 4)
 
 rx_meter_c_sptr make_rx_meter_c(double quad_rate)
 {
@@ -41,9 +42,9 @@
 {
     /* allocate circular buffer */
 #if GNURADIO_VERSION < 0x031000
-    d_writer = gr::make_buffer(d_avgsize + d_quadrate, sizeof(gr_complex));
+    d_writer = gr::make_buffer(MAX_METER_SIZE * 2, sizeof(gr_complex));
 #else
-    d_writer = gr::make_buffer(d_avgsize + d_quadrate, sizeof(gr_complex), 1, 1);
+    d_writer = gr::make_buffer(MAX_METER_SIZE * 2, sizeof(gr_complex), 1, 1);
 #endif
     d_reader = gr::buffer_add_reader(d_writer, 0);

but the patch is obviously not clean at all and probably requires more changes.

Could you have a look at that, please ?

(the problem has originally been reported here: https://bugs.freebsd.org/bugzilla/show_bug.cgi?id=272543)

Thanks in advance,
Best regards,

Ganael.

@argilo
Copy link
Member

argilo commented Sep 1, 2023

Hi there,

I can't reproduce this on Linux or MacOS. The buffer size calculations look correct, and I'm starting to suspect there could be an issue with GNU Radio's double-mapped buffers on FreeBSD.

@argilo
Copy link
Member

argilo commented Sep 1, 2023

I installed GNU Radio on FreeBSD and immediately ran into a similar segfault when attempting to run a simple flow graph (Noise Source --> Throttle --> Null Sink). So it would seem that the issue lies in GNU Radio or FreeBSD's GNU Radio package, rather then Gqrx.

@martymac
Copy link
Contributor Author

martymac commented Sep 5, 2023

Hello @argilo

Thanks for your reply and investigation. Well, I'll try to investigate on the GNURadio side then !

Best regards,

Ganael.

@martymac martymac closed this as completed Sep 5, 2023
@martymac
Copy link
Contributor Author

martymac commented Sep 8, 2023

Hello,

I could reproduce the immediate crash you are observing, it is related to ASLR. Gnuradio seems to have problems when it is enabled. If you disable it globally with (before running Gr flow's python code) :

# sysctl kern.elf64.aslr.enable=0

your simple flow graph (Noise Source --> Throttle --> Null Sink) will not crash anymore.

Actually, I had to disable ASLR for Gqrx to make it work again when ASLR has been turned on by default on FreeBSD, see:

https://cgit.freebsd.org/ports/commit/?id=f05f6dcedb5dad5b7b515ebede9bf6a58de1d46c

so Gqrx' crash I am writing about here is not related as what you are observing happens with ASLR disabled.

The backtrace above clearly shows an access to an invalid write pointer during memcpy() (see: /usr/ports/comms/gqrx/work/gqrx-2.16/src/dsp/rx_meter.cpp:73) that triggers SIGSEGV.

What I don't know is if it is due to a bug in Gqrx code or in Gnuradio buffers handling on FreeBSD :/

If you want to reproduce the issue on FreeBSD and need help, do not hesitate to contact me, I'll help you setting things up.

Best regards,

Ganael.

@martymac martymac reopened this Sep 8, 2023
@martymac
Copy link
Contributor Author

martymac commented Sep 9, 2023

An interesting thing: if I start Gqrx with WFM preselected and just turn DSP on, it does not crash. Problems arise when changing DSP mode (e.g. AM -> WFM x). Could there be a problem with de-allocation/re-allocation of Gr buffers when switching mode ?

@willcode
Copy link
Contributor

willcode commented Sep 9, 2023

This may be related to #1265. The fix also requires GNU Radio v3.10.7.0, which fixes a bug in buffer size limts.

@martymac
Copy link
Contributor Author

Hello Jeff,

Thanks for your feedback.

This does not appear to be the same symptom as in #1233 (fixed by PR #1265): the DSP does not hang, but Gqrx crashes with SIGSEGV.

Unfortunately, the same bug still occurs with Gnuradio 3.10.7.0 and latest master (d7219a6) from Gqrx.

@argilo
Copy link
Member

argilo commented Sep 18, 2023

@martymac I did some more digging, and it appears the issue lies in GNU Radio. I used truss to monitor allocation of GNU Radio's double-mapped buffers, and it can be seen that the buffer that causes a segfault in Gqrx is not contiguous:

getpid()                                         = 1230 (0x4ce)
shm_open2("/gnuradio-1230-20",O_RDWR|O_CREAT|O_EXCL|O_CLOEXEC,0600,0,"(null)") = 21 (0x15)
ftruncate(21,0x408000)                           = 0 (0x0)
mmap(0x0,4227072,PROT_READ|PROT_WRITE,MAP_SHARED,21,0x0) = 34758197248 (0x817c00000)
munmap(0x817e04000,2113536)                      = 0 (0x0)
mmap(0x817e04000,2113536,PROT_READ|PROT_WRITE,MAP_SHARED,21,0x0) = 34762391552 (0x818000000)
close(21)                                        = 0 (0x0)
shm_unlink("/gnuradio-1230-20")                  = 0 (0x0)

GNU Radio requested that the second half of the double-mapped buffer be allocated at 0x817e04000 (immediately after the first half), but the kernel did not accept this hint and instead allocated it at 0x818000000. Gqrx segfaults as soon as it attempts to write beyond the end of the first half of the double-mapped buffer.

GNU Radio's vmcircbuf_mmap_shm_open::vmcircbuf_mmap_shm_open constructor does not check for discontiguity:

https://github.com/gnuradio/gnuradio/blob/f5d35bcfc137c6314545a16300a8e75e684359f0/gnuradio-runtime/lib/vmcircbuf_mmap_shm_open.cc#L111-L124

Interestingly, one of the other double-mapped buffer implementations (vmcircbuf_mmap_tmpfile::vmcircbuf_mmap_tmpfile) does have a contiguity check, although it simply raises a std::runtime_error if the allocation was discontiguous:

https://github.com/gnuradio/gnuradio/blob/f5d35bcfc137c6314545a16300a8e75e684359f0/gnuradio-runtime/lib/vmcircbuf_mmap_tmpfile.cc#L98-L121

@argilo
Copy link
Member

argilo commented Sep 18, 2023

mmap has a MAP_FIXED flag which indicates that the exact addr value must be used. Perhaps GNU Radio should be using that.

@argilo
Copy link
Member

argilo commented Sep 18, 2023

The crash in the simple flow graph when ASLR is enabled appears to have the same root cause; truss shows that a discontiguous double-mapped buffer is allocated in that case as well.

@willcode
Copy link
Contributor

The order of precedence is

  • vmcircbuf_createfilemapping
  • vmcircbuf_sysv_shm
  • vmcircbuf_mmap_shm
  • vmcircbuf_mmap_tmpfile

Linux seems to work with sysv and createfilemapping is for windows, so the mmap versions probably don't get tested much. I take it FreeBSD doesn't have sysv IPC?

@argilo
Copy link
Member

argilo commented Sep 18, 2023

I opened a pull request, which I hope will fix this: gnuradio/gnuradio#6854

@martymac Would you be able to take this for a spin on FreeBSD? I expect that things should work correctly even when ALSR is enabled.

@argilo argilo added the bug label Sep 19, 2023
@argilo
Copy link
Member

argilo commented Sep 19, 2023

Closing, since the root cause was a bug in GNU Radio, which was fixed in gnuradio/gnuradio#6854.

@argilo argilo closed this as completed Sep 19, 2023
@martymac
Copy link
Contributor Author

Thanks a lot for your help, Clayton! I'll give it a try and tell you how it goes :)

@argilo
Copy link
Member

argilo commented Sep 19, 2023

Glad I could help.

If you run into any trouble after applying the GNU Radio patch, let me know.

@martymac
Copy link
Contributor Author

Everything seems to be working fine with your patch :) Thanks again for your precious help!

freebsd-git pushed a commit to freebsd/freebsd-ports that referenced this issue Sep 20, 2023
This patch is a backport of ca44241 from upstream:
runtime: Use MAP_FIXED flag to ensure buffer halves are contiguous

It fixes SIGSEGV observed with GNU Radio buffers consumers such as
comms/gqrx.

Discussed here:
  gqrx-sdr/gqrx#1275
  gnuradio/gnuradio#6854

PR:	272543
Reported by:	trasz
Obtained from:	GNU Radio team (GH pull request: 6854)
MFH:	2023Q3
freebsd-git pushed a commit to freebsd/freebsd-ports that referenced this issue Sep 20, 2023
This patch is a backport of ca44241 from upstream:
runtime: Use MAP_FIXED flag to ensure buffer halves are contiguous

It fixes SIGSEGV observed with GNU Radio buffers consumers such as
comms/gqrx.

Discussed here:
  gqrx-sdr/gqrx#1275
  gnuradio/gnuradio#6854

PR:	272543
Reported by:	trasz
Obtained from:	GNU Radio team (GH pull request: 6854)
MFH:	2023Q3

(cherry picked from commit 35f7383)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

3 participants