From patchwork Thu Jan 15 11:56:58 2015 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Tomi Valkeinen X-Patchwork-Id: 5638841 Return-Path: X-Original-To: patchwork-linux-fbdev@patchwork.kernel.org Delivered-To: patchwork-parsemail@patchwork2.web.kernel.org Received: from mail.kernel.org (mail.kernel.org [198.145.29.136]) by patchwork2.web.kernel.org (Postfix) with ESMTP id AEF89C058E for ; Thu, 15 Jan 2015 11:57:50 +0000 (UTC) Received: from mail.kernel.org (localhost [127.0.0.1]) by mail.kernel.org (Postfix) with ESMTP id F164B20158 for ; Thu, 15 Jan 2015 11:57:49 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.kernel.org (Postfix) with ESMTP id 827FB2016C for ; Thu, 15 Jan 2015 11:57:44 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1753133AbbAOL5l (ORCPT ); Thu, 15 Jan 2015 06:57:41 -0500 Received: from devils.ext.ti.com ([198.47.26.153]:60702 "EHLO devils.ext.ti.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751502AbbAOL5j (ORCPT ); Thu, 15 Jan 2015 06:57:39 -0500 Received: from dlelxv90.itg.ti.com ([172.17.2.17]) by devils.ext.ti.com (8.13.7/8.13.7) with ESMTP id t0FBv375011612; Thu, 15 Jan 2015 05:57:03 -0600 Received: from DLEE70.ent.ti.com (dlemailx.itg.ti.com [157.170.170.113]) by dlelxv90.itg.ti.com (8.14.3/8.13.8) with ESMTP id t0FBv3kW009102; Thu, 15 Jan 2015 05:57:03 -0600 Received: from dlep33.itg.ti.com (157.170.170.75) by DLEE70.ent.ti.com (157.170.170.113) with Microsoft SMTP Server id 14.3.174.1; Thu, 15 Jan 2015 05:57:02 -0600 Received: from [172.22.232.90] (ileax41-snat.itg.ti.com [10.172.224.153]) by dlep33.itg.ti.com (8.14.3/8.13.8) with ESMTP id t0FBux0p023605; Thu, 15 Jan 2015 05:56:59 -0600 Message-ID: <54B7AB0A.3020708@ti.com> Date: Thu, 15 Jan 2015 13:56:58 +0200 From: Tomi Valkeinen User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:31.0) Gecko/20100101 Thunderbird/31.3.0 MIME-Version: 1.0 To: Tomi Valkeinen , David Ung , "'Jingoo Han'" , "'Dan Carpenter'" CC: "'Jean-Christophe Plagniol-Villard'" , "'Daniel Vetter'" , "'Laurent Pinchart'" , "'Rob Clark'" , "linux-fbdev@vger.kernel.org" , "kernel-janitors@vger.kernel.org" Subject: Re: [patch] fbdev: off by one test (harmless) References: <20141226172657.GA14762@mwanda> <000101d02174$8bcee760$a36cb620$%han@samsung.com> <410219d77f8c4d0982cc9e99d0364acb@DRHQMAIL102.nvidia.com> <54B4FF35.8050703@ti.com> In-Reply-To: <54B4FF35.8050703@ti.com> Sender: linux-fbdev-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-fbdev@vger.kernel.org X-Spam-Status: No, score=-6.9 required=5.0 tests=BAYES_00, RCVD_IN_DNSWL_HI, T_RP_MATCHES_RCVD, T_TVD_MIME_EPI, UNPARSEABLE_RELAY autolearn=unavailable version=3.3.1 X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on mail.kernel.org X-Virus-Scanned: ClamAV using ClamSMTP On 13/01/15 13:19, Tomi Valkeinen wrote: > On 29/12/14 23:51, David Ung wrote: >> >>>> test is really a no-op. But it's still off by one and upsets the >>>> static checkers so we may as well correct it. >>> >>> 'idx' should be 0~63, because cea_modes array is defined as >>> 'cea_modes[64]'. >> >> According to CEA/EIA-861E, there are 64 defined modes, but idx is valid for 1-64, 0 is reserved hence the check for >> >> If (!idx || idx > 63) { >> >> Think idx check really should be !idx || idx > 64 if following CEA/EIA-861E > > In that case there's something funny with the code. The code indexes > 'cea_modes' using 'idx', and I _think_ cea_modes is already offset > properly, i.e. there's no entry at 0. But its length is 64, which is not > right, as there's the empty item in the beginning. > > So maybe the correct fix is to increase the length of cea_modes to 65, > and change the idx check as you mention above? Here's a fix for the cea_array size. Does it silence the static checker? Tomi From ec1edd4683ca3cc0b3b6e5feffae861e404d16d2 Mon Sep 17 00:00:00 2001 From: Tomi Valkeinen Date: Thu, 15 Jan 2015 13:47:19 +0200 Subject: [PATCH] fbdev: fix cea_modes array size CEA defines 64 modes, indexed from 1 to 64. modedb has cea_modes arrays, which contains 64 entries. However, the code uses the CEA indices directly, i.e. the first mode is at cea_modes[1]. This means the array is one too short. This does not cause references to uninitialized memory as the code in fbmon only allows indexes up to 63, and the cea_modes does not contain an entry for the mode 64 so it could not be used in any case. However, the code contains a check 'if (idx > ARRAY_SIZE(cea_modes)', and while that check is a no-op as at that point idx cannot be >= 63, it upsets static checkers. Fix this by increasing the cea_array size to be 65, and change the code to allow mode 64. Signed-off-by: Tomi Valkeinen --- drivers/video/fbdev/core/fbmon.c | 2 +- drivers/video/fbdev/core/modedb.c | 2 +- include/linux/fb.h | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/drivers/video/fbdev/core/fbmon.c b/drivers/video/fbdev/core/fbmon.c index 5b0e313849bd..0fea923c3cb6 100644 --- a/drivers/video/fbdev/core/fbmon.c +++ b/drivers/video/fbdev/core/fbmon.c @@ -1059,7 +1059,7 @@ void fb_edid_add_monspecs(unsigned char *edid, struct fb_monspecs *specs) for (i = specs->modedb_len + num; i < specs->modedb_len + num + svd_n; i++) { int idx = svd[i - specs->modedb_len - num]; - if (!idx || idx > 63) { + if (!idx || idx > 64) { pr_warning("Reserved SVD code %d\n", idx); } else if (idx > ARRAY_SIZE(cea_modes) || !cea_modes[idx].xres) { pr_warning("Unimplemented SVD code %d\n", idx); diff --git a/drivers/video/fbdev/core/modedb.c b/drivers/video/fbdev/core/modedb.c index 388f7971494b..b2aba4e3150a 100644 --- a/drivers/video/fbdev/core/modedb.c +++ b/drivers/video/fbdev/core/modedb.c @@ -289,7 +289,7 @@ static const struct fb_videomode modedb[] = { }; #ifdef CONFIG_FB_MODE_HELPERS -const struct fb_videomode cea_modes[64] = { +const struct fb_videomode cea_modes[65] = { /* #1: 640x480p@59.94/60Hz */ [1] = { NULL, 60, 640, 480, 39722, 48, 16, 33, 10, 96, 2, 0, diff --git a/include/linux/fb.h b/include/linux/fb.h index 09bb7a18d287..024260687698 100644 --- a/include/linux/fb.h +++ b/include/linux/fb.h @@ -779,7 +779,7 @@ struct fb_videomode { extern const char *fb_mode_option; extern const struct fb_videomode vesa_modes[]; -extern const struct fb_videomode cea_modes[64]; +extern const struct fb_videomode cea_modes[65]; struct fb_modelist { struct list_head list;