drm/i915: avoid potential uninitialized variable use
diff mbox

Message ID 20171005120835.437022-1-arnd@arndb.de
State New
Headers show

Commit Message

Arnd Bergmann Oct. 5, 2017, 12:08 p.m. UTC
One of the recent changes introduced a warning about
undefined behavior in the sanity checking:

drivers/gpu/drm/i915/intel_ddi.c: In function 'intel_ddi_hdmi_level':
drivers/gpu/drm/i915/intel_ddi.c:654:6: error: 'n_hdmi_entries' may be used uninitialized in this function [-Werror=maybe-uninitialized]

It seems that the new cnl specific get_buf_trans functions
can return uninitialized data if the voltage level is set
to an unexpected value. This changes the code to always return
'1' in that error case, which seems like the safest choice
as we use one less than the number as an array index later on.

Fixes: cc9cabfdec38 ("drm/i915/cnl: Move voltage check into ddi buf trans functions.")
Signed-off-by: Arnd Bergmann <arnd@arndb.de>
---
 drivers/gpu/drm/i915/intel_ddi.c | 12 +++++++++---
 1 file changed, 9 insertions(+), 3 deletions(-)

Comments

Daniel Vetter Oct. 5, 2017, 1:32 p.m. UTC | #1
On Thu, Oct 05, 2017 at 02:08:26PM +0200, Arnd Bergmann wrote:
> One of the recent changes introduced a warning about
> undefined behavior in the sanity checking:
> 
> drivers/gpu/drm/i915/intel_ddi.c: In function 'intel_ddi_hdmi_level':
> drivers/gpu/drm/i915/intel_ddi.c:654:6: error: 'n_hdmi_entries' may be used uninitialized in this function [-Werror=maybe-uninitialized]
> 
> It seems that the new cnl specific get_buf_trans functions
> can return uninitialized data if the voltage level is set
> to an unexpected value. This changes the code to always return
> '1' in that error case, which seems like the safest choice
> as we use one less than the number as an array index later on.
> 
> Fixes: cc9cabfdec38 ("drm/i915/cnl: Move voltage check into ddi buf trans functions.")
> Signed-off-by: Arnd Bergmann <arnd@arndb.de>
> ---
>  drivers/gpu/drm/i915/intel_ddi.c | 12 +++++++++---
>  1 file changed, 9 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_ddi.c b/drivers/gpu/drm/i915/intel_ddi.c
> index 93cbbcbbc193..d0b786078bea 100644
> --- a/drivers/gpu/drm/i915/intel_ddi.c
> +++ b/drivers/gpu/drm/i915/intel_ddi.c
> @@ -602,8 +602,10 @@ cnl_get_buf_trans_hdmi(struct drm_i915_private *dev_priv, int *n_entries)
>  	} else if (voltage == VOLTAGE_INFO_1_05V) {
>  		*n_entries = ARRAY_SIZE(cnl_ddi_translations_hdmi_1_05V);
>  		return cnl_ddi_translations_hdmi_1_05V;
> -	} else
> +	} else {
> +		*n_entries = 1;
>  		MISSING_CASE(voltage);
> +	}

Somewhat meh on this, so added a /* shut up gcc */ comment and merged.

Thanks, Daniel

>  	return NULL;
>  }
>  
> @@ -621,8 +623,10 @@ cnl_get_buf_trans_dp(struct drm_i915_private *dev_priv, int *n_entries)
>  	} else if (voltage == VOLTAGE_INFO_1_05V) {
>  		*n_entries = ARRAY_SIZE(cnl_ddi_translations_dp_1_05V);
>  		return cnl_ddi_translations_dp_1_05V;
> -	} else
> +	} else {
> +		*n_entries = 1;
>  		MISSING_CASE(voltage);
> +	}
>  	return NULL;
>  }
>  
> @@ -641,8 +645,10 @@ cnl_get_buf_trans_edp(struct drm_i915_private *dev_priv, int *n_entries)
>  		} else if (voltage == VOLTAGE_INFO_1_05V) {
>  			*n_entries = ARRAY_SIZE(cnl_ddi_translations_edp_1_05V);
>  			return cnl_ddi_translations_edp_1_05V;
> -		} else
> +		} else {
> +			*n_entries = 1;
>  			MISSING_CASE(voltage);
> +		}
>  		return NULL;
>  	} else {
>  		return cnl_get_buf_trans_dp(dev_priv, n_entries);
> -- 
> 2.9.0
>

Patch
diff mbox

diff --git a/drivers/gpu/drm/i915/intel_ddi.c b/drivers/gpu/drm/i915/intel_ddi.c
index 93cbbcbbc193..d0b786078bea 100644
--- a/drivers/gpu/drm/i915/intel_ddi.c
+++ b/drivers/gpu/drm/i915/intel_ddi.c
@@ -602,8 +602,10 @@  cnl_get_buf_trans_hdmi(struct drm_i915_private *dev_priv, int *n_entries)
 	} else if (voltage == VOLTAGE_INFO_1_05V) {
 		*n_entries = ARRAY_SIZE(cnl_ddi_translations_hdmi_1_05V);
 		return cnl_ddi_translations_hdmi_1_05V;
-	} else
+	} else {
+		*n_entries = 1;
 		MISSING_CASE(voltage);
+	}
 	return NULL;
 }
 
@@ -621,8 +623,10 @@  cnl_get_buf_trans_dp(struct drm_i915_private *dev_priv, int *n_entries)
 	} else if (voltage == VOLTAGE_INFO_1_05V) {
 		*n_entries = ARRAY_SIZE(cnl_ddi_translations_dp_1_05V);
 		return cnl_ddi_translations_dp_1_05V;
-	} else
+	} else {
+		*n_entries = 1;
 		MISSING_CASE(voltage);
+	}
 	return NULL;
 }
 
@@ -641,8 +645,10 @@  cnl_get_buf_trans_edp(struct drm_i915_private *dev_priv, int *n_entries)
 		} else if (voltage == VOLTAGE_INFO_1_05V) {
 			*n_entries = ARRAY_SIZE(cnl_ddi_translations_edp_1_05V);
 			return cnl_ddi_translations_edp_1_05V;
-		} else
+		} else {
+			*n_entries = 1;
 			MISSING_CASE(voltage);
+		}
 		return NULL;
 	} else {
 		return cnl_get_buf_trans_dp(dev_priv, n_entries);