ipa: rpi: Prevent segfault if AGC algorithm is absent
diff mbox series

Message ID 20250523-fix_segfault_agc-v1-1-aac60d87fd5c@foss.st.com
State New
Headers show
Series
  • ipa: rpi: Prevent segfault if AGC algorithm is absent
Related show

Commit Message

Benjamin Mugnier May 23, 2025, 9:56 a.m. UTC 
Even without AGC definition in the tuning file, the application would
still dereference agc unconditionally, leading to a segmentation fault
if AGC is absent.
This is relevant for sensors already providing AGC/AEC by themselves.
Check if AGC is present prior to setting maximum exposure time.

Signed-off-by: Benjamin Mugnier <benjamin.mugnier@foss.st.com>
---
 src/ipa/rpi/common/ipa_base.cpp | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)


---
base-commit: ad5326c926831fe7a943d10fd800de43e596f171
change-id: 20250523-fix_segfault_agc-5f2735edec5f

Best regards,

Comments

Paul Elder May 23, 2025, 10:50 a.m. UTC | #1 
Quoting Benjamin Mugnier (2025-05-23 18:56:56)
> Even without AGC definition in the tuning file, the application would
> still dereference agc unconditionally, leading to a segmentation fault
> if AGC is absent.
> This is relevant for sensors already providing AGC/AEC by themselves.
> Check if AGC is present prior to setting maximum exposure time.
> 
> Signed-off-by: Benjamin Mugnier <benjamin.mugnier@foss.st.com>

Reviewed-by: Paul Elder <paul.elder@ideasonboard.com>

> ---
>  src/ipa/rpi/common/ipa_base.cpp | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/src/ipa/rpi/common/ipa_base.cpp b/src/ipa/rpi/common/ipa_base.cpp
> index e0a93daa9db229bb2829803fffc6b2f6c8f11061..e0f8b7e782f4340a069a2d7b9cf9dee3141c72df 100644
> --- a/src/ipa/rpi/common/ipa_base.cpp
> +++ b/src/ipa/rpi/common/ipa_base.cpp
> @@ -1563,7 +1563,8 @@ void IpaBase::applyFrameDurations(Duration minFrameDuration, Duration maxFrameDu
>  
>         RPiController::AgcAlgorithm *agc = dynamic_cast<RPiController::AgcAlgorithm *>(
>                 controller_.getAlgorithm("agc"));
> -       agc->setMaxExposureTime(maxExposureTime);
> +       if (agc)
> +               agc->setMaxExposureTime(maxExposureTime);
>  }
>  
>  void IpaBase::applyAGC(const struct AgcStatus *agcStatus, ControlList &ctrls)
> 
> ---
> base-commit: ad5326c926831fe7a943d10fd800de43e596f171
> change-id: 20250523-fix_segfault_agc-5f2735edec5f
> 
> Best regards,
> -- 
> Benjamin Mugnier <benjamin.mugnier@foss.st.com>
>
Barnabás Pőcze May 23, 2025, 11:10 a.m. UTC | #2 
Hi

2025. 05. 23. 11:56 keltezéssel, Benjamin Mugnier írta:
> Even without AGC definition in the tuning file, the application would
> still dereference agc unconditionally, leading to a segmentation fault
> if AGC is absent.
> This is relevant for sensors already providing AGC/AEC by themselves.
> Check if AGC is present prior to setting maximum exposure time.
> 
> Signed-off-by: Benjamin Mugnier <benjamin.mugnier@foss.st.com>

Tested-by: Barnabás Pőcze <barnabas.pocze@ideasonboard.com> # RPi4 + imx708_wide

../src/ipa/rpi/common/ipa_base.cpp:1591:25: runtime error: member access within null pointer of type 'struct AgcAlgorithm'
AddressSanitizer:DEADLYSIGNAL
=================================================================
==415==ERROR: AddressSanitizer: SEGV on unknown address 0x000000000000 (pc 0xffff6dbf44a4 bp 0xffff706faed0 sp 0xffff706fad60 T1)
==415==The signal is caused by a READ memory access.
==415==Hint: address points to the zero page.
     #0 0xffff6dbf44a4 in libcamera::ipa::RPi::IpaBase::applyFrameDurations(libcamera::utils::Duration, libcamera::utils::Duration) ../src/ipa/rpi/common/ipa_base.cpp:1591
     ...


Regards,
Barnabás Pőcze


> ---
>   src/ipa/rpi/common/ipa_base.cpp | 3 ++-
>   1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/src/ipa/rpi/common/ipa_base.cpp b/src/ipa/rpi/common/ipa_base.cpp
> index e0a93daa9db229bb2829803fffc6b2f6c8f11061..e0f8b7e782f4340a069a2d7b9cf9dee3141c72df 100644
> --- a/src/ipa/rpi/common/ipa_base.cpp
> +++ b/src/ipa/rpi/common/ipa_base.cpp
> @@ -1563,7 +1563,8 @@ void IpaBase::applyFrameDurations(Duration minFrameDuration, Duration maxFrameDu
>   
>   	RPiController::AgcAlgorithm *agc = dynamic_cast<RPiController::AgcAlgorithm *>(
>   		controller_.getAlgorithm("agc"));
> -	agc->setMaxExposureTime(maxExposureTime);
> +	if (agc)
> +		agc->setMaxExposureTime(maxExposureTime);
>   }
>   
>   void IpaBase::applyAGC(const struct AgcStatus *agcStatus, ControlList &ctrls)
> 
> ---
> base-commit: ad5326c926831fe7a943d10fd800de43e596f171
> change-id: 20250523-fix_segfault_agc-5f2735edec5f
> 
> Best regards,

Patch
diff mbox series 

diff --git a/src/ipa/rpi/common/ipa_base.cpp b/src/ipa/rpi/common/ipa_base.cpp
index e0a93daa9db229bb2829803fffc6b2f6c8f11061..e0f8b7e782f4340a069a2d7b9cf9dee3141c72df 100644
--- a/src/ipa/rpi/common/ipa_base.cpp
+++ b/src/ipa/rpi/common/ipa_base.cpp
@@ -1563,7 +1563,8 @@  void IpaBase::applyFrameDurations(Duration minFrameDuration, Duration maxFrameDu
 
 	RPiController::AgcAlgorithm *agc = dynamic_cast<RPiController::AgcAlgorithm *>(
 		controller_.getAlgorithm("agc"));
-	agc->setMaxExposureTime(maxExposureTime);
+	if (agc)
+		agc->setMaxExposureTime(maxExposureTime);
 }
 
 void IpaBase::applyAGC(const struct AgcStatus *agcStatus, ControlList &ctrls)