mirror of
https://github.com/opencv/opencv.git
synced 2026-01-18 17:21:42 +01:00
Merge pull request #28259 from falloficarus22:fix/bilateral-filter-oob
Fix the out-of-bounds read in cv::bilateralFilter for 32f images #28259 ### Root Cause Analysis The issue was caused by a discrepancy between the image range used to allocate the color weight look-up table (LUT) and the actual range of pixel values encountered during filtering, especially near the image borders. - Range Computation: `cv::bilateralFilter` computes the min/max values of the source image and allocates a `LUT (expLUT)` of size `kExpNumBins + 2` based on this range. - Border Padding: If `cv::BORDER_CONSTANT` is used (defaulting to 0), and 0 is outside the image's original range (e.g., an image with values between 100 and 200), the padded image will contain values (0) that create differences larger than those accounted for in the `LUT`. - Out-of-Bounds Access: When calculating the color weight, the code computes an index `idx` from the absolute difference. If this difference exceeds the expected range, `idx` can reach or exceed `kExpNumBins + 1`. Since the code performs linear interpolation using `expLUT[idx]` and `expLUT[idx + 1]`, an `idx` of `kExpNumBins + 1` causes an access to `expLUT[kExpNumBins + 2]`, which is out of bounds. ### Fix I implemented a robust clamping mechanism in both the SIMD (AVX/SSE) and scalar paths of the bilateral filter invoker: - Signature Update: Updated `bilateralFilterInvoker_32f` to accept `kExpNumBins` (the maximum valid `LUT` index). - Clamping: Clamped the computed color difference (alpha) to `kExpNumBins` before calculating the `LUT` index. This ensures that any difference exceeding the planned range is safely treated as the maximum difference in the `LUT` (which usually corresponds to a weight of 0), avoiding any out-of-bounds memory access. ### Modified Files Modified Files: `modules/imgproc/src/bilateral_filter.simd.hpp`: Updated the invoker class and SIMD/scalar loops to clamp the LUT index. `modules/imgproc/src/bilateral_filter.dispatch.cpp`: Updated the dispatch call site to pass the correct LUT size. Closes #28254 ### Pull Request Readiness Checklist - [x] I agree to contribute to the project under Apache 2 License. - [x] To the best of my knowledge, the proposed patch is not based on a code under GPL or another license that is incompatible with OpenCV - [x] The PR is proposed to the proper branch - [x] There is a reference to the original bug report and related work - [x] There is accuracy test, performance test and test data in opencv_extra repository, if applicable Patch to opencv_extra has the same branch name. - [x] The feature is well documented and sample code can be built with the project CMake
This commit is contained in:
@@ -321,7 +321,7 @@ bilateralFilter_32f( const Mat& src, Mat& dst, int d,
|
||||
}
|
||||
|
||||
// parallel_for usage
|
||||
CV_CPU_DISPATCH(bilateralFilterInvoker_32f, (cn, radius, maxk, space_ofs, temp, dst, scale_index, space_weight, expLUT),
|
||||
CV_CPU_DISPATCH(bilateralFilterInvoker_32f, (cn, radius, maxk, space_ofs, temp, dst, scale_index, space_weight, expLUT, kExpNumBins),
|
||||
CV_CPU_DISPATCH_MODES_ALL);
|
||||
}
|
||||
|
||||
|
||||
@@ -58,7 +58,7 @@ void bilateralFilterInvoker_8u(
|
||||
int* space_ofs, float *space_weight, float *color_weight);
|
||||
void bilateralFilterInvoker_32f(
|
||||
int cn, int radius, int maxk, int *space_ofs,
|
||||
const Mat& temp, Mat& dst, float scale_index, float *space_weight, float *expLUT);
|
||||
const Mat& temp, Mat& dst, float scale_index, float *space_weight, float *expLUT, int kExpNumBins);
|
||||
|
||||
#ifndef CV_CPU_OPTIMIZATION_DECLARATIONS_ONLY
|
||||
|
||||
@@ -495,9 +495,9 @@ class BilateralFilter_32f_Invoker :
|
||||
public:
|
||||
|
||||
BilateralFilter_32f_Invoker(int _cn, int _radius, int _maxk, int *_space_ofs,
|
||||
const Mat& _temp, Mat& _dest, float _scale_index, float *_space_weight, float *_expLUT) :
|
||||
const Mat& _temp, Mat& _dest, float _scale_index, float *_space_weight, float *_expLUT, int _kExpNumBins) :
|
||||
cn(_cn), radius(_radius), maxk(_maxk), space_ofs(_space_ofs),
|
||||
temp(&_temp), dest(&_dest), scale_index(_scale_index), space_weight(_space_weight), expLUT(_expLUT)
|
||||
temp(&_temp), dest(&_dest), scale_index(_scale_index), space_weight(_space_weight), expLUT(_expLUT), kExpNumBins(_kExpNumBins)
|
||||
{
|
||||
}
|
||||
|
||||
@@ -550,7 +550,7 @@ public:
|
||||
v_float32 val0 = vx_load(ksptr);
|
||||
v_float32 knan0 = v_not_nan(val0);
|
||||
v_float32 alpha0 = v_and(v_and(v_mul(v_absdiff(val0, rval0), sindex), v_not_nan(rval0)), knan0);
|
||||
v_int32 idx0 = v_trunc(alpha0);
|
||||
v_int32 idx0 = v_min(v_trunc(alpha0), vx_setall_s32(kExpNumBins));
|
||||
alpha0 = v_sub(alpha0, v_cvt_f32(idx0));
|
||||
v_float32 w0 = v_and(v_mul(kweight, v_muladd(v_lut(this->expLUT + 1, idx0), alpha0, v_mul(v_lut(this->expLUT, idx0), v_sub(v_one, alpha0)))), knan0);
|
||||
v_wsum0 = v_add(v_wsum0, w0);
|
||||
@@ -560,7 +560,7 @@ public:
|
||||
v_float32 val1 = vx_load(ksptr + nlanes);
|
||||
v_float32 knan1 = v_not_nan(val1);
|
||||
v_float32 alpha1 = v_and(v_and(v_mul(v_absdiff(val1, rval1), sindex), v_not_nan(rval1)), knan1);
|
||||
v_int32 idx1 = v_trunc(alpha1);
|
||||
v_int32 idx1 = v_min(v_trunc(alpha1), vx_setall_s32(kExpNumBins));
|
||||
alpha1 = v_sub(alpha1, v_cvt_f32(idx1));
|
||||
v_float32 w1 = v_and(v_mul(kweight, v_muladd(v_lut(this->expLUT + 1, idx1), alpha1, v_mul(v_lut(this->expLUT, idx1), v_sub(v_one, alpha1)))), knan1);
|
||||
v_wsum1 = v_add(v_wsum1, w1);
|
||||
@@ -570,7 +570,7 @@ public:
|
||||
v_float32 val2 = vx_load(ksptr + nlanes_2);
|
||||
v_float32 knan2 = v_not_nan(val2);
|
||||
v_float32 alpha2 = v_and(v_and(v_mul(v_absdiff(val2, rval2), sindex), v_not_nan(rval2)), knan2);
|
||||
v_int32 idx2 = v_trunc(alpha2);
|
||||
v_int32 idx2 = v_min(v_trunc(alpha2), vx_setall_s32(kExpNumBins));
|
||||
alpha2 = v_sub(alpha2, v_cvt_f32(idx2));
|
||||
v_float32 w2 = v_and(v_mul(kweight, v_muladd(v_lut(this->expLUT + 1, idx2), alpha2, v_mul(v_lut(this->expLUT, idx2), v_sub(v_one, alpha2)))), knan2);
|
||||
v_wsum2 = v_add(v_wsum2, w2);
|
||||
@@ -580,7 +580,7 @@ public:
|
||||
v_float32 val3 = vx_load(ksptr + nlanes_3);
|
||||
v_float32 knan3 = v_not_nan(val3);
|
||||
v_float32 alpha3 = v_and(v_and(v_mul(v_absdiff(val3, rval3), sindex), v_not_nan(rval3)), knan3);
|
||||
v_int32 idx3 = v_trunc(alpha3);
|
||||
v_int32 idx3 = v_min(v_trunc(alpha3), vx_setall_s32(kExpNumBins));
|
||||
alpha3 = v_sub(alpha3, v_cvt_f32(idx3));
|
||||
v_float32 w3 = v_and(v_mul(kweight, v_muladd(v_lut(this->expLUT + 1, idx3), alpha3, v_mul(v_lut(this->expLUT, idx3), v_sub(v_one, alpha3)))), knan3);
|
||||
v_wsum3 = v_add(v_wsum3, w3);
|
||||
@@ -611,7 +611,7 @@ public:
|
||||
v_float32 val0 = vx_load(ksptr);
|
||||
v_float32 knan0 = v_not_nan(val0);
|
||||
v_float32 alpha0 = v_and(v_and(v_mul(v_absdiff(val0, rval0), sindex), rval0_not_nan), knan0);
|
||||
v_int32 idx0 = v_trunc(alpha0);
|
||||
v_int32 idx0 = v_min(v_trunc(alpha0), vx_setall_s32(kExpNumBins));
|
||||
alpha0 = v_sub(alpha0, v_cvt_f32(idx0));
|
||||
v_float32 w0 = v_and(v_mul(kweight, v_muladd(v_lut(this->expLUT + 1, idx0), alpha0, v_mul(v_lut(this->expLUT, idx0), v_sub(v_one, alpha0)))), knan0);
|
||||
v_wsum0 = v_add(v_wsum0, w0);
|
||||
@@ -621,7 +621,7 @@ public:
|
||||
v_float32 val1 = vx_load(ksptr + nlanes);
|
||||
v_float32 knan1 = v_not_nan(val1);
|
||||
v_float32 alpha1 = v_and(v_and(v_mul(v_absdiff(val1, rval1), sindex), rval1_not_nan), knan1);
|
||||
v_int32 idx1 = v_trunc(alpha1);
|
||||
v_int32 idx1 = v_min(v_trunc(alpha1), vx_setall_s32(kExpNumBins));
|
||||
alpha1 = v_sub(alpha1, v_cvt_f32(idx1));
|
||||
v_float32 w1 = v_and(v_mul(kweight, v_muladd(v_lut(this->expLUT + 1, idx1), alpha1, v_mul(v_lut(this->expLUT, idx1), v_sub(v_one, alpha1)))), knan1);
|
||||
v_wsum1 = v_add(v_wsum1, w1);
|
||||
@@ -640,7 +640,7 @@ public:
|
||||
{
|
||||
const float* ksptr = sptr_j + space_ofs[k];
|
||||
float val = *ksptr;
|
||||
float alpha = std::abs(val - rval) * scale_index;
|
||||
float alpha = std::min(std::abs(val - rval) * scale_index, (float)kExpNumBins);
|
||||
int idx = cvFloor(alpha);
|
||||
alpha -= idx;
|
||||
if (!cvIsNaN(val))
|
||||
@@ -681,7 +681,7 @@ public:
|
||||
|
||||
v_float32 knan = v_and(v_and(v_not_nan(kb), v_not_nan(kg)), v_not_nan(kr));
|
||||
v_float32 alpha = v_and(v_and(v_and(v_and(v_mul(v_add(v_add(v_absdiff(kb, rb), v_absdiff(kg, rg)), v_absdiff(kr, rr)), sindex), v_not_nan(rb)), v_not_nan(rg)), v_not_nan(rr)), knan);
|
||||
v_int32 idx = v_trunc(alpha);
|
||||
v_int32 idx = v_min(v_trunc(alpha), vx_setall_s32(kExpNumBins));
|
||||
alpha = v_sub(alpha, v_cvt_f32(idx));
|
||||
|
||||
v_float32 w = v_and(v_mul(kweight, v_muladd(v_lut(this->expLUT + 1, idx), alpha, v_mul(v_lut(this->expLUT, idx), v_sub(v_one, alpha)))), knan);
|
||||
@@ -709,7 +709,7 @@ public:
|
||||
bool v_NAN = cvIsNaN(b) || cvIsNaN(g) || cvIsNaN(r);
|
||||
float rb = rsptr[0], rg = rsptr[1], rr = rsptr[2];
|
||||
bool r_NAN = cvIsNaN(rb) || cvIsNaN(rg) || cvIsNaN(rr);
|
||||
float alpha = (std::abs(b - rb) + std::abs(g - rg) + std::abs(r - rr)) * scale_index;
|
||||
float alpha = std::min((std::abs(b - rb) + std::abs(g - rg) + std::abs(r - rr)) * scale_index, (float)kExpNumBins);
|
||||
int idx = cvFloor(alpha);
|
||||
alpha -= idx;
|
||||
if (!v_NAN)
|
||||
@@ -753,17 +753,18 @@ private:
|
||||
const Mat* temp;
|
||||
Mat *dest;
|
||||
float scale_index, *space_weight, *expLUT;
|
||||
int kExpNumBins;
|
||||
};
|
||||
|
||||
} // namespace anon
|
||||
|
||||
void bilateralFilterInvoker_32f(
|
||||
int cn, int radius, int maxk, int *space_ofs,
|
||||
const Mat& temp, Mat& dst, float scale_index, float *space_weight, float *expLUT)
|
||||
const Mat& temp, Mat& dst, float scale_index, float *space_weight, float *expLUT, int kExpNumBins)
|
||||
{
|
||||
CV_INSTRUMENT_REGION();
|
||||
|
||||
BilateralFilter_32f_Invoker body(cn, radius, maxk, space_ofs, temp, dst, scale_index, space_weight, expLUT);
|
||||
BilateralFilter_32f_Invoker body(cn, radius, maxk, space_ofs, temp, dst, scale_index, space_weight, expLUT, kExpNumBins);
|
||||
parallel_for_(Range(0, dst.rows), body, dst.total()/(double)(1<<16));
|
||||
}
|
||||
|
||||
|
||||
@@ -291,4 +291,33 @@ namespace opencv_test { namespace {
|
||||
test.safe_run();
|
||||
}
|
||||
|
||||
// Regression test for issue #28254
|
||||
// Out-of-bounds read in AVX2 bilateralFilter 32f path with BORDER_CONSTANT
|
||||
TEST(Imgproc_BilateralFilter, regression_28254_oob_read)
|
||||
{
|
||||
// Create a 64x64 CV_32FC1 image with values in range [100, 200]
|
||||
// Image must be large enough (width >= 32) to trigger SIMD/AVX2 code path.
|
||||
// Values are set so BORDER_CONSTANT padding (default 0) is outside the range,
|
||||
// which triggers the out-of-bounds condition in the LUT access.
|
||||
cv::Mat src(64, 64, CV_32FC1);
|
||||
cv::randu(src, 100.0f, 200.0f);
|
||||
cv::Mat dst;
|
||||
|
||||
// Parameters that trigger the bug
|
||||
int d = -1;
|
||||
double sigmaColor = 2.7;
|
||||
double sigmaSpace = 44.5;
|
||||
int borderType = cv::BORDER_CONSTANT;
|
||||
|
||||
// This should not crash or trigger AddressSanitizer
|
||||
EXPECT_NO_THROW(
|
||||
cv::bilateralFilter(src, dst, d, sigmaColor, sigmaSpace, borderType)
|
||||
);
|
||||
|
||||
// Verify output is valid
|
||||
EXPECT_FALSE(dst.empty());
|
||||
EXPECT_EQ(dst.size(), src.size());
|
||||
EXPECT_EQ(dst.type(), src.type());
|
||||
}
|
||||
|
||||
}} // namespace
|
||||
|
||||
Reference in New Issue
Block a user