From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-12.2 required=3.0 tests=BAYES_00, DKIM_ADSP_CUSTOM_MED,DKIM_SIGNED,DKIM_VALID,FREEMAIL_FORGED_FROMDOMAIN, FREEMAIL_FROM,HEADER_FROM_DIFFERENT_DOMAINS,INCLUDES_CR_TRAILER, INCLUDES_PATCH,MAILING_LIST_MULTI,NICE_REPLY_A,SPF_HELO_NONE,SPF_PASS, URIBL_BLOCKED,USER_AGENT_SANE_1 autolearn=unavailable autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id A0A6EC433C1 for ; Thu, 25 Mar 2021 14:52:19 +0000 (UTC) Received: from desiato.infradead.org (desiato.infradead.org [90.155.92.199]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mail.kernel.org (Postfix) with ESMTPS id 3022D61A01 for ; Thu, 25 Mar 2021 14:52:19 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org 3022D61A01 Authentication-Results: mail.kernel.org; dmarc=fail (p=none dis=none) header.from=gmail.com Authentication-Results: mail.kernel.org; spf=none smtp.mailfrom=linux-arm-kernel-bounces+linux-arm-kernel=archiver.kernel.org@lists.infradead.org DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=lists.infradead.org; s=desiato.20200630; h=Sender:Content-Type: Content-Transfer-Encoding:List-Subscribe:List-Help:List-Post:List-Archive: List-Unsubscribe:List-Id:In-Reply-To:MIME-Version:Date:Message-ID:From: References:Cc:To:Subject:Reply-To:Content-ID:Content-Description:Resent-Date: Resent-From:Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID:List-Owner; bh=TvbAGmtaBmIs3/W02Rfy7jOlCCRRF4QWNWlsebF8wzU=; b=OxoCYA+w3ALnfb6iD5ps8mbNE hj/G4234L6Jw7fqJLkf8mKRjt/rS2oQXJE70sRZz0WiSfNhWObf8dFy+wkE8lWaxuFUahk2LXb6Io flBTfDswroC9cuQqy2d3HfCaI8vaAFrSdZp9/Q9qB4t3ChXqbZu0mwv8CS5CM9k0JB8Ci6PE6n+dd zGlrAMYMLJFQLkbAzOHn+bGRDM6BpSlDFXBL43YhAFT+CfEmgbqODhnS8gHXqoJeUXkQ4Yx1AMlSv rqscv5pPqXsaC+0TbCU+/SLyYwXszQJyT+Zla7fBk3mvd+jsQ6yk/fi284VX15hG3fOdZbyCpNuI3 EgTWxgvaQ==; Received: from localhost ([::1] helo=desiato.infradead.org) by desiato.infradead.org with esmtp (Exim 4.94 #2 (Red Hat Linux)) id 1lPRJM-001ewE-FZ; Thu, 25 Mar 2021 14:50:12 +0000 Received: from mail-qk1-x735.google.com ([2607:f8b0:4864:20::735]) by desiato.infradead.org with esmtps (Exim 4.94 #2 (Red Hat Linux)) id 1lPRJD-001eul-9U; Thu, 25 Mar 2021 14:50:05 +0000 Received: by mail-qk1-x735.google.com with SMTP id y18so1964623qky.11; Thu, 25 Mar 2021 07:50:02 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20161025; h=subject:to:cc:references:from:message-id:date:user-agent :mime-version:in-reply-to:content-language:content-transfer-encoding; bh=A01xU9scL5QHqCTcEARA+UZXhX3w6QfTLVjGvG0qhWs=; b=fjIpx38pKKpF7tuHHnx+6bHTa8GIAr+gRLGjKt2x69P7v8gjKVe3W9dYqxgP9vSwI9 QihvfqnQepbko6JQmAIRW+EQAppxHtxDBVpKo/ulTx0XeTA+TsLT2XPd5R47EHfs/vL+ RcfmGpqKXdbxR340ysSTk2TR5aaFQcoTZCnFjyd/rFWDxI9wey0GlS/SNuFSvLhs+psi 0EVXYddJbSDWFSlhrUt5I6M67vOMO/RGQOVQ0jvf2R4mqWnxzVz/163P9EVlylTqlOyg RuvSoLqCSa3d1HolOz0J7uXNfTUCd96jvr8FQ9pq7Hu06fFtmIslpU2LxY/2YEkWewFc MUFw== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:subject:to:cc:references:from:message-id:date :user-agent:mime-version:in-reply-to:content-language :content-transfer-encoding; bh=A01xU9scL5QHqCTcEARA+UZXhX3w6QfTLVjGvG0qhWs=; b=oyLkEqBHESM8EzceKtJrCLOYJFsbcdmMWTvqfvTm019Yj/IT9xS4FvKdHzlbrwlHSG i03yRkaecEjAaHUE7alOVNAzGvHga1izIqGZwtKbsDUv57OBm+fAdkaBPy1fkt8vbiMU HnUDbvXsmoGPgBOx/l2xxYuLkOLpxlufFoBFx+G+PNwverl8iPtRFhR/qQ84n7BwyLmB YVI6ETokQf9DRUTtO8gW50cysjNet3+YmuryhAw8I79vxhjmE2Gy7aKZD61MTZg+qq5x 6QJFhCjTlw22H0zR4E7zx+2wxwAuYC24JcchtmOo3Pz6PXRiHm7f0gLDNP6I4gYBvzep 15uA== X-Gm-Message-State: AOAM532V2pYYlbwuCd51uDic92qcOUiH1gjp0pXsTWHMlxUTv0bjWa33 lNDWICaACvpuZ0TTYS8HxZI= X-Google-Smtp-Source: ABdhPJx2RAnajmw8ltd8BDZCZjs+AmRvn2izOpJq6Wnqeic5eWA8Vk7CLaR0JiE3Cx4o6fmRuVGtdA== X-Received: by 2002:a05:620a:1369:: with SMTP id d9mr8566930qkl.378.1616683801356; Thu, 25 Mar 2021 07:50:01 -0700 (PDT) Received: from [192.168.0.41] (71-218-23-248.hlrn.qwest.net. [71.218.23.248]) by smtp.gmail.com with ESMTPSA id y1sm4324368qki.9.2021.03.25.07.49.57 (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Thu, 25 Mar 2021 07:50:00 -0700 (PDT) Subject: Re: [PATCH 11/11] [RFC] drm/i915/dp: fix array overflow warning To: Arnd Bergmann , Jani Nikula Cc: Linux Kernel Mailing List , Martin Sebor , Joonas Lahtinen , Rodrigo Vivi , David Airlie , Daniel Vetter , the arch/x86 maintainers , Ning Sun , Kalle Valo , Simon Kelley , James Smart , "James E.J. Bottomley" , Anders Larsen , Tejun Heo , Serge Hallyn , Imre Deak , Linux ARM , tboot-devel@lists.sourceforge.net, Intel Graphics , dri-devel , ath11k@lists.infradead.org, linux-wireless , Networking , linux-scsi , Cgroups , LSM List , =?UTF-8?B?VmlsbGUgU3lyasOkbMOk?= , Manasi Navare , Uma Shankar , Ankit Nautiyal , Gwan-gyeong Mun , Animesh Manna , Sean Paul References: <20210322160253.4032422-1-arnd@kernel.org> <20210322160253.4032422-12-arnd@kernel.org> <87wntv3bgt.fsf@intel.com> From: Martin Sebor Message-ID: <44ad545d-cc07-2e5f-9ec8-ad848f39268a@gmail.com> Date: Thu, 25 Mar 2021 08:49:56 -0600 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:68.0) Gecko/20100101 Thunderbird/68.2.2 MIME-Version: 1.0 In-Reply-To: Content-Language: en-US X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20210325_145003_455688_BF3259BD X-CRM114-Status: GOOD ( 40.03 ) X-BeenThere: linux-arm-kernel@lists.infradead.org X-Mailman-Version: 2.1.34 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Content-Transfer-Encoding: 7bit Content-Type: text/plain; charset="us-ascii"; Format="flowed" Sender: "linux-arm-kernel" Errors-To: linux-arm-kernel-bounces+linux-arm-kernel=archiver.kernel.org@lists.infradead.org On 3/25/21 3:53 AM, Arnd Bergmann wrote: > On Thu, Mar 25, 2021 at 9:05 AM Jani Nikula wrote: >>> Clearly something is wrong here, but I can't quite figure out what. >>> Changing the array size to 16 bytes avoids the warning, but is >>> probably the wrong solution here. >> >> Ugh. drm_dp_channel_eq_ok() does not actually require more than >> DP_LINK_STATUS_SIZE - 2 elements in the link_status. It's some other >> related functions that do, and in most cases it's convenient to read all >> those DP_LINK_STATUS_SIZE bytes. >> >> However, here the case is slightly different for DP MST, and the change >> causes reserved DPCD addresses to be read. Not sure it matters, but >> really I think the problem is what drm_dp_channel_eq_ok() advertizes. >> >> I also don't like the array notation with sizes in function parameters >> in general, because I think it's misleading. Would gcc-11 warn if a >> function actually accesses the memory out of bounds of the size? > > Yes, that is the point of the warning. Using an explicit length in an > array argument type tells gcc that the function will never access > beyond the end of that bound, and that passing a short array > is a bug. > > I don't know if this /only/ means triggering a warning, or if gcc > is also able to make optimizations after classifying this as undefined > behavior that it would not make for an unspecified length. GCC uses the array parameter notation as a hint for warnings but it doesn't optimize on this basis and never will be able to because code that accesses more elements from the array isn't invalid. Adding static to the bound, as in void f (int[static N]) does imply that the function won't access more than N elements and C intends for optimizers to rely on it, although GCC doesn't yet. The warning for the array notation is a more portable alternative to explicitly annotating functions with attribute access, and to -Wvla-parameter for VLA parameters. The latter seem to be used relatively rarely, sometimes deliberately because of the bad rap of VLA objects, even though VLA parameters don't suffer from the same problems. Martin > >> Anyway. I don't think we're going to get rid of the array notation >> anytime soon, if ever, no matter how much I dislike it, so I think the >> right fix would be to at least state the correct required size in >> drm_dp_channel_eq_ok(). > > Ok. Just to confirm: Changing the declaration to an unspecified length > avoids the warnings, as does the patch below: > > diff --git a/drivers/gpu/drm/drm_dp_helper.c b/drivers/gpu/drm/drm_dp_helper.c > index eedbb48815b7..6ebeec3d88a7 100644 > --- a/drivers/gpu/drm/drm_dp_helper.c > +++ b/drivers/gpu/drm/drm_dp_helper.c > @@ -46,12 +46,12 @@ > */ > > /* Helpers for DP link training */ > -static u8 dp_link_status(const u8 link_status[DP_LINK_STATUS_SIZE], int r) > +static u8 dp_link_status(const u8 link_status[DP_LINK_STATUS_SIZE - 2], int r) > { > return link_status[r - DP_LANE0_1_STATUS]; > } > > -static u8 dp_get_lane_status(const u8 link_status[DP_LINK_STATUS_SIZE], > +static u8 dp_get_lane_status(const u8 link_status[DP_LINK_STATUS_SIZE - 2], > int lane) > { > int i = DP_LANE0_1_STATUS + (lane >> 1); > @@ -61,7 +61,7 @@ static u8 dp_get_lane_status(const u8 > link_status[DP_LINK_STATUS_SIZE], > return (l >> s) & 0xf; > } > > -bool drm_dp_channel_eq_ok(const u8 link_status[DP_LINK_STATUS_SIZE], > +bool drm_dp_channel_eq_ok(const u8 link_status[DP_LINK_STATUS_SIZE - 2], > int lane_count) > { > u8 lane_align; > diff --git a/include/drm/drm_dp_helper.h b/include/drm/drm_dp_helper.h > index edffd1dcca3e..160f7fd127b1 100644 > --- a/include/drm/drm_dp_helper.h > +++ b/include/drm/drm_dp_helper.h > @@ -1456,7 +1456,7 @@ enum drm_dp_phy { > > #define DP_LINK_CONSTANT_N_VALUE 0x8000 > #define DP_LINK_STATUS_SIZE 6 > -bool drm_dp_channel_eq_ok(const u8 link_status[DP_LINK_STATUS_SIZE], > +bool drm_dp_channel_eq_ok(const u8 link_status[DP_LINK_STATUS_SIZE - 2], > int lane_count); > bool drm_dp_clock_recovery_ok(const u8 link_status[DP_LINK_STATUS_SIZE], > int lane_count); > > > This obviously needs a good explanation in the code and the changelog text, > which I don't have, but if the above is what you had in mind, please take that > and add Reported-by/Tested-by: Arnd Bergmann . > > Arnd > _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel