All of lore.kernel.org
 help / color / mirror / Atom feed
From: "Gustavo A. R. Silva" <gustavoars@kernel.org>
To: Kevin Barnett <kevin.barnett@microsemi.com>,
	Don Brace <don.brace@microchip.com>,
	storagedev@microchip.com,
	"James E.J. Bottomley" <jejb@linux.ibm.com>,
	"Martin K. Petersen" <martin.petersen@oracle.com>
Cc: linux-scsi@vger.kernel.org, linux-kernel@vger.kernel.org,
	"Gustavo A. R. Silva" <gustavoars@kernel.org>,
	linux-hardening@vger.kernel.org
Subject: [PATCH 1/3][next] scsi: smartpqi: Replace one-element array with flexible-array member
Date: Wed, 21 Sep 2022 23:28:35 -0500	[thread overview]
Message-ID: <c80c0979933e0c05e80d95792ef167a28640a14b.1663816572.git.gustavoars@kernel.org> (raw)
In-Reply-To: <cover.1663816572.git.gustavoars@kernel.org>

One-element arrays are deprecated, and we are replacing them with flexible
array members instead. So, replace one-element array with flexible-array
member in struct MR_DRV_RAID_MAP and refactor the the rest of the code
accordingly.

It seems that the addition of sizeof(struct report_log_lun) in all the
places that are modified by this patch is due to the fact that
the one-element array struct report_log_lun lun_entries[1]; always
contributes to the size of the containing structure struct
report_log_lun_list.

Notice that at line 1267 while allocating memory for an instance of
struct report_log_lun_list, some _extra_ space seems to be allocated
for one element of type struct report_log_lun, which is the type of
the elements in array lun_entries:

 1267         internal_logdev_list = kmalloc(logdev_data_length +
 1268                 sizeof(struct report_log_lun), GFP_KERNEL);

However, at line 1275 just logdev_data_length bytes are copied into
internal_logdev_list (remember that we allocated space for logdev_data_length +
sizeof(struct report_log_lun) bytes at line 1267), and then exactly
sizeof(struct report_log_lun) bytes are being zeroing out at line 1276.

 1275         memcpy(internal_logdev_list, logdev_data, logdev_data_length);
 1276         memset((u8 *)internal_logdev_list + logdev_data_length, 0,
 1277                 sizeof(struct report_log_lun));

All the above makes think that it's just fine if we transform array
lun_entries into a flexible-array member and just don't allocate
that extra sizeof(struct report_log_lun) bytes of space. With this
we can remove that memset() call and we also need to modify the code
that updates the total length (internal_logdev_list->header.list_length)
of array lun_entries at line 1278:

 1278         put_unaligned_be32(logdev_list_length +
 1279                 sizeof(struct report_log_lun),
 1280                 &internal_logdev_list->header.list_length);

This helps with the ongoing efforts to tighten the FORTIFY_SOURCE routines
on memcpy().

Link: https://github.com/KSPP/linux/issues/79
Link: https://github.com/KSPP/linux/issues/204
Signed-off-by: Gustavo A. R. Silva <gustavoars@kernel.org>
---
And of course, it'd be great if maintainers can confirm what I described
in the changelog text. :)

 drivers/scsi/smartpqi/smartpqi.h      |  2 +-
 drivers/scsi/smartpqi/smartpqi_init.c | 10 +++-------
 2 files changed, 4 insertions(+), 8 deletions(-)

diff --git a/drivers/scsi/smartpqi/smartpqi.h b/drivers/scsi/smartpqi/smartpqi.h
index e550b12e525a..d1756c9d1112 100644
--- a/drivers/scsi/smartpqi/smartpqi.h
+++ b/drivers/scsi/smartpqi/smartpqi.h
@@ -954,7 +954,7 @@ struct report_log_lun {
 
 struct report_log_lun_list {
 	struct report_lun_header header;
-	struct report_log_lun lun_entries[1];
+	struct report_log_lun lun_entries[];
 };
 
 struct report_phys_lun_8byte_wwid {
diff --git a/drivers/scsi/smartpqi/smartpqi_init.c b/drivers/scsi/smartpqi/smartpqi_init.c
index b971fbe3b3a1..544cd18a90d7 100644
--- a/drivers/scsi/smartpqi/smartpqi_init.c
+++ b/drivers/scsi/smartpqi/smartpqi_init.c
@@ -1264,8 +1264,7 @@ static int pqi_get_device_lists(struct pqi_ctrl_info *ctrl_info,
 	logdev_data_length = sizeof(struct report_lun_header) +
 		logdev_list_length;
 
-	internal_logdev_list = kmalloc(logdev_data_length +
-		sizeof(struct report_log_lun), GFP_KERNEL);
+	internal_logdev_list = kmalloc(logdev_data_length, GFP_KERNEL);
 	if (!internal_logdev_list) {
 		kfree(*logdev_list);
 		*logdev_list = NULL;
@@ -1273,11 +1272,8 @@ static int pqi_get_device_lists(struct pqi_ctrl_info *ctrl_info,
 	}
 
 	memcpy(internal_logdev_list, logdev_data, logdev_data_length);
-	memset((u8 *)internal_logdev_list + logdev_data_length, 0,
-		sizeof(struct report_log_lun));
-	put_unaligned_be32(logdev_list_length +
-		sizeof(struct report_log_lun),
-		&internal_logdev_list->header.list_length);
+	put_unaligned_be32(logdev_list_length,
+			   &internal_logdev_list->header.list_length);
 
 	kfree(*logdev_list);
 	*logdev_list = internal_logdev_list;
-- 
2.34.1


  reply	other threads:[~2022-09-22  4:28 UTC|newest]

Thread overview: 10+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-09-22  4:26 [PATCH 0/3][next] scsi: smartpqi: Replace one-element arrays with flexible-array members Gustavo A. R. Silva
2022-09-22  4:28 ` Gustavo A. R. Silva [this message]
2023-02-02 20:25   ` [PATCH 1/3][next] scsi: smartpqi: Replace one-element array with flexible-array member Kees Cook
     [not found]   ` <CY4PR11MB12387B9F495BC7B5D0F5FD84E1DA9@CY4PR11MB1238.namprd11.prod.outlook.com>
2023-02-06 22:28     ` Gustavo A. R. Silva
     [not found]       ` <CY4PR11MB123855C0E92965CD5B46C9B6E1DB9@CY4PR11MB1238.namprd11.prod.outlook.com>
2023-02-07 21:53         ` Gustavo A. R. Silva
2022-09-22  4:29 ` [PATCH 2/3][next] scsi: smartpqi: Replace one-element arrays with flexible-array members Gustavo A. R. Silva
2023-02-02 20:28   ` Kees Cook
2022-09-22  4:30 ` [PATCH 3/3][next] scsi: smartpqi: Use struct_size() helper in pqi_report_phys_luns() Gustavo A. R. Silva
2023-02-02 20:29   ` Kees Cook
2023-01-31 20:14 ` [PATCH 0/3][next] scsi: smartpqi: Replace one-element arrays with flexible-array members Gustavo A. R. Silva

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=c80c0979933e0c05e80d95792ef167a28640a14b.1663816572.git.gustavoars@kernel.org \
    --to=gustavoars@kernel.org \
    --cc=don.brace@microchip.com \
    --cc=jejb@linux.ibm.com \
    --cc=kevin.barnett@microsemi.com \
    --cc=linux-hardening@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-scsi@vger.kernel.org \
    --cc=martin.petersen@oracle.com \
    --cc=storagedev@microchip.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.