Content-Type: text/plain; charset="utf-8"
MIME-Version: 1.0
Content-Transfer-Encoding: 7bit
Subject: Re: [PATCH v2] wifi: ipw2x00: convert ipw_fw_error->elem to flexible
 array[]
From:   Kalle Valo <kvalo@kernel.org>
In-Reply-To: <20230307230148.3735684-1-jacob.e.keller@intel.com>
References: <20230307230148.3735684-1-jacob.e.keller@intel.com>
To:     Jacob Keller <jacob.e.keller@intel.com>
Cc:     Johannes Berg <johannes@sipsolutions.net>,
        linux-wireless@vger.kernel.org,
        Jacob Keller <jacob.e.keller@intel.com>,
        Stanislav Yakovlev <stas.yakovlev@gmail.com>
User-Agent: pwcli/0.1.1-git (https://github.com/kvalo/pwcli/) Python/3.7.3
Message-ID: <167871506266.31347.15867066093999254730.kvalo@kernel.org>
Date:   Mon, 13 Mar 2023 13:44:24 +0000 (UTC)
Precedence: bulk

Jacob Keller <jacob.e.keller@intel.com> wrote:

> The ipw_fw_error structure contains a payload[] flexible array as well as
> two pointers to this array area, ->elem, and ->log. The total size of the
> allocated structure is computed without use of the <linux/overflow.h>
> macros.
> 
> There's no reason to keep both a payload[] and an extra pointer to both the
> elem and log members. Convert the elem pointer member into the flexible
> array member, removing payload.
> 
> Fix the allocation of the ipw_fw_error structure to use size_add(),
> struct_size(), and array_size() to compute the allocation. This ensures
> that any overflow saturates at SIZE_MAX rather than overflowing and
> potentially allowing an undersized allocation.
> 
> Before the structure change, the layout of ipw_fw_error was:
> 
> struct ipw_fw_error {
>         long unsigned int          jiffies;              /*     0     8 */
>         u32                        status;               /*     8     4 */
>         u32                        config;               /*    12     4 */
>         u32                        elem_len;             /*    16     4 */
>         u32                        log_len;              /*    20     4 */
>         struct ipw_error_elem *    elem;                 /*    24     8 */
>         struct ipw_event *         log;                  /*    32     8 */
>         u8                         payload[];            /*    40     0 */
> 
>         /* size: 40, cachelines: 1, members: 8 */
>         /* last cacheline: 40 bytes */
> };
> 
> After this change, the layout is now:
> 
> struct ipw_fw_error {
>         long unsigned int          jiffies;              /*     0     8 */
>         u32                        status;               /*     8     4 */
>         u32                        config;               /*    12     4 */
>         u32                        elem_len;             /*    16     4 */
>         u32                        log_len;              /*    20     4 */
>         struct ipw_event *         log;                  /*    24     8 */
>         struct ipw_error_elem      elem[];               /*    32     0 */
> 
>         /* size: 32, cachelines: 1, members: 7 */
>         /* last cacheline: 32 bytes */
> };
> 
> This saves a total of 8 bytes for every ipw_fw_error allocation, and
> removes the risk of a potential overflow on the allocation.
> 
> Signed-off-by: Jacob Keller <jacob.e.keller@intel.com>
> Cc: Stanislav Yakovlev <stas.yakovlev@gmail.com>

Patch applied to wireless-next.git, thanks.

a23c82e006db wifi: ipw2x00: convert ipw_fw_error->elem to flexible array[]

-- 
https://patchwork.kernel.org/project/linux-wireless/patch/20230307230148.3735684-1-jacob.e.keller@intel.com/

https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches