From 5dd293ed0e07f679f20800438797cdc239c12c61 Mon Sep 17 00:00:00 2001 From: Carles Fernandez Date: Wed, 23 Sep 2026 12:11:27 +0200 Subject: [PATCH] Fix heap overflow in set_doppler_num_bins() for N = full grid - 1 A narrowed search of N < d_num_doppler_bins candidates adds up to two noise-reference rows, so N = d_num_doppler_bins - 1 needs d_num_doppler_bins + 1 rows. The step-1 grid buffers were sized to the full grid's own row count, which has no reference rows when the peak-ratio statistic is used or the CFAR wraparound distance suffices, so that request wrote one row past the end of d_grid_doppler_wipeoffs and d_magnitude_grid. Size the step-1 buffers (and the CUDA engine) for d_num_doppler_bins + 2 rows, which covers every row count set_doppler_num_bins() can produce, and add a regression test. --- .../gnuradio_blocks/pcps_acquisition.cc | 7 ++++--- .../gnuradio_blocks/pcps_acquisition.h | 15 +++++++++------ .../pcps_acquisition_doppler_narrowing_test.cc | 13 +++++++++++++ 3 files changed, 26 insertions(+), 9 deletions(-) diff --git a/src/algorithms/acquisition/gnuradio_blocks/pcps_acquisition.cc b/src/algorithms/acquisition/gnuradio_blocks/pcps_acquisition.cc index ed7d6a9ad..c17fff78d 100644 --- a/src/algorithms/acquisition/gnuradio_blocks/pcps_acquisition.cc +++ b/src/algorithms/acquisition/gnuradio_blocks/pcps_acquisition.cc @@ -144,6 +144,7 @@ pcps_acquisition::pcps_acquisition(const Acq_Conf& conf_) // comment for why the count differs at exactly 1 candidate) -- so this // ceiling reserves 2 rows, not 1, whenever the row is needed at all. d_num_doppler_bins_full_grid_active(d_num_doppler_bins + (d_full_grid_reference_needs_extra_row ? (d_num_doppler_bins > 1U ? 2U : 1U) : 0U)), + d_num_doppler_bins_capacity(d_num_doppler_bins + 2U), d_threshold_step_two(conf_.pfa2 > 0.0 ? compute_threshold(conf_.pfa2, d_effective_fft_size, d_num_doppler_bins_step2, conf_.bit_transition_flag ? 1 : conf_.max_dwells) : conf_.threshold), d_cshort(conf_.it_size != sizeof(gr_complex)), d_use_CFAR_algorithm_flag(conf_.use_CFAR_algorithm_flag), @@ -167,12 +168,12 @@ pcps_acquisition::pcps_acquisition(const Acq_Conf& conf_) d_dump_number(0), d_input_power(0), d_doppler_center_step_two(0), - d_magnitude_grid(std::max(d_num_doppler_bins_full_grid_active, d_num_doppler_bins_step2) * d_magnitude_grid_stride), + d_magnitude_grid(std::max(d_num_doppler_bins_capacity, d_num_doppler_bins_step2) * d_magnitude_grid_stride), d_tmp_buffer(d_effective_fft_size), d_input_signal(d_fft_size), d_grid_doppler_wipeoffs_step_two(d_acq_parameters.make_2_steps ? d_num_doppler_bins_step2 * d_doppler_wipeoffs_stride : 0), d_ifft(gnss_fft_rev_make_unique(d_fft_size)), - d_grid_doppler_wipeoffs(d_num_doppler_bins_full_grid_active * d_doppler_wipeoffs_stride), + d_grid_doppler_wipeoffs(d_num_doppler_bins_capacity * d_doppler_wipeoffs_stride), d_fft_codes(d_fft_size), d_fft_if(gnss_fft_fwd_make_unique(d_fft_size)) { @@ -231,7 +232,7 @@ pcps_acquisition::pcps_acquisition(const Acq_Conf& conf_) #if CUDA_GPU_ACCEL void pcps_acquisition::init_cuda_engine() { - const uint32_t max_bins = std::max(d_num_doppler_bins_full_grid_active, d_num_doppler_bins_step2); + const uint32_t max_bins = std::max(d_num_doppler_bins_capacity, d_num_doppler_bins_step2); auto engine = std::make_unique(d_fft_size, d_effective_fft_size, max_bins, d_acq_parameters.cuda_device); if (!engine->is_valid()) { diff --git a/src/algorithms/acquisition/gnuradio_blocks/pcps_acquisition.h b/src/algorithms/acquisition/gnuradio_blocks/pcps_acquisition.h index f30455485..a5d75eb25 100644 --- a/src/algorithms/acquisition/gnuradio_blocks/pcps_acquisition.h +++ b/src/algorithms/acquisition/gnuradio_blocks/pcps_acquisition.h @@ -316,10 +316,14 @@ private: // always > 1 candidate bin in practice) when d_full_grid_reference_needs_extra_row // is true. This (not d_num_doppler_bins) is the full grid's row count -- both // the constructed value of d_num_doppler_bins_active and what - // set_doppler_num_bins(0) (the "full range" sentinel) restores it to -- and - // what d_magnitude_grid/d_grid_doppler_wipeoffs are sized to accommodate: the - // ceiling every set_doppler_num_bins() call must stay within. + // set_doppler_num_bins(0) (the "full range" sentinel) restores it to. const uint32_t d_num_doppler_bins_full_grid_active; + // Step-1 row capacity of d_magnitude_grid/d_grid_doppler_wipeoffs (and of the + // CUDA engine). A narrowed search of N < d_num_doppler_bins candidates adds up + // to 2 reference rows, so N = d_num_doppler_bins - 1 needs d_num_doppler_bins + 1 + // rows even when the full grid itself needs none; d_num_doppler_bins + 2 covers + // every row count set_doppler_num_bins() can produce. + const uint32_t d_num_doppler_bins_capacity; const float d_threshold_step_two; const bool d_cshort; const bool d_use_CFAR_algorithm_flag; @@ -336,9 +340,8 @@ private: // Number of step-1 Doppler grid rows actually searched right now: the // candidate bin count last passed to set_doppler_num_bins() (or the full // grid's candidate count, if it was called with the 0 sentinel), plus - // d_num_reference_rows_active. Always <= d_num_doppler_bins_full_grid_active - // -- set_doppler_num_bins() clamps to that ceiling -- so it never exceeds - // what d_magnitude_grid/d_grid_doppler_wipeoffs were allocated for. + // d_num_reference_rows_active. Always <= d_num_doppler_bins_capacity, the row + // count d_magnitude_grid/d_grid_doppler_wipeoffs are allocated for. uint32_t d_num_doppler_bins_active; // How many of d_num_doppler_bins_active's trailing rows are dedicated // noise-only reference rows (see needs_extra_reference_row()) rather than diff --git a/tests/unit-tests/signal-processing-blocks/acquisition/pcps_acquisition_doppler_narrowing_test.cc b/tests/unit-tests/signal-processing-blocks/acquisition/pcps_acquisition_doppler_narrowing_test.cc index dbf8dfe3f..516143aa2 100644 --- a/tests/unit-tests/signal-processing-blocks/acquisition/pcps_acquisition_doppler_narrowing_test.cc +++ b/tests/unit-tests/signal-processing-blocks/acquisition/pcps_acquisition_doppler_narrowing_test.cc @@ -692,3 +692,16 @@ TEST_F(PcpsAcquisitionDopplerNarrowingTest /*unused*/, FullGridReferenceRowSelec ASSERT_EQ(1, result.rx_message) << "Acquisition failure with the true signal at the outermost candidate -- opposite-sign reference selection should not be interfering."; EXPECT_LE(result.doppler_error_hz, 666) << "Doppler error exceeds the expected value: 666 Hz = 2/(3*integration period)"; } + + +TEST_F(PcpsAcquisitionDopplerNarrowingTest /*unused*/, NumBinsJustBelowFullGridStaysInBounds /*unused*/) +{ + // doppler_max = 5000, doppler_step = 1000 -> d_num_doppler_bins = 10. With the + // peak-ratio statistic the full grid needs no reference rows, but a narrowed + // search of N = 9 candidates adds 2 (at +/-doppler_max), i.e. 11 rows: one + // more than the full grid. The grid buffers must be sized for that. + init(/*doppler_max=*/5000, /*doppler_step=*/1000, /*use_cfar=*/false); + const RunResult result = run_acquisition(top_block, config.get(), gnss_synchro, 0, 9); + ASSERT_EQ(1, result.rx_message) << "Acquisition failure with N = d_num_doppler_bins - 1."; + EXPECT_LE(result.doppler_error_hz, 1000); +}