All of lore.kernel.org
 help / color / mirror / Atom feed
From: Kees Cook <keescook@chromium.org>
To: "Gustavo A. R. Silva" <gustavoars@kernel.org>
Cc: "Martin K. Petersen" <martin.petersen@oracle.com>,
	"James E.J. Bottomley" <jejb@linux.ibm.com>,
	Dick Kennedy <dick.kennedy@broadcom.com>,
	James Smart <james.smart@broadcom.com>,
	linux-scsi@vger.kernel.org, linux-kernel@vger.kernel.org,
	linux-hardening@vger.kernel.org
Subject: Re: [PATCH 2/2][next] scsi: lpfc: Use struct_size() helper
Date: Tue, 23 May 2023 10:27:51 -0700	[thread overview]
Message-ID: <202305231026.594B2F913D@keescook> (raw)
In-Reply-To: <ZGzQt2VFG4P/Vufn@work>

On Tue, May 23, 2023 at 08:41:59AM -0600, Gustavo A. R. Silva wrote:
> On Wed, May 17, 2023 at 04:01:47PM -0700, Kees Cook wrote:
> > On Wed, May 17, 2023 at 03:23:01PM -0600, Gustavo A. R. Silva wrote:
> > > Prefer struct_size() over open-coded versions of idiom:
> > > 
> > > sizeof(struct-with-flex-array) + sizeof(typeof-flex-array-elements) * count
> > > 
> > > where count is the max number of items the flexible array is supposed to
> > > contain.
> > > 
> > > Link: https://github.com/KSPP/linux/issues/160
> > > Signed-off-by: Gustavo A. R. Silva <gustavoars@kernel.org>
> > > ---
> > >  drivers/scsi/lpfc/lpfc_ct.c | 6 ++----
> > >  1 file changed, 2 insertions(+), 4 deletions(-)
> > > 
> > > diff --git a/drivers/scsi/lpfc/lpfc_ct.c b/drivers/scsi/lpfc/lpfc_ct.c
> > > index e880d127d7f5..3b95c56023bf 100644
> > > --- a/drivers/scsi/lpfc/lpfc_ct.c
> > > +++ b/drivers/scsi/lpfc/lpfc_ct.c
> > > @@ -3748,8 +3748,7 @@ lpfc_vmid_cmd(struct lpfc_vport *vport,
> > >  		rap->obj[0].entity_id_len = vmid->vmid_len;
> > >  		memcpy(rap->obj[0].entity_id, vmid->host_vmid, vmid->vmid_len);
> > >  		size = RAPP_IDENT_OFFSET +
> > > -			sizeof(struct lpfc_vmid_rapp_ident_list) +
> > > -			sizeof(struct entity_id_object);
> > > +			struct_size(rap, obj, rap->no_of_objects);
> > 
> > Has rap->no_of_objects always been "1"? (i.e. there was a prior
> > multiplication here before...
> 
> Mmh.. not sure what multiplication you are talking about. I based these
> changes on the fact that rap->no_of_objects is set to cpu_to_be32(1);
> for both instances. It doesn't show up in the context of the patch, so
> here you go:
> 
> 3747                 rap->no_of_objects = cpu_to_be32(1);

Ah-ha! So, yeah, this patch is bad then, since no_of_objects will be a
big-endian "1".

This change:

+			struct_size(rap, obj, rap->no_of_objects);

needs to explicitly be:

+			struct_size(rap, obj, 1);

Or, alternatively, just drop the patch entirely.

-- 
Kees Cook

  reply	other threads:[~2023-05-23 17:28 UTC|newest]

Thread overview: 13+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-05-17 21:22 [PATCH 0/2][next] scsi: lpfc: Replace one-element array with flexible-array member Gustavo A. R. Silva
2023-05-17 21:22 ` [PATCH 1/2][next] " Gustavo A. R. Silva
2023-05-17 23:02   ` Kees Cook
2023-05-22 22:02   ` Martin K. Petersen
2023-05-22 22:12     ` Gustavo A. R. Silva
2023-05-23 17:31     ` Kees Cook
2023-05-23 19:46       ` Gustavo A. R. Silva
2023-05-23 20:29         ` Kees Cook
2023-05-17 21:23 ` [PATCH 2/2][next] scsi: lpfc: Use struct_size() helper Gustavo A. R. Silva
2023-05-17 23:01   ` Kees Cook
2023-05-23 14:41     ` Gustavo A. R. Silva
2023-05-23 17:27       ` Kees Cook [this message]
2023-06-01  0:43 ` [PATCH 0/2][next] scsi: lpfc: Replace one-element array with flexible-array member Martin K. Petersen

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=202305231026.594B2F913D@keescook \
    --to=keescook@chromium.org \
    --cc=dick.kennedy@broadcom.com \
    --cc=gustavoars@kernel.org \
    --cc=james.smart@broadcom.com \
    --cc=jejb@linux.ibm.com \
    --cc=linux-hardening@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-scsi@vger.kernel.org \
    --cc=martin.petersen@oracle.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.