Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by smtp.lore.kernel.org (Postfix) with ESMTP id AAA68C4167B for ; Tue, 23 Nov 2021 23:53:52 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S240287AbhKWX5A (ORCPT ); Tue, 23 Nov 2021 18:57:00 -0500 Received: from mga05.intel.com ([192.55.52.43]:50219 "EHLO mga05.intel.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S233264AbhKWX45 (ORCPT ); Tue, 23 Nov 2021 18:56:57 -0500 X-IronPort-AV: E=McAfee;i="6200,9189,10177"; a="321397703" X-IronPort-AV: E=Sophos;i="5.87,258,1631602800"; d="scan'208";a="321397703" Received: from orsmga008.jf.intel.com ([10.7.209.65]) by fmsmga105.fm.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 23 Nov 2021 15:53:48 -0800 X-IronPort-AV: E=Sophos;i="5.87,258,1631602800"; d="scan'208";a="509610560" Received: from pshinde-mobl.amr.corp.intel.com ([10.213.85.70]) by orsmga008-auth.jf.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 23 Nov 2021 15:53:41 -0800 Message-ID: Subject: Re: [PATCH v2 12/63] thermal: intel: int340x_thermal: Use struct_group() for memcpy() region From: Srinivas Pandruvada To: "Rafael J. Wysocki" , Kees Cook , Zhang Rui Cc: Linux Kernel Mailing List , Daniel Lezcano , Amit Kucheria , Linux PM , "Gustavo A. R. Silva" , Greg Kroah-Hartman , Andrew Morton , "open list:NETWORKING DRIVERS (WIRELESS)" , netdev , dri-devel , linux-staging@lists.linux.dev, linux-block@vger.kernel.org, linux-kbuild@vger.kernel.org, clang-built-linux@googlegroups.com, Rasmus Villemoes , linux-hardening@vger.kernel.org Date: Tue, 23 Nov 2021 15:53:38 -0800 In-Reply-To: References: <20210818060533.3569517-1-keescook@chromium.org> <20210818060533.3569517-13-keescook@chromium.org> Content-Type: text/plain; charset="UTF-8" User-Agent: Evolution 3.40.0-1 MIME-Version: 1.0 Content-Transfer-Encoding: 8bit Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Tue, 2021-11-23 at 14:19 +0100, Rafael J. Wysocki wrote: > On Wed, Aug 18, 2021 at 8:08 AM Kees Cook > wrote: > > > > In preparation for FORTIFY_SOURCE performing compile-time and run- > > time > > field bounds checking for memcpy(), avoid intentionally writing > > across > > neighboring fields. > > > > Use struct_group() in struct art around members weight, and ac[0- > > 9]_max, > > so they can be referenced together. This will allow memcpy() and > > sizeof() > > to more easily reason about sizes, improve readability, and avoid > > future > > warnings about writing beyond the end of weight. > > > > "pahole" shows no size nor member offset changes to struct art. > > "objdump -d" shows no meaningful object code changes (i.e. only > > source > > line number induced differences). > > > > Cc: Zhang Rui > > Cc: Daniel Lezcano > > Cc: Amit Kucheria > > Cc: linux-pm@vger.kernel.org > > Signed-off-by: Kees Cook > > Rui, Srinivas, any comments here? Looks good. Reviewed-by: Srinivas Pandruvada Thanks, Srinivas > > > --- > >  .../intel/int340x_thermal/acpi_thermal_rel.c  |  5 +- > >  .../intel/int340x_thermal/acpi_thermal_rel.h  | 48 ++++++++++------- > > -- > >  2 files changed, 29 insertions(+), 24 deletions(-) > > > > diff --git a/drivers/thermal/intel/int340x_thermal/acpi_thermal_rel.c > > b/drivers/thermal/intel/int340x_thermal/acpi_thermal_rel.c > > index a478cff8162a..e90690a234c4 100644 > > --- a/drivers/thermal/intel/int340x_thermal/acpi_thermal_rel.c > > +++ b/drivers/thermal/intel/int340x_thermal/acpi_thermal_rel.c > > @@ -250,8 +250,9 @@ static int fill_art(char __user *ubuf) > >                 get_single_name(arts[i].source, > > art_user[i].source_device); > >                 get_single_name(arts[i].target, > > art_user[i].target_device); > >                 /* copy the rest int data in addition to source and > > target */ > > -               memcpy(&art_user[i].weight, &arts[i].weight, > > -                       sizeof(u64) * (ACPI_NR_ART_ELEMENTS - 2)); > > +               BUILD_BUG_ON(sizeof(art_user[i].data) != > > +                            sizeof(u64) * (ACPI_NR_ART_ELEMENTS - > > 2)); > > +               memcpy(&art_user[i].data, &arts[i].data, > > sizeof(art_user[i].data)); > >         } > > > >         if (copy_to_user(ubuf, art_user, art_len)) > > diff --git a/drivers/thermal/intel/int340x_thermal/acpi_thermal_rel.h > > b/drivers/thermal/intel/int340x_thermal/acpi_thermal_rel.h > > index 58822575fd54..78d942477035 100644 > > --- a/drivers/thermal/intel/int340x_thermal/acpi_thermal_rel.h > > +++ b/drivers/thermal/intel/int340x_thermal/acpi_thermal_rel.h > > @@ -17,17 +17,19 @@ > >  struct art { > >         acpi_handle source; > >         acpi_handle target; > > -       u64 weight; > > -       u64 ac0_max; > > -       u64 ac1_max; > > -       u64 ac2_max; > > -       u64 ac3_max; > > -       u64 ac4_max; > > -       u64 ac5_max; > > -       u64 ac6_max; > > -       u64 ac7_max; > > -       u64 ac8_max; > > -       u64 ac9_max; > > +       struct_group(data, > > +               u64 weight; > > +               u64 ac0_max; > > +               u64 ac1_max; > > +               u64 ac2_max; > > +               u64 ac3_max; > > +               u64 ac4_max; > > +               u64 ac5_max; > > +               u64 ac6_max; > > +               u64 ac7_max; > > +               u64 ac8_max; > > +               u64 ac9_max; > > +       ); > >  } __packed; > > > >  struct trt { > > @@ -47,17 +49,19 @@ union art_object { > >         struct { > >                 char source_device[8]; /* ACPI single name */ > >                 char target_device[8]; /* ACPI single name */ > > -               u64 weight; > > -               u64 ac0_max_level; > > -               u64 ac1_max_level; > > -               u64 ac2_max_level; > > -               u64 ac3_max_level; > > -               u64 ac4_max_level; > > -               u64 ac5_max_level; > > -               u64 ac6_max_level; > > -               u64 ac7_max_level; > > -               u64 ac8_max_level; > > -               u64 ac9_max_level; > > +               struct_group(data, > > +                       u64 weight; > > +                       u64 ac0_max_level; > > +                       u64 ac1_max_level; > > +                       u64 ac2_max_level; > > +                       u64 ac3_max_level; > > +                       u64 ac4_max_level; > > +                       u64 ac5_max_level; > > +                       u64 ac6_max_level; > > +                       u64 ac7_max_level; > > +                       u64 ac8_max_level; > > +                       u64 ac9_max_level; > > +               ); > >         }; > >         u64 __data[ACPI_NR_ART_ELEMENTS]; > >  }; > > -- > > 2.30.2 > >