jgaeddert / liquid-dsp

digital signal processing library for software-defined radios
http://liquidsdr.org
MIT License
1.9k stars 443 forks source link

Buffer overrun in qdetector #351

Open johnfb opened 11 months ago

johnfb commented 11 months ago

I was using the qdetector and my tooling said I had a buffer overrun inside it for one of my data sets. I managed to track it down to rxy_index being zero in qdetector_execute_seek. When rxy_index is zero, _q->counter will be _q->nfft on the next call, which results in writing one past the end in the function qdetector_execute_align. There is a todo comment here https://github.com/jgaeddert/liquid-dsp/blob/5432e317e3c31f069e8223d185c28078fc41c8cc/src/framing/src/qdetector.proto.c#L552 about dealing with this case.

The most straight forward way I can think to address this would be to split qdetector_execute_align into two functions and if rxy_index in qdetector_execute_seek is zero for a detection call the second half of the old qdetector_execute_align.

Here is a diff of what I think the change might be:

diff --git a/src/framing/src/qdetector.proto.c b/src/framing/src/qdetector.proto.c
index 5c13f45e..dd99a7b5 100644
--- a/src/framing/src/qdetector.proto.c
+++ b/src/framing/src/qdetector.proto.c
@@ -39,6 +39,7 @@ int QDETECTOR(_execute_seek)(QDETECTOR() _q, TI _x);

 // align signal in time, compute offset estimates
 int QDETECTOR(_execute_align)(QDETECTOR() _q, TI _x);
+int QDETECTOR(_execute_do_align)(QDETECTOR() _q);

 // main object definition
 struct QDETECTOR(_s) {
@@ -549,7 +550,9 @@ int QDETECTOR(_execute_seek)(QDETECTOR() _q, TI _x)
         _q->state = QDETECTOR_STATE_ALIGN;
         _q->offset = rxy_offset;
         _q->rxy    = rxy_peak; // note that this is a coarse estimate
-        // TODO: check for edge case where rxy_index is zero (signal already aligned)
+        if (0 == rxy_index) {
+            return QDETECTOR(_execute_do_align)(_q);
+        }

         // copy last part of fft input buffer to front
         memmove(_q->buf_time_0, _q->buf_time_0 + rxy_index, (_q->nfft - rxy_index)*sizeof(TI));
@@ -578,7 +581,11 @@ int QDETECTOR(_execute_align)(QDETECTOR() _q, TI _x)

     if (_q->counter < _q->nfft)
         return LIQUID_OK;
+    return QDETECTOR(_execute_do_align)(_q);
+}

+int QDETECTOR(_execute_do_align)(QDETECTOR() _q)
+{
     //printf("signal is aligned!\n");

     // estimate timing offset

As an aside, this library is great, thank you so much for writing it and providing it for our use.

luzpaz commented 9 months ago

cc @jgaeddert