Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
Message-ID: <22139a80-3f64-1f21-6b5c-65d250bafe09@linux.ibm.com>
Date:   Fri, 5 Nov 2021 12:24:13 +0100
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:91.0) Gecko/20100101
 Thunderbird/91.2.0
Subject: Re: [PATCH][next] scsi: Replace one-element arrays with
 flexible-array members
Content-Language: en-US
To:     "Gustavo A. R. Silva" <gustavoars@kernel.org>,
        "James E.J. Bottomley" <jejb@linux.ibm.com>,
        "Martin K. Petersen" <martin.petersen@oracle.com>,
        Hannes Reinecke <hare@suse.de>
Cc:     linux-scsi@vger.kernel.org, linux-kernel@vger.kernel.org,
        linux-hardening@vger.kernel.org, Kees Cook <keescook@chromium.org>,
        Javed Hasan <jhasan@marvell.com>,
        Himanshu Madhani <himanshu.madhani@oracle.com>,
        Neerav Parikh <neerav.parikh@intel.com>,
        Ross Brattain <ross.b.brattain@intel.com>,
        Robert Love <robert.w.love@intel.com>
References: <20211105091102.GA126301@embeddedor>
From:   Steffen Maier <maier@linux.ibm.com>
In-Reply-To: <20211105091102.GA126301@embeddedor>
Content-Type: text/plain; charset=UTF-8; format=flowed
Content-Transfer-Encoding: 7bit
MIME-Version: 1.0
Precedence: bulk

On 11/5/21 10:11, Gustavo A. R. Silva wrote:
> Use flexible-array members in struct fc_fdmi_attr_entry and
> fs_fdmi_attrs instead of one-element arrays, and refactor the
> code accordingly.
> 
> Also, turn the one-element array _port_ in struct fc_fdmi_rpl
> into a simple object of type struct fc_fdmi_port_name, as it
> seems there is no more than just one port expected:
> 
> $ git grep -nw numport drivers/scsi/
> drivers/scsi/csiostor/csio_lnode.c:447: reg_pl->numport = htonl(1);
> drivers/scsi/libfc/fc_encode.h:232:             put_unaligned_be32(1, &ct->payload.rhba.port.numport);
> 
> Also, this helps with the ongoing efforts to globally enable
> -Warray-bounds and get us closer to being able to tighten the
> FORTIFY_SOURCE routines on memcpy().
> 
> https://github.com/KSPP/linux/issues/79
> Signed-off-by: Gustavo A. R. Silva <gustavoars@kernel.org>
> ---
>   drivers/scsi/csiostor/csio_lnode.c | 2 +-
>   drivers/scsi/libfc/fc_encode.h     | 4 ++--
>   include/scsi/fc/fc_ms.h            | 6 +++---
>   3 files changed, 6 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/scsi/csiostor/csio_lnode.c b/drivers/scsi/csiostor/csio_lnode.c
> index d5ac93897023..cf9dd79ee488 100644
> --- a/drivers/scsi/csiostor/csio_lnode.c
> +++ b/drivers/scsi/csiostor/csio_lnode.c
> @@ -445,7 +445,7 @@ csio_ln_fdmi_dprt_cbfn(struct csio_hw *hw, struct csio_ioreq *fdmi_req)
>   	/* Register one port per hba */
>   	reg_pl = (struct fc_fdmi_rpl *)pld;
>   	reg_pl->numport = htonl(1);
> -	memcpy(&reg_pl->port[0].portname, csio_ln_wwpn(ln), 8);
> +	memcpy(&reg_pl->port.portname, csio_ln_wwpn(ln), 8);
>   	pld += sizeof(*reg_pl);
> 
>   	/* Start appending HBA attributes hba */
> diff --git a/drivers/scsi/libfc/fc_encode.h b/drivers/scsi/libfc/fc_encode.h
> index 74ae7fd15d8d..5806f99e4061 100644
> --- a/drivers/scsi/libfc/fc_encode.h
> +++ b/drivers/scsi/libfc/fc_encode.h
> @@ -232,7 +232,7 @@ static inline int fc_ct_ms_fill(struct fc_lport *lport,
>   		put_unaligned_be32(1, &ct->payload.rhba.port.numport);
>   		/* Port Name */
>   		put_unaligned_be64(lport->wwpn,
> -				   &ct->payload.rhba.port.port[0].portname);
> +				   &ct->payload.rhba.port.port.portname);
> 
>   		/* HBA Attributes */
>   		put_unaligned_be32(numattrs,

> diff --git a/include/scsi/fc/fc_ms.h b/include/scsi/fc/fc_ms.h
> index 00191695233a..44fbe84fa664 100644
> --- a/include/scsi/fc/fc_ms.h
> +++ b/include/scsi/fc/fc_ms.h

> @@ -174,7 +174,7 @@ struct fs_fdmi_attrs {

/*
  * Registered Port List

>    */
>   struct fc_fdmi_rpl {
>   	__be32				numport;
> -	struct fc_fdmi_port_name	port[1];
> +	struct fc_fdmi_port_name	port;
>   } __attribute__((__packed__));

While I'm not affected by the change, it feels to me as if these are protocol 
definitions originating in a T11 Fibre Channel standard FC-GS. It's a port 
*list*. Can you "modify" the standard here?

The fact, that currently existing code users only ever seem to use one single 
port in the list, would be an independent thing to me.


-- 
Mit freundlichen Gruessen / Kind regards
Steffen Maier

Linux on IBM Z and LinuxONE

https://www.ibm.com/privacy/us/en/
IBM Deutschland Research & Development GmbH
Vorsitzender des Aufsichtsrats: Gregor Pillen
Geschaeftsfuehrung: Dirk Wittkopp
Sitz der Gesellschaft: Boeblingen
Registergericht: Amtsgericht Stuttgart, HRB 243294