From mboxrd@z Thu Jan 1 00:00:00 1970 From: Dan Carpenter Subject: Re: drm/i915/bdw: Provide the BDW specific HDMI buffer translation table Date: Wed, 27 Aug 2014 16:04:46 +0300 Message-ID: <20140827130446.GA17306@mwanda> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: Content-Disposition: inline List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: intel-gfx-bounces@lists.freedesktop.org Sender: "Intel-gfx" To: damien.lespiau@intel.com Cc: intel-gfx@lists.freedesktop.org, dri-devel@lists.freedesktop.org List-Id: intel-gfx@lists.freedesktop.org Hello Damien Lespiau, The patch a26aa8baee6c: "drm/i915/bdw: Provide the BDW specific HDMI buffer translation table" from Aug 1, 2014, leads to the following static checker warning: drivers/gpu/drm/i915/intel_ddi.c:225 intel_prepare_ddi_buffers() error: buffer overflow 'ddi_translations_hdmi' 24 <= 31 drivers/gpu/drm/i915/intel_ddi.c 155 static void intel_prepare_ddi_buffers(struct drm_device *dev, enum port port) 156 { 157 struct drm_i915_private *dev_priv = dev->dev_private; 158 u32 reg; 159 int i, n_hdmi_entries, hdmi_800mV_0dB; 160 int hdmi_level = dev_priv->vbt.ddi_port_info[port].hdmi_level_shift; ^^^^^^^^^^ The static checker thinks this is a number between 0-15. I didn't check if that was correct. 161 const u32 *ddi_translations_fdi; [ snip ] 186 ddi_translations_hdmi = bdw_ddi_translations_hdmi; 187 n_hdmi_entries = ARRAY_SIZE(bdw_ddi_translations_hdmi); ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ limit is the size of the array. 188 hdmi_800mV_0dB = 7; 189 } 190 191 switch (port) { 192 case PORT_A: 193 ddi_translations = ddi_translations_edp; 194 break; 195 case PORT_B: 196 case PORT_C: 197 ddi_translations = ddi_translations_dp; 198 break; 199 case PORT_D: 200 if (intel_dp_is_edp(dev, PORT_D)) 201 ddi_translations = ddi_translations_edp; 202 else 203 ddi_translations = ddi_translations_dp; 204 break; 205 case PORT_E: 206 ddi_translations = ddi_translations_fdi; 207 break; 208 default: 209 BUG(); 210 } 211 212 for (i = 0, reg = DDI_BUF_TRANS(port); 213 i < ARRAY_SIZE(hsw_ddi_translations_fdi); i++) { 214 I915_WRITE(reg, ddi_translations[i]); 215 reg += 4; 216 } 217 218 /* Choose a good default if VBT is badly populated */ 219 if (hdmi_level == HDMI_LEVEL_SHIFT_UNKNOWN || 220 hdmi_level >= n_hdmi_entries) ^^^^^^^^^^^^^^ We cap it at the size of the array which is either 20 or 24. The static checker thinks it's already capped at 15 so this is not a problem. Should this be n_hdmi_entries / 2 or something? Maybe a - 1 in there? 221 hdmi_level = hdmi_800mV_0dB; 222 223 /* Entry 9 is for HDMI: */ 224 for (i = 0; i < 2; i++) { 225 I915_WRITE(reg, ddi_translations_hdmi[hdmi_level * 2 + i]); ^^^^^^^^^^^^^^^^^^ The static checker thinks "15 * 2 + 1 = 31" so we are beyond the size of the largest array (either 20 or 24). 226 reg += 4; 227 } 228 } regards, dan carpenter