[PATCH 0/2] scsi: qla2xxx: flexible array / field-spanning write issue

[PATCH 0/2] scsi: qla2xxx: flexible array / field-spanning write issue

[Chris Leech]

Bryan Gurney was running into these field-spanning write warnings while
testing some in-progress NVMe FPIN work with qla2xxx.

https://lore.kernel.org/linux-nvme/20250709211919.49100-1-bgurney@redhat.com/
  >  kernel: memcpy: detected field-spanning write (size 60) of single field
  >  "((uint8_t *)fpin_pkt + buffer_copy_offset)"
  >  at drivers/scsi/qla2xxx/qla_isr.c:1221 (size 44)

I think these changes should fix up the non-standard flexible array
allocations, and fix he warning for Bryan's testing.

Chris Leech (2):
  scsi: qla2xxx: replace non-standard flexible array purex_item.iocb
  scsi: qla2xxx: unwrap purex_item.iocb.iocb so that __counted_by can be
    used

 drivers/scsi/qla2xxx/qla_def.h  |  5 ++---
 drivers/scsi/qla2xxx/qla_isr.c  | 15 +++++++--------
 drivers/scsi/qla2xxx/qla_nvme.c |  2 +-
 drivers/scsi/qla2xxx/qla_os.c   |  5 +++--
 4 files changed, 13 insertions(+), 14 deletions(-)

-- 
2.50.1

[PATCH 1/2] scsi: qla2xxx: replace non-standard flexible array purex_item.iocb

[Chris Leech]

This is defined as a 64-element u8 array, but 64 is the minimum size and
it can be allocated larger. I don't know why the array was wrapped as a
single element struct of the same name.

Replace with a union around DECLARE_FLEX_ARRAY and padding to maintain
sizeof(struct purex_item) and associated use.

This was triggering a field-spanning write warning during FPIN testing
https://lore.kernel.org/linux-nvme/20250709211919.49100-1-bgurney@redhat.com/

  >  kernel: memcpy: detected field-spanning write (size 60) of single field
  >  "((uint8_t *)fpin_pkt + buffer_copy_offset)"
  >  at drivers/scsi/qla2xxx/qla_isr.c:1221 (size 44)

Tested-by: Bryan Gurney <bgurney@redhat.com>
Signed-off-by: Chris Leech <cleech@redhat.com>
---
 drivers/scsi/qla2xxx/qla_def.h | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/drivers/scsi/qla2xxx/qla_def.h b/drivers/scsi/qla2xxx/qla_def.h
index cb95b7b12051d..6237fefeca149 100644
--- a/drivers/scsi/qla2xxx/qla_def.h
+++ b/drivers/scsi/qla2xxx/qla_def.h
@@ -4890,8 +4890,9 @@ struct purex_item {
 			     struct purex_item *pkt);
 	atomic_t in_use;
 	uint16_t size;
-	struct {
-		uint8_t iocb[64];
+	union {
+		uint8_t __padding[QLA_DEFAULT_PAYLOAD_SIZE];
+		DECLARE_FLEX_ARRAY(uint8_t, iocb);
 	} iocb;
 };
 
-- 
2.50.1

Re: [PATCH 1/2] scsi: qla2xxx: replace non-standard flexible array purex_item.iocb

[Kees Cook]

On Fri, Jul 25, 2025 at 02:27:31PM -0700, Chris Leech wrote:
> This is defined as a 64-element u8 array, but 64 is the minimum size and
> it can be allocated larger. I don't know why the array was wrapped as a
> single element struct of the same name.
> 
> Replace with a union around DECLARE_FLEX_ARRAY and padding to maintain
> sizeof(struct purex_item) and associated use.
> 
> This was triggering a field-spanning write warning during FPIN testing
> https://lore.kernel.org/linux-nvme/20250709211919.49100-1-bgurney@redhat.com/
> 
>   >  kernel: memcpy: detected field-spanning write (size 60) of single field
>   >  "((uint8_t *)fpin_pkt + buffer_copy_offset)"
>   >  at drivers/scsi/qla2xxx/qla_isr.c:1221 (size 44)

I think this is:

                                memcpy(((uint8_t *)fpin_pkt +
                                    buffer_copy_offset), new_pkt->data,
                                    no_bytes);

I was briefly confused since fpin_pkt is "void *", but I see the
bounds information comes from this assignment: 

        item = qla24xx_alloc_purex_item(vha, total_bytes);
        if (!item)
                return item;

        fpin_pkt = &item->iocb;

> Tested-by: Bryan Gurney <bgurney@redhat.com>
> Signed-off-by: Chris Leech <cleech@redhat.com>
> ---
>  drivers/scsi/qla2xxx/qla_def.h | 5 +++--
>  1 file changed, 3 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/scsi/qla2xxx/qla_def.h b/drivers/scsi/qla2xxx/qla_def.h
> index cb95b7b12051d..6237fefeca149 100644
> --- a/drivers/scsi/qla2xxx/qla_def.h
> +++ b/drivers/scsi/qla2xxx/qla_def.h
> @@ -4890,8 +4890,9 @@ struct purex_item {
>  			     struct purex_item *pkt);
>  	atomic_t in_use;
>  	uint16_t size;
> -	struct {
> -		uint8_t iocb[64];
> +	union {
> +		uint8_t __padding[QLA_DEFAULT_PAYLOAD_SIZE];
> +		DECLARE_FLEX_ARRAY(uint8_t, iocb);
>  	} iocb;
>  };

This won't work, unfortunately, as it seems struct purex_item
is embedded into struct scsi_qla_host. (i.e. try building with
KCFLAGS="-Wflex-array-member-not-at-end" and you should see a warning.)

Maybe qla24xx_alloc_purex_item() could return a union type like:

union purex_item_payload
{
	struct purex_item item;
	struct {
		uint8_t __padding[sizeof(struct purex_item) - 64];
		DECLARE_FLEX_ARRAY(uint8_t, iocb);
	};
};

And refactor the handful of callers? In this particular case:


diff --git a/drivers/scsi/qla2xxx/qla_isr.c b/drivers/scsi/qla2xxx/qla_isr.c
index fe98c76e9be3..1b3d96d623c1 100644
--- a/drivers/scsi/qla2xxx/qla_isr.c
+++ b/drivers/scsi/qla2xxx/qla_isr.c
@@ -1158,7 +1158,7 @@ qla27xx_copy_fpin_pkt(struct scsi_qla_host *vha, void **pkt,
 	uint16_t no_bytes = 0, total_bytes = 0, pending_bytes = 0;
 	uint16_t buffer_copy_offset = 0;
 	uint16_t entry_count, entry_count_remaining;
-	struct purex_item *item;
+	struct purex_item_blob *blob;
 	void *fpin_pkt = NULL;
 
 	total_bytes = (le16_to_cpu(purex->frame_size) & 0x0FFF)
@@ -1171,11 +1171,11 @@ qla27xx_copy_fpin_pkt(struct scsi_qla_host *vha, void **pkt,
 	       "FPIN ELS, frame_size 0x%x, entry count %d\n",
 	       total_bytes, entry_count);
 
-	item = qla24xx_alloc_purex_item(vha, total_bytes);
-	if (!item)
-		return item;
+	blob = qla24xx_alloc_purex_item(vha, total_bytes);
+	if (!blob)
+		return NULL;
 
-	fpin_pkt = &item->iocb;
+	fpin_pkt = blob->iocb;
 
 	memcpy(fpin_pkt, &purex->els_frame_payload[0], no_bytes);
 	buffer_copy_offset += no_bytes;
@@ -1238,12 +1238,12 @@ qla27xx_copy_fpin_pkt(struct scsi_qla_host *vha, void **pkt,
 			ql_log(ql_log_fatal, vha, 0x508b,
 			       "Dropping partial FPIN, underrun bytes = 0x%x, entry cnts 0x%x\n",
 			       total_bytes, entry_count_remaining);
-			qla24xx_free_purex_item(item);
+			qla24xx_free_purex_item(blob->item);
 			return NULL;
 		}
 	} while (entry_count_remaining > 0);
-	host_to_fcp_swap((uint8_t *)&item->iocb, total_bytes);
-	return item;
+	host_to_fcp_swap((uint8_t *)&blob->iocb, total_bytes);
+	return blob->item;
 }
 
 /**




-- 
Kees Cook

[PATCH v2 1/1] scsi: qla2xxx: replace non-standard flexible array purex_item.iocb

[Chris Leech]

purex_item.iocb is defined as a 64-element u8 array, but 64 is the
minimum size and it can be allocated larger.
This makes it a standard empty flex array.

This was motivated by field-spanning write warnings during FPIN testing.
https://lore.kernel.org/linux-nvme/20250709211919.49100-1-bgurney@redhat.com/

  >  kernel: memcpy: detected field-spanning write (size 60) of single field
  >  "((uint8_t *)fpin_pkt + buffer_copy_offset)"
  >  at drivers/scsi/qla2xxx/qla_isr.c:1221 (size 44)

I removed the outer wrapper from the iocb flex array, so that it can be
linked to purex_item.size with __counted_by.

The non-flex-array portion of purex_item is now defined within the
struct_group_tagged macro to create a type definition for just the
header. This was then used for the scsi_qla_host.default_item
definition to deal with -Wflex-array-member-not-at-end warnings.

These changes remove the default minimum 64-byte allocation, requiring
further changes.

  In struct scsi_qla_host the embedded default_item is now followed by
  __default_item_iocb[QLA_DEFAULT_PAYLOAD_SIZE] to reserve space that
  will be used as default_item.iocb

  qla24xx_alloc_purex_item is adjusted to no longer expect the default
  minimum size to be part of sizeof(struct purex_item), the entire
  flexible array size is added to the structure size for allocation.

This also slightly changes the layout of the purex_item struct, as
2-bytes of padding are added to purex_item_hdr and therefor between size
and iocb. The resulting size is the same, but iocb is shifted 2-bytes
(the old purex_item was padded at the end, after the 64-byte defined
array size). I don't think this is a problem.

Tested-by: Bryan Gurney <bgurney@redhat.com>
Signed-off-by: Chris Leech <cleech@redhat.com>
---
 drivers/scsi/qla2xxx/qla_def.h  | 23 ++++++++++++-----------
 drivers/scsi/qla2xxx/qla_isr.c  | 19 +++++++++----------
 drivers/scsi/qla2xxx/qla_nvme.c |  2 +-
 drivers/scsi/qla2xxx/qla_os.c   |  9 +++++----
 4 files changed, 27 insertions(+), 26 deletions(-)

diff --git a/drivers/scsi/qla2xxx/qla_def.h b/drivers/scsi/qla2xxx/qla_def.h
index cb95b7b12051d..0e7d201b1feb0 100644
--- a/drivers/scsi/qla2xxx/qla_def.h
+++ b/drivers/scsi/qla2xxx/qla_def.h
@@ -4883,16 +4883,16 @@ struct active_regions {
  * is variable) starting at "iocb".
  */
 struct purex_item {
-	void *purls_context;
-	struct list_head list;
-	struct scsi_qla_host *vha;
-	void (*process_item)(struct scsi_qla_host *vha,
-			     struct purex_item *pkt);
-	atomic_t in_use;
-	uint16_t size;
-	struct {
-		uint8_t iocb[64];
-	} iocb;
+	struct_group_tagged(purex_item_hdr, __hdr,
+		void *purls_context;
+		struct list_head list;
+		struct scsi_qla_host *vha;
+		void (*process_item)(struct scsi_qla_host *vha,
+				     struct purex_item *pkt);
+		atomic_t in_use;
+		uint16_t size;
+	);
+	uint8_t iocb[] __counted_by(size);
 };
 
 #include "qla_edif.h"
@@ -5101,7 +5101,8 @@ typedef struct scsi_qla_host {
 		struct list_head head;
 		spinlock_t lock;
 	} purex_list;
-	struct purex_item default_item;
+	struct purex_item_hdr default_item;
+	uint8_t __default_item_iocb[QLA_DEFAULT_PAYLOAD_SIZE];
 
 	struct name_list_extended gnl;
 	/* Count of active session/fcport */
diff --git a/drivers/scsi/qla2xxx/qla_isr.c b/drivers/scsi/qla2xxx/qla_isr.c
index fe98c76e9be32..a00c06a9898ec 100644
--- a/drivers/scsi/qla2xxx/qla_isr.c
+++ b/drivers/scsi/qla2xxx/qla_isr.c
@@ -1077,17 +1077,17 @@ static struct purex_item *
 qla24xx_alloc_purex_item(scsi_qla_host_t *vha, uint16_t size)
 {
 	struct purex_item *item = NULL;
-	uint8_t item_hdr_size = sizeof(*item);
 
 	if (size > QLA_DEFAULT_PAYLOAD_SIZE) {
-		item = kzalloc(item_hdr_size +
-		    (size - QLA_DEFAULT_PAYLOAD_SIZE), GFP_ATOMIC);
+		item = kzalloc(struct_size(item, iocb, size), GFP_ATOMIC);
 	} else {
 		if (atomic_inc_return(&vha->default_item.in_use) == 1) {
-			item = &vha->default_item;
+			item = (struct purex_item *)&vha->default_item;
 			goto initialize_purex_header;
 		} else {
-			item = kzalloc(item_hdr_size, GFP_ATOMIC);
+			item = kzalloc(
+				struct_size(item, iocb, QLA_DEFAULT_PAYLOAD_SIZE),
+				GFP_ATOMIC);
 		}
 	}
 	if (!item) {
@@ -1127,17 +1127,16 @@ qla24xx_queue_purex_item(scsi_qla_host_t *vha, struct purex_item *pkt,
  * @vha: SCSI driver HA context
  * @pkt: ELS packet
  */
-static struct purex_item
-*qla24xx_copy_std_pkt(struct scsi_qla_host *vha, void *pkt)
+static struct purex_item *
+qla24xx_copy_std_pkt(struct scsi_qla_host *vha, void *pkt)
 {
 	struct purex_item *item;
 
-	item = qla24xx_alloc_purex_item(vha,
-					QLA_DEFAULT_PAYLOAD_SIZE);
+	item = qla24xx_alloc_purex_item(vha, QLA_DEFAULT_PAYLOAD_SIZE);
 	if (!item)
 		return item;
 
-	memcpy(&item->iocb, pkt, sizeof(item->iocb));
+	memcpy(&item->iocb, pkt, QLA_DEFAULT_PAYLOAD_SIZE);
 	return item;
 }
 
diff --git a/drivers/scsi/qla2xxx/qla_nvme.c b/drivers/scsi/qla2xxx/qla_nvme.c
index 8ee2e337c9e1b..92488890bc04e 100644
--- a/drivers/scsi/qla2xxx/qla_nvme.c
+++ b/drivers/scsi/qla2xxx/qla_nvme.c
@@ -1308,7 +1308,7 @@ void qla2xxx_process_purls_iocb(void **pkt, struct rsp_que **rsp)
 
 	ql_dbg(ql_dbg_unsol, vha, 0x2121,
 	       "PURLS OP[%01x] size %d xchg addr 0x%x portid %06x\n",
-	       item->iocb.iocb[3], item->size, uctx->exchange_address,
+	       item->iocb[3], item->size, uctx->exchange_address,
 	       fcport->d_id.b24);
 	/* +48    0  1  2  3  4  5  6  7  8  9  A  B  C  D  E  F
 	 * ----- -----------------------------------------------
diff --git a/drivers/scsi/qla2xxx/qla_os.c b/drivers/scsi/qla2xxx/qla_os.c
index d4b484c0fd9d7..ef8a4fce0e03c 100644
--- a/drivers/scsi/qla2xxx/qla_os.c
+++ b/drivers/scsi/qla2xxx/qla_os.c
@@ -3971,7 +3971,7 @@ qla2x00_remove_one(struct pci_dev *pdev)
 static inline void
 qla24xx_free_purex_list(struct purex_list *list)
 {
-	struct purex_item *item, *next;
+	struct purex_item_hdr *item, *next;
 	ulong flags;
 
 	spin_lock_irqsave(&list->lock, flags);
@@ -6459,9 +6459,10 @@ void qla24xx_process_purex_rdp(struct scsi_qla_host *vha,
 void
 qla24xx_free_purex_item(struct purex_item *item)
 {
-	if (item == &item->vha->default_item)
-		memset(&item->vha->default_item, 0, sizeof(struct purex_item));
-	else
+	if (item == (struct purex_item *)&item->vha->default_item) {
+		memset(&item->vha->default_item, 0, sizeof(struct purex_item_hdr));
+		memset(&item->vha->__default_item_iocb, 0, QLA_DEFAULT_PAYLOAD_SIZE);
+	} else
 		kfree(item);
 }
 
-- 
2.50.1

Re: [PATCH v2 1/1] scsi: qla2xxx: replace non-standard flexible array purex_item.iocb

[Gustavo A. R. Silva]

On 28/07/25 12:57, Chris Leech wrote:
> purex_item.iocb is defined as a 64-element u8 array, but 64 is the
> minimum size and it can be allocated larger.
> This makes it a standard empty flex array.
> 
> This was motivated by field-spanning write warnings during FPIN testing.
> https://lore.kernel.org/linux-nvme/20250709211919.49100-1-bgurney@redhat.com/
> 
>    >  kernel: memcpy: detected field-spanning write (size 60) of single field
>    >  "((uint8_t *)fpin_pkt + buffer_copy_offset)"
>    >  at drivers/scsi/qla2xxx/qla_isr.c:1221 (size 44)
> 
> I removed the outer wrapper from the iocb flex array, so that it can be
> linked to purex_item.size with __counted_by.
> 
> The non-flex-array portion of purex_item is now defined within the
> struct_group_tagged macro to create a type definition for just the
> header. This was then used for the scsi_qla_host.default_item
> definition to deal with -Wflex-array-member-not-at-end warnings.
> 
> These changes remove the default minimum 64-byte allocation, requiring
> further changes.
> 
>    In struct scsi_qla_host the embedded default_item is now followed by
>    __default_item_iocb[QLA_DEFAULT_PAYLOAD_SIZE] to reserve space that
>    will be used as default_item.iocb
> 
>    qla24xx_alloc_purex_item is adjusted to no longer expect the default
>    minimum size to be part of sizeof(struct purex_item), the entire
>    flexible array size is added to the structure size for allocation.
> 
> This also slightly changes the layout of the purex_item struct, as
> 2-bytes of padding are added to purex_item_hdr and therefor between size
> and iocb. The resulting size is the same, but iocb is shifted 2-bytes
> (the old purex_item was padded at the end, after the 64-byte defined
> array size). I don't think this is a problem.
> 
> Tested-by: Bryan Gurney <bgurney@redhat.com>
> Signed-off-by: Chris Leech <cleech@redhat.com>
> ---
>   drivers/scsi/qla2xxx/qla_def.h  | 23 ++++++++++++-----------
>   drivers/scsi/qla2xxx/qla_isr.c  | 19 +++++++++----------
>   drivers/scsi/qla2xxx/qla_nvme.c |  2 +-
>   drivers/scsi/qla2xxx/qla_os.c   |  9 +++++----
>   4 files changed, 27 insertions(+), 26 deletions(-)
> 
> diff --git a/drivers/scsi/qla2xxx/qla_def.h b/drivers/scsi/qla2xxx/qla_def.h
> index cb95b7b12051d..0e7d201b1feb0 100644
> --- a/drivers/scsi/qla2xxx/qla_def.h
> +++ b/drivers/scsi/qla2xxx/qla_def.h
> @@ -4883,16 +4883,16 @@ struct active_regions {
>    * is variable) starting at "iocb".
>    */
>   struct purex_item {
> -	void *purls_context;
> -	struct list_head list;
> -	struct scsi_qla_host *vha;
> -	void (*process_item)(struct scsi_qla_host *vha,
> -			     struct purex_item *pkt);
> -	atomic_t in_use;
> -	uint16_t size;
> -	struct {
> -		uint8_t iocb[64];
> -	} iocb;
> +	struct_group_tagged(purex_item_hdr, __hdr,
> +		void *purls_context;
> +		struct list_head list;
> +		struct scsi_qla_host *vha;
> +		void (*process_item)(struct scsi_qla_host *vha,
> +				     struct purex_item *pkt);
> +		atomic_t in_use;
> +		uint16_t size;
> +	);
> +	uint8_t iocb[] __counted_by(size);
>   };
>   
>   #include "qla_edif.h"
> @@ -5101,7 +5101,8 @@ typedef struct scsi_qla_host {
>   		struct list_head head;
>   		spinlock_t lock;
>   	} purex_list;
> -	struct purex_item default_item;
> +	struct purex_item_hdr default_item;
> +	uint8_t __default_item_iocb[QLA_DEFAULT_PAYLOAD_SIZE];
>   
>   	struct name_list_extended gnl;
>   	/* Count of active session/fcport */
> diff --git a/drivers/scsi/qla2xxx/qla_isr.c b/drivers/scsi/qla2xxx/qla_isr.c
> index fe98c76e9be32..a00c06a9898ec 100644
> --- a/drivers/scsi/qla2xxx/qla_isr.c
> +++ b/drivers/scsi/qla2xxx/qla_isr.c
> @@ -1077,17 +1077,17 @@ static struct purex_item *
>   qla24xx_alloc_purex_item(scsi_qla_host_t *vha, uint16_t size)
>   {
>   	struct purex_item *item = NULL;
> -	uint8_t item_hdr_size = sizeof(*item);
>   
>   	if (size > QLA_DEFAULT_PAYLOAD_SIZE) {
> -		item = kzalloc(item_hdr_size +
> -		    (size - QLA_DEFAULT_PAYLOAD_SIZE), GFP_ATOMIC);
> +		item = kzalloc(struct_size(item, iocb, size), GFP_ATOMIC);

With the inclusion of `counted_by`, I think `item->size` should be updated
here:
		item->size = size;

>   	} else {
>   		if (atomic_inc_return(&vha->default_item.in_use) == 1) {
> -			item = &vha->default_item;
> +			item = (struct purex_item *)&vha->default_item;
>   			goto initialize_purex_header;
>   		} else {
> -			item = kzalloc(item_hdr_size, GFP_ATOMIC);
> +			item = kzalloc(
> +				struct_size(item, iocb, QLA_DEFAULT_PAYLOAD_SIZE),
> +				GFP_ATOMIC);

...and here:
			item->size = QLA_DEFAULT_PAYLOAD_SIZE;

Then remove `item->size = size;` just before `return item;`

Thanks
-Gustavo

>   		}
>   	}
>   	if (!item) {
> @@ -1127,17 +1127,16 @@ qla24xx_queue_purex_item(scsi_qla_host_t *vha, struct purex_item *pkt,
>    * @vha: SCSI driver HA context
>    * @pkt: ELS packet
>    */
> -static struct purex_item
> -*qla24xx_copy_std_pkt(struct scsi_qla_host *vha, void *pkt)
> +static struct purex_item *
> +qla24xx_copy_std_pkt(struct scsi_qla_host *vha, void *pkt)
>   {
>   	struct purex_item *item;
>   
> -	item = qla24xx_alloc_purex_item(vha,
> -					QLA_DEFAULT_PAYLOAD_SIZE);
> +	item = qla24xx_alloc_purex_item(vha, QLA_DEFAULT_PAYLOAD_SIZE);
>   	if (!item)
>   		return item;
>   
> -	memcpy(&item->iocb, pkt, sizeof(item->iocb));
> +	memcpy(&item->iocb, pkt, QLA_DEFAULT_PAYLOAD_SIZE);
>   	return item;
>   }
>   
> diff --git a/drivers/scsi/qla2xxx/qla_nvme.c b/drivers/scsi/qla2xxx/qla_nvme.c
> index 8ee2e337c9e1b..92488890bc04e 100644
> --- a/drivers/scsi/qla2xxx/qla_nvme.c
> +++ b/drivers/scsi/qla2xxx/qla_nvme.c
> @@ -1308,7 +1308,7 @@ void qla2xxx_process_purls_iocb(void **pkt, struct rsp_que **rsp)
>   
>   	ql_dbg(ql_dbg_unsol, vha, 0x2121,
>   	       "PURLS OP[%01x] size %d xchg addr 0x%x portid %06x\n",
> -	       item->iocb.iocb[3], item->size, uctx->exchange_address,
> +	       item->iocb[3], item->size, uctx->exchange_address,
>   	       fcport->d_id.b24);
>   	/* +48    0  1  2  3  4  5  6  7  8  9  A  B  C  D  E  F
>   	 * ----- -----------------------------------------------
> diff --git a/drivers/scsi/qla2xxx/qla_os.c b/drivers/scsi/qla2xxx/qla_os.c
> index d4b484c0fd9d7..ef8a4fce0e03c 100644
> --- a/drivers/scsi/qla2xxx/qla_os.c
> +++ b/drivers/scsi/qla2xxx/qla_os.c
> @@ -3971,7 +3971,7 @@ qla2x00_remove_one(struct pci_dev *pdev)
>   static inline void
>   qla24xx_free_purex_list(struct purex_list *list)
>   {
> -	struct purex_item *item, *next;
> +	struct purex_item_hdr *item, *next;
>   	ulong flags;
>   
>   	spin_lock_irqsave(&list->lock, flags);
> @@ -6459,9 +6459,10 @@ void qla24xx_process_purex_rdp(struct scsi_qla_host *vha,
>   void
>   qla24xx_free_purex_item(struct purex_item *item)
>   {
> -	if (item == &item->vha->default_item)
> -		memset(&item->vha->default_item, 0, sizeof(struct purex_item));
> -	else
> +	if (item == (struct purex_item *)&item->vha->default_item) {
> +		memset(&item->vha->default_item, 0, sizeof(struct purex_item_hdr));
> +		memset(&item->vha->__default_item_iocb, 0, QLA_DEFAULT_PAYLOAD_SIZE);
> +	} else
>   		kfree(item);
>   }
>   

Re: [PATCH v2 1/1] scsi: qla2xxx: replace non-standard flexible array purex_item.iocb

[Gustavo A. R. Silva]

>> --- a/drivers/scsi/qla2xxx/qla_isr.c
>> +++ b/drivers/scsi/qla2xxx/qla_isr.c
>> @@ -1077,17 +1077,17 @@ static struct purex_item *
>>   qla24xx_alloc_purex_item(scsi_qla_host_t *vha, uint16_t size)
>>   {
>>       struct purex_item *item = NULL;
>> -    uint8_t item_hdr_size = sizeof(*item);
>>       if (size > QLA_DEFAULT_PAYLOAD_SIZE) {
>> -        item = kzalloc(item_hdr_size +
>> -            (size - QLA_DEFAULT_PAYLOAD_SIZE), GFP_ATOMIC);
>> +        item = kzalloc(struct_size(item, iocb, size), GFP_ATOMIC);
> 
> With the inclusion of `counted_by`, I think `item->size` should be updated
> here:
>          item->size = size;
> 
>>       } else {
>>           if (atomic_inc_return(&vha->default_item.in_use) == 1) {
>> -            item = &vha->default_item;
>> +            item = (struct purex_item *)&vha->default_item;
>>               goto initialize_purex_header;
>>           } else {
>> -            item = kzalloc(item_hdr_size, GFP_ATOMIC);
>> +            item = kzalloc(
>> +                struct_size(item, iocb, QLA_DEFAULT_PAYLOAD_SIZE),
>> +                GFP_ATOMIC);
> 
> ...and here:
>              item->size = QLA_DEFAULT_PAYLOAD_SIZE;
> 
> Then remove `item->size = size;` just before `return item;`

... or probably set to zero `item->size = 0;` ?

Thanks
-Gustavo

Re: [PATCH v2 1/1] scsi: qla2xxx: replace non-standard flexible array purex_item.iocb

[Chris Leech]

On Mon, Jul 28, 2025 at 01:43:10PM -0600, Gustavo A. R. Silva wrote:
> On 28/07/25 12:57, Chris Leech wrote:
> > diff --git a/drivers/scsi/qla2xxx/qla_isr.c b/drivers/scsi/qla2xxx/qla_isr.c
> > index fe98c76e9be32..a00c06a9898ec 100644
> > --- a/drivers/scsi/qla2xxx/qla_isr.c
> > +++ b/drivers/scsi/qla2xxx/qla_isr.c
> > @@ -1077,17 +1077,17 @@ static struct purex_item *
> >   qla24xx_alloc_purex_item(scsi_qla_host_t *vha, uint16_t size)
> >   {
> >   	struct purex_item *item = NULL;
> > -	uint8_t item_hdr_size = sizeof(*item);
> >   	if (size > QLA_DEFAULT_PAYLOAD_SIZE) {
> > -		item = kzalloc(item_hdr_size +
> > -		    (size - QLA_DEFAULT_PAYLOAD_SIZE), GFP_ATOMIC);
> > +		item = kzalloc(struct_size(item, iocb, size), GFP_ATOMIC);
> 
> With the inclusion of `counted_by`, I think `item->size` should be updated
> here:
> 		item->size = size;
> 
> >   	} else {
> >   		if (atomic_inc_return(&vha->default_item.in_use) == 1) {
> > -			item = &vha->default_item;
> > +			item = (struct purex_item *)&vha->default_item;
> >   			goto initialize_purex_header;
> >   		} else {
> > -			item = kzalloc(item_hdr_size, GFP_ATOMIC);
> > +			item = kzalloc(
> > +				struct_size(item, iocb, QLA_DEFAULT_PAYLOAD_SIZE),
> > +				GFP_ATOMIC);
> 
> ...and here:
> 			item->size = QLA_DEFAULT_PAYLOAD_SIZE;
> 
> Then remove `item->size = size;` just before `return item;`

Hmm, I don't think I agree with that. The single assignment before
returning just keeps the allocation failure check in one place.
The conditional nesting in this function is a little odd, but I don't
want to start reworking it completly for this.

Is there a problem with the size referenced by counted_by potentially
being smaller than the allocation?  That looks possible in the case of
using the single pre-allocated default_item, but not needing to use the
entire 64-bytes (I don't know if that happens).

- Chris

Re: [PATCH v2 1/1] scsi: qla2xxx: replace non-standard flexible array purex_item.iocb

[Gustavo A. R. Silva]

On 28/07/25 15:15, Chris Leech wrote:
> On Mon, Jul 28, 2025 at 01:43:10PM -0600, Gustavo A. R. Silva wrote:
>> On 28/07/25 12:57, Chris Leech wrote:
>>> diff --git a/drivers/scsi/qla2xxx/qla_isr.c b/drivers/scsi/qla2xxx/qla_isr.c
>>> index fe98c76e9be32..a00c06a9898ec 100644
>>> --- a/drivers/scsi/qla2xxx/qla_isr.c
>>> +++ b/drivers/scsi/qla2xxx/qla_isr.c
>>> @@ -1077,17 +1077,17 @@ static struct purex_item *
>>>    qla24xx_alloc_purex_item(scsi_qla_host_t *vha, uint16_t size)
>>>    {
>>>    	struct purex_item *item = NULL;
>>> -	uint8_t item_hdr_size = sizeof(*item);
>>>    	if (size > QLA_DEFAULT_PAYLOAD_SIZE) {
>>> -		item = kzalloc(item_hdr_size +
>>> -		    (size - QLA_DEFAULT_PAYLOAD_SIZE), GFP_ATOMIC);
>>> +		item = kzalloc(struct_size(item, iocb, size), GFP_ATOMIC);
>>
>> With the inclusion of `counted_by`, I think `item->size` should be updated
>> here:
>> 		item->size = size;
>>
>>>    	} else {
>>>    		if (atomic_inc_return(&vha->default_item.in_use) == 1) {
>>> -			item = &vha->default_item;
>>> +			item = (struct purex_item *)&vha->default_item;
>>>    			goto initialize_purex_header;
>>>    		} else {
>>> -			item = kzalloc(item_hdr_size, GFP_ATOMIC);
>>> +			item = kzalloc(
>>> +				struct_size(item, iocb, QLA_DEFAULT_PAYLOAD_SIZE),
>>> +				GFP_ATOMIC);
>>
>> ...and here:
>> 			item->size = QLA_DEFAULT_PAYLOAD_SIZE;
>>
>> Then remove `item->size = size;` just before `return item;`
> 
> Hmm, I don't think I agree with that. The single assignment before
> returning just keeps the allocation failure check in one place.
> The conditional nesting in this function is a little odd, but I don't
> want to start reworking it completly for this.
> 
> Is there a problem with the size referenced by counted_by potentially
> being smaller than the allocation?  That looks possible in the case of
> using the single pre-allocated default_item, but not needing to use the
> entire 64-bytes (I don't know if that happens).

But if both allocations for `struct purex_item *item` fail, then
you'd end up with a flexible-array member of size 0, and `item->size`
potentially being greater than zero, since `default_item` doesn't
contain `uint8_t iocb[64];` anymore (it was turned into flex-array
member `uint8_t iocb[] __counted_by(size);`)... unless I'm missing
something?

Thanks
-Gustavo

Re: [PATCH v2 1/1] scsi: qla2xxx: replace non-standard flexible array purex_item.iocb

[Chris Leech]

On Mon, Jul 28, 2025 at 04:55:12PM -0600, Gustavo A. R. Silva wrote:
> 
> 
> On 28/07/25 15:15, Chris Leech wrote:
> > On Mon, Jul 28, 2025 at 01:43:10PM -0600, Gustavo A. R. Silva wrote:
> > > On 28/07/25 12:57, Chris Leech wrote:
> > > > diff --git a/drivers/scsi/qla2xxx/qla_isr.c b/drivers/scsi/qla2xxx/qla_isr.c
> > > > index fe98c76e9be32..a00c06a9898ec 100644
> > > > --- a/drivers/scsi/qla2xxx/qla_isr.c
> > > > +++ b/drivers/scsi/qla2xxx/qla_isr.c
> > > > @@ -1077,17 +1077,17 @@ static struct purex_item *
> > > >    qla24xx_alloc_purex_item(scsi_qla_host_t *vha, uint16_t size)
> > > >    {
> > > >    	struct purex_item *item = NULL;
> > > > -	uint8_t item_hdr_size = sizeof(*item);
> > > >    	if (size > QLA_DEFAULT_PAYLOAD_SIZE) {
> > > > -		item = kzalloc(item_hdr_size +
> > > > -		    (size - QLA_DEFAULT_PAYLOAD_SIZE), GFP_ATOMIC);
> > > > +		item = kzalloc(struct_size(item, iocb, size), GFP_ATOMIC);
> > > 
> > > With the inclusion of `counted_by`, I think `item->size` should be updated
> > > here:
> > > 		item->size = size;
> > > 
> > > >    	} else {
> > > >    		if (atomic_inc_return(&vha->default_item.in_use) == 1) {
> > > > -			item = &vha->default_item;
> > > > +			item = (struct purex_item *)&vha->default_item;
> > > >    			goto initialize_purex_header;
> > > >    		} else {
> > > > -			item = kzalloc(item_hdr_size, GFP_ATOMIC);
> > > > +			item = kzalloc(
> > > > +				struct_size(item, iocb, QLA_DEFAULT_PAYLOAD_SIZE),
> > > > +				GFP_ATOMIC);
> > > 
> > > ...and here:
> > > 			item->size = QLA_DEFAULT_PAYLOAD_SIZE;
> > > 
> > > Then remove `item->size = size;` just before `return item;`
> > 
> > Hmm, I don't think I agree with that. The single assignment before
> > returning just keeps the allocation failure check in one place.
> > The conditional nesting in this function is a little odd, but I don't
> > want to start reworking it completly for this.
> > 
> > Is there a problem with the size referenced by counted_by potentially
> > being smaller than the allocation?  That looks possible in the case of
> > using the single pre-allocated default_item, but not needing to use the
> > entire 64-bytes (I don't know if that happens).
> 
> But if both allocations for `struct purex_item *item` fail, then
> you'd end up with a flexible-array member of size 0, and `item->size`
> potentially being greater than zero, since `default_item` doesn't
> contain `uint8_t iocb[64];` anymore (it was turned into flex-array
> member `uint8_t iocb[] __counted_by(size);`)... unless I'm missing
> something?

This might be confusing to see as a diff; qla24xx_alloc_purex_item is
either allocating a new heap item with a flexible array size, or if the
size is small enough and there is only one in use at a time it's
returning default_item.

If the allocation fails, then the driver is already not using
default_item for this call and returns NULL.

  static struct purex_item *
  qla24xx_alloc_purex_item(scsi_qla_host_t *vha, uint16_t size)
  {
      struct purex_item *item = NULL;

      if (size > QLA_DEFAULT_PAYLOAD_SIZE) {
          item = kzalloc(struct_size(item, iocb, size), GFP_ATOMIC);
      } else {
          if (atomic_inc_return(&vha->default_item.in_use) == 1) {
              item = (struct purex_item *)&vha->default_item;
              goto initialize_purex_header;
          } else {
              item = kzalloc(
                  struct_size(item, iocb, QLA_DEFAULT_PAYLOAD_SIZE),
                  GFP_ATOMIC);
          }
      }
      if (!item) {
          ql_log(ql_log_warn, vha, 0x5092,
                 ">> Failed allocate purex list item.\n");

          return NULL;
      }

  initialize_purex_header:
      item->vha = vha;
      item->size = size;
      return item;
  }


default item was replaced with the header and a static sized array,
cast to a purex_item * when returned through the alloc function

  -       struct purex_item default_item;
  +       struct purex_item_hdr default_item;
  +       uint8_t __default_item_iocb[QLA_DEFAULT_PAYLOAD_SIZE];

- Chris

Re: [PATCH v2 1/1] scsi: qla2xxx: replace non-standard flexible array purex_item.iocb

[Gustavo A. R. Silva]

> default item was replaced with the header and a static sized array,
> cast to a purex_item * when returned through the alloc function
> 
>    -       struct purex_item default_item;
>    +       struct purex_item_hdr default_item;
>    +       uint8_t __default_item_iocb[QLA_DEFAULT_PAYLOAD_SIZE];

I see; yeah, this works as `(struct purex_item *)default_item->iocb`
and `__default_item_iocb` happen to be just perfectly aligned. However,
I wonder if there is any chance of this changing in the future...

Probably, a code comment stating that both `(struct purex_item *)default_item->iocb`
and `__default_item_iocb` are expected to share the same address would
likely be helpful for anyone modifying something around this code in
the future.

BTW, instead of directly casting to `struct purex_item *`, you could
use `container_of()` like this:

item = container_of(&vha->default_item, struct purex_item, __hdr);

but I guess that's up to maintainer's preference.

Thanks
-Gustavo

Re: [PATCH v2 1/1] scsi: qla2xxx: replace non-standard flexible array purex_item.iocb

[Gustavo A. R. Silva]

On 28/07/25 19:37, Gustavo A. R. Silva wrote:
> 
>> default item was replaced with the header and a static sized array,
>> cast to a purex_item * when returned through the alloc function
>>
>>    -       struct purex_item default_item;
>>    +       struct purex_item_hdr default_item;
>>    +       uint8_t __default_item_iocb[QLA_DEFAULT_PAYLOAD_SIZE];
> 
> I see; yeah, this works as `(struct purex_item *)default_item->iocb`
> and `__default_item_iocb` happen to be just perfectly aligned. However,
> I wonder if there is any chance of this changing in the future...
> 
> Probably, a code comment stating that both `(struct purex_item *)default_item->iocb`
> and `__default_item_iocb` are expected to share the same address would
> likely be helpful for anyone modifying something around this code in
> the future.
> 
> BTW, instead of directly casting to `struct purex_item *`, you could
> use `container_of()` like this:
> 
> item = container_of(&vha->default_item, struct purex_item, __hdr);
> 
> but I guess that's up to maintainer's preference.
> 

I just noticed that the new TRAILING_OVERLAP() helper just landed in
Linus' tree, and this issue seems to be a perfect candidate for it.

The (untested) patch below avoids the use of `struct_group_tagged()`
and the casts to `struct purex_item *`:

diff --git a/drivers/scsi/qla2xxx/qla_def.h b/drivers/scsi/qla2xxx/qla_def.h
index cb95b7b12051..4bdf8adf04ed 100644
--- a/drivers/scsi/qla2xxx/qla_def.h
+++ b/drivers/scsi/qla2xxx/qla_def.h
@@ -4890,9 +4890,7 @@ struct purex_item {
                              struct purex_item *pkt);
         atomic_t in_use;
         uint16_t size;
-       struct {
-               uint8_t iocb[64];
-       } iocb;
+       uint8_t iocb[] __counted_by(size);
  };

  #include "qla_edif.h"
@@ -5101,7 +5099,6 @@ typedef struct scsi_qla_host {
                 struct list_head head;
                 spinlock_t lock;
         } purex_list;
-       struct purex_item default_item;

         struct name_list_extended gnl;
         /* Count of active session/fcport */
@@ -5130,6 +5127,10 @@ typedef struct scsi_qla_host {
  #define DPORT_DIAG_IN_PROGRESS                 BIT_0
  #define DPORT_DIAG_CHIP_RESET_IN_PROGRESS      BIT_1
         uint16_t dport_status;
+
+       TRAILING_OVERLAP(struct purex_item, default_item, iocb,
+               uint8_t __default_item_iocb[QLA_DEFAULT_PAYLOAD_SIZE];
+       );
  } scsi_qla_host_t;

  struct qla27xx_image_status {
diff --git a/drivers/scsi/qla2xxx/qla_isr.c b/drivers/scsi/qla2xxx/qla_isr.c
index c4c6b5c6658c..4559b490614d 100644
--- a/drivers/scsi/qla2xxx/qla_isr.c
+++ b/drivers/scsi/qla2xxx/qla_isr.c
@@ -1077,17 +1077,17 @@ static struct purex_item *
  qla24xx_alloc_purex_item(scsi_qla_host_t *vha, uint16_t size)
  {
         struct purex_item *item = NULL;
-       uint8_t item_hdr_size = sizeof(*item);

         if (size > QLA_DEFAULT_PAYLOAD_SIZE) {
-               item = kzalloc(item_hdr_size +
-                   (size - QLA_DEFAULT_PAYLOAD_SIZE), GFP_ATOMIC);
+               item = kzalloc(struct_size(item, iocb, size), GFP_ATOMIC);
         } else {
                 if (atomic_inc_return(&vha->default_item.in_use) == 1) {
                         item = &vha->default_item;
                         goto initialize_purex_header;
                 } else {
-                       item = kzalloc(item_hdr_size, GFP_ATOMIC);
+                       item = kzalloc(
+                               struct_size(item, iocb, QLA_DEFAULT_PAYLOAD_SIZE),
+                               GFP_ATOMIC);
                 }
         }
         if (!item) {
@@ -1127,17 +1127,16 @@ qla24xx_queue_purex_item(scsi_qla_host_t *vha, struct purex_item *pkt,
   * @vha: SCSI driver HA context
   * @pkt: ELS packet
   */
-static struct purex_item
-*qla24xx_copy_std_pkt(struct scsi_qla_host *vha, void *pkt)
+static struct purex_item *
+qla24xx_copy_std_pkt(struct scsi_qla_host *vha, void *pkt)
  {
         struct purex_item *item;

-       item = qla24xx_alloc_purex_item(vha,
-                                       QLA_DEFAULT_PAYLOAD_SIZE);
+       item = qla24xx_alloc_purex_item(vha, QLA_DEFAULT_PAYLOAD_SIZE);
         if (!item)
                 return item;

-       memcpy(&item->iocb, pkt, sizeof(item->iocb));
+       memcpy(&item->iocb, pkt, QLA_DEFAULT_PAYLOAD_SIZE);
         return item;
  }

diff --git a/drivers/scsi/qla2xxx/qla_nvme.c b/drivers/scsi/qla2xxx/qla_nvme.c
index 8ee2e337c9e1..92488890bc04 100644
--- a/drivers/scsi/qla2xxx/qla_nvme.c
+++ b/drivers/scsi/qla2xxx/qla_nvme.c
@@ -1308,7 +1308,7 @@ void qla2xxx_process_purls_iocb(void **pkt, struct rsp_que **rsp)

         ql_dbg(ql_dbg_unsol, vha, 0x2121,
                "PURLS OP[%01x] size %d xchg addr 0x%x portid %06x\n",
-              item->iocb.iocb[3], item->size, uctx->exchange_address,
+              item->iocb[3], item->size, uctx->exchange_address,
                fcport->d_id.b24);
         /* +48    0  1  2  3  4  5  6  7  8  9  A  B  C  D  E  F
          * ----- -----------------------------------------------
diff --git a/drivers/scsi/qla2xxx/qla_os.c b/drivers/scsi/qla2xxx/qla_os.c
index d4b484c0fd9d..253f802605d6 100644
--- a/drivers/scsi/qla2xxx/qla_os.c
+++ b/drivers/scsi/qla2xxx/qla_os.c
@@ -6459,9 +6459,10 @@ void qla24xx_process_purex_rdp(struct scsi_qla_host *vha,
  void
  qla24xx_free_purex_item(struct purex_item *item)
  {
-       if (item == &item->vha->default_item)
+       if (item == &item->vha->default_item) {
                 memset(&item->vha->default_item, 0, sizeof(struct purex_item));
-       else
+               memset(&item->vha->__default_item_iocb, 0, QLA_DEFAULT_PAYLOAD_SIZE);
+       } else
                 kfree(item);

Thanks
-Gustavo

Re: [PATCH v2 1/1] scsi: qla2xxx: replace non-standard flexible array purex_item.iocb

[Chris Leech]

On Mon, Jul 28, 2025 at 08:20:12PM -0600, Gustavo A. R. Silva wrote:
> 
> I just noticed that the new TRAILING_OVERLAP() helper just landed in
> Linus' tree, and this issue seems to be a perfect candidate for it.
> 
> The (untested) patch below avoids the use of `struct_group_tagged()`
> and the casts to `struct purex_item *`:

(I was largely basing my use of struct_group on your blog post from a
few month back, but now there's another new macro!)

Yes, that looks like it would work as long as the driver maintainer
doesn't object to moving that field around in scsi_qla_host.

I'm getting ready to be away from the computer for a week, so I have to
leave this for now. Maybe Nilesh has an opinion?

- Chris

Re: [PATCH v2 1/1] scsi: qla2xxx: replace non-standard flexible array purex_item.iocb

[Gustavo A. R. Silva]

On 29/07/25 18:04, Chris Leech wrote:
> On Mon, Jul 28, 2025 at 08:20:12PM -0600, Gustavo A. R. Silva wrote:
>>
>> I just noticed that the new TRAILING_OVERLAP() helper just landed in
>> Linus' tree, and this issue seems to be a perfect candidate for it.
>>
>> The (untested) patch below avoids the use of `struct_group_tagged()`
>> and the casts to `struct purex_item *`:
> 
> (I was largely basing my use of struct_group on your blog post from a
> few month back, but now there's another new macro!)

Yep, the kernel is constantly evolving. :)

I'm actually writing a follow up post (and a presentation for OSSEU)
where I'll share updates on enabling -Wflex-array-member-not-at-end,
including details about this new helper.

> 
> Yes, that looks like it would work as long as the driver maintainer
> doesn't object to moving that field around in scsi_qla_host.
> 
> I'm getting ready to be away from the computer for a week, so I have to
> leave this for now. Maybe Nilesh has an opinion?

No worries, I can take it up from here.

Thanks!
-Gustavo

Re: [PATCH v2 1/1] scsi: qla2xxx: replace non-standard flexible array purex_item.iocb

[Gustavo A. R. Silva]

Hi Bryan,

I wonder if you could run your tests on the patch below and let me
know if it passes. If it does, I'll go ahead and submit it as a
proper patch (including your Tested-by tag) to the SCSI list.

Thank you!
-Gustavo

> 
> The (untested) patch below avoids the use of `struct_group_tagged()`
> and the casts to `struct purex_item *`:
> 
> diff --git a/drivers/scsi/qla2xxx/qla_def.h b/drivers/scsi/qla2xxx/qla_def.h
> index cb95b7b12051..4bdf8adf04ed 100644
> --- a/drivers/scsi/qla2xxx/qla_def.h
> +++ b/drivers/scsi/qla2xxx/qla_def.h
> @@ -4890,9 +4890,7 @@ struct purex_item {
>                               struct purex_item *pkt);
>          atomic_t in_use;
>          uint16_t size;
> -       struct {
> -               uint8_t iocb[64];
> -       } iocb;
> +       uint8_t iocb[] __counted_by(size);
>   };
> 
>   #include "qla_edif.h"
> @@ -5101,7 +5099,6 @@ typedef struct scsi_qla_host {
>                  struct list_head head;
>                  spinlock_t lock;
>          } purex_list;
> -       struct purex_item default_item;
> 
>          struct name_list_extended gnl;
>          /* Count of active session/fcport */
> @@ -5130,6 +5127,10 @@ typedef struct scsi_qla_host {
>   #define DPORT_DIAG_IN_PROGRESS                 BIT_0
>   #define DPORT_DIAG_CHIP_RESET_IN_PROGRESS      BIT_1
>          uint16_t dport_status;
> +
> +       TRAILING_OVERLAP(struct purex_item, default_item, iocb,
> +               uint8_t __default_item_iocb[QLA_DEFAULT_PAYLOAD_SIZE];
> +       );
>   } scsi_qla_host_t;
> 
>   struct qla27xx_image_status {
> diff --git a/drivers/scsi/qla2xxx/qla_isr.c b/drivers/scsi/qla2xxx/qla_isr.c
> index c4c6b5c6658c..4559b490614d 100644
> --- a/drivers/scsi/qla2xxx/qla_isr.c
> +++ b/drivers/scsi/qla2xxx/qla_isr.c
> @@ -1077,17 +1077,17 @@ static struct purex_item *
>   qla24xx_alloc_purex_item(scsi_qla_host_t *vha, uint16_t size)
>   {
>          struct purex_item *item = NULL;
> -       uint8_t item_hdr_size = sizeof(*item);
> 
>          if (size > QLA_DEFAULT_PAYLOAD_SIZE) {
> -               item = kzalloc(item_hdr_size +
> -                   (size - QLA_DEFAULT_PAYLOAD_SIZE), GFP_ATOMIC);
> +               item = kzalloc(struct_size(item, iocb, size), GFP_ATOMIC);
>          } else {
>                  if (atomic_inc_return(&vha->default_item.in_use) == 1) {
>                          item = &vha->default_item;
>                          goto initialize_purex_header;
>                  } else {
> -                       item = kzalloc(item_hdr_size, GFP_ATOMIC);
> +                       item = kzalloc(
> +                               struct_size(item, iocb, QLA_DEFAULT_PAYLOAD_SIZE),
> +                               GFP_ATOMIC);
>                  }
>          }
>          if (!item) {
> @@ -1127,17 +1127,16 @@ qla24xx_queue_purex_item(scsi_qla_host_t *vha, struct purex_item *pkt,
>    * @vha: SCSI driver HA context
>    * @pkt: ELS packet
>    */
> -static struct purex_item
> -*qla24xx_copy_std_pkt(struct scsi_qla_host *vha, void *pkt)
> +static struct purex_item *
> +qla24xx_copy_std_pkt(struct scsi_qla_host *vha, void *pkt)
>   {
>          struct purex_item *item;
> 
> -       item = qla24xx_alloc_purex_item(vha,
> -                                       QLA_DEFAULT_PAYLOAD_SIZE);
> +       item = qla24xx_alloc_purex_item(vha, QLA_DEFAULT_PAYLOAD_SIZE);
>          if (!item)
>                  return item;
> 
> -       memcpy(&item->iocb, pkt, sizeof(item->iocb));
> +       memcpy(&item->iocb, pkt, QLA_DEFAULT_PAYLOAD_SIZE);
>          return item;
>   }
> 
> diff --git a/drivers/scsi/qla2xxx/qla_nvme.c b/drivers/scsi/qla2xxx/qla_nvme.c
> index 8ee2e337c9e1..92488890bc04 100644
> --- a/drivers/scsi/qla2xxx/qla_nvme.c
> +++ b/drivers/scsi/qla2xxx/qla_nvme.c
> @@ -1308,7 +1308,7 @@ void qla2xxx_process_purls_iocb(void **pkt, struct rsp_que **rsp)
> 
>          ql_dbg(ql_dbg_unsol, vha, 0x2121,
>                 "PURLS OP[%01x] size %d xchg addr 0x%x portid %06x\n",
> -              item->iocb.iocb[3], item->size, uctx->exchange_address,
> +              item->iocb[3], item->size, uctx->exchange_address,
>                 fcport->d_id.b24);
>          /* +48    0  1  2  3  4  5  6  7  8  9  A  B  C  D  E  F
>           * ----- -----------------------------------------------
> diff --git a/drivers/scsi/qla2xxx/qla_os.c b/drivers/scsi/qla2xxx/qla_os.c
> index d4b484c0fd9d..253f802605d6 100644
> --- a/drivers/scsi/qla2xxx/qla_os.c
> +++ b/drivers/scsi/qla2xxx/qla_os.c
> @@ -6459,9 +6459,10 @@ void qla24xx_process_purex_rdp(struct scsi_qla_host *vha,
>   void
>   qla24xx_free_purex_item(struct purex_item *item)
>   {
> -       if (item == &item->vha->default_item)
> +       if (item == &item->vha->default_item) {
>                  memset(&item->vha->default_item, 0, sizeof(struct purex_item));
> -       else
> +               memset(&item->vha->__default_item_iocb, 0, QLA_DEFAULT_PAYLOAD_SIZE);
> +       } else
>                  kfree(item);
> 
> Thanks
> -Gustavo

Re: [PATCH v2 1/1] scsi: qla2xxx: replace non-standard flexible array purex_item.iocb

[Bryan Gurney]

Hi Gustavo,

Yes, it passes.  When I test this on top of the NVMe FPIN link
integrity v9 patchset, I only see the "kernel: qla2xxx... : FPIN ELS"
event.

Tested-by: Bryan Gurney <bgurney@redhat.com>


Thanks,

Bryan

On Tue, Jul 29, 2025 at 11:45 PM Gustavo A. R. Silva
<gustavo@embeddedor.com> wrote:
>
> Hi Bryan,
>
> I wonder if you could run your tests on the patch below and let me
> know if it passes. If it does, I'll go ahead and submit it as a
> proper patch (including your Tested-by tag) to the SCSI list.
>
> Thank you!
> -Gustavo
>
> >
> > The (untested) patch below avoids the use of `struct_group_tagged()`
> > and the casts to `struct purex_item *`:
> >
> > diff --git a/drivers/scsi/qla2xxx/qla_def.h b/drivers/scsi/qla2xxx/qla_def.h
> > index cb95b7b12051..4bdf8adf04ed 100644
> > --- a/drivers/scsi/qla2xxx/qla_def.h
> > +++ b/drivers/scsi/qla2xxx/qla_def.h
> > @@ -4890,9 +4890,7 @@ struct purex_item {
> >                               struct purex_item *pkt);
> >          atomic_t in_use;
> >          uint16_t size;
> > -       struct {
> > -               uint8_t iocb[64];
> > -       } iocb;
> > +       uint8_t iocb[] __counted_by(size);
> >   };
> >
> >   #include "qla_edif.h"
> > @@ -5101,7 +5099,6 @@ typedef struct scsi_qla_host {
> >                  struct list_head head;
> >                  spinlock_t lock;
> >          } purex_list;
> > -       struct purex_item default_item;
> >
> >          struct name_list_extended gnl;
> >          /* Count of active session/fcport */
> > @@ -5130,6 +5127,10 @@ typedef struct scsi_qla_host {
> >   #define DPORT_DIAG_IN_PROGRESS                 BIT_0
> >   #define DPORT_DIAG_CHIP_RESET_IN_PROGRESS      BIT_1
> >          uint16_t dport_status;
> > +
> > +       TRAILING_OVERLAP(struct purex_item, default_item, iocb,
> > +               uint8_t __default_item_iocb[QLA_DEFAULT_PAYLOAD_SIZE];
> > +       );
> >   } scsi_qla_host_t;
> >
> >   struct qla27xx_image_status {
> > diff --git a/drivers/scsi/qla2xxx/qla_isr.c b/drivers/scsi/qla2xxx/qla_isr.c
> > index c4c6b5c6658c..4559b490614d 100644
> > --- a/drivers/scsi/qla2xxx/qla_isr.c
> > +++ b/drivers/scsi/qla2xxx/qla_isr.c
> > @@ -1077,17 +1077,17 @@ static struct purex_item *
> >   qla24xx_alloc_purex_item(scsi_qla_host_t *vha, uint16_t size)
> >   {
> >          struct purex_item *item = NULL;
> > -       uint8_t item_hdr_size = sizeof(*item);
> >
> >          if (size > QLA_DEFAULT_PAYLOAD_SIZE) {
> > -               item = kzalloc(item_hdr_size +
> > -                   (size - QLA_DEFAULT_PAYLOAD_SIZE), GFP_ATOMIC);
> > +               item = kzalloc(struct_size(item, iocb, size), GFP_ATOMIC);
> >          } else {
> >                  if (atomic_inc_return(&vha->default_item.in_use) == 1) {
> >                          item = &vha->default_item;
> >                          goto initialize_purex_header;
> >                  } else {
> > -                       item = kzalloc(item_hdr_size, GFP_ATOMIC);
> > +                       item = kzalloc(
> > +                               struct_size(item, iocb, QLA_DEFAULT_PAYLOAD_SIZE),
> > +                               GFP_ATOMIC);
> >                  }
> >          }
> >          if (!item) {
> > @@ -1127,17 +1127,16 @@ qla24xx_queue_purex_item(scsi_qla_host_t *vha, struct purex_item *pkt,
> >    * @vha: SCSI driver HA context
> >    * @pkt: ELS packet
> >    */
> > -static struct purex_item
> > -*qla24xx_copy_std_pkt(struct scsi_qla_host *vha, void *pkt)
> > +static struct purex_item *
> > +qla24xx_copy_std_pkt(struct scsi_qla_host *vha, void *pkt)
> >   {
> >          struct purex_item *item;
> >
> > -       item = qla24xx_alloc_purex_item(vha,
> > -                                       QLA_DEFAULT_PAYLOAD_SIZE);
> > +       item = qla24xx_alloc_purex_item(vha, QLA_DEFAULT_PAYLOAD_SIZE);
> >          if (!item)
> >                  return item;
> >
> > -       memcpy(&item->iocb, pkt, sizeof(item->iocb));
> > +       memcpy(&item->iocb, pkt, QLA_DEFAULT_PAYLOAD_SIZE);
> >          return item;
> >   }
> >
> > diff --git a/drivers/scsi/qla2xxx/qla_nvme.c b/drivers/scsi/qla2xxx/qla_nvme.c
> > index 8ee2e337c9e1..92488890bc04 100644
> > --- a/drivers/scsi/qla2xxx/qla_nvme.c
> > +++ b/drivers/scsi/qla2xxx/qla_nvme.c
> > @@ -1308,7 +1308,7 @@ void qla2xxx_process_purls_iocb(void **pkt, struct rsp_que **rsp)
> >
> >          ql_dbg(ql_dbg_unsol, vha, 0x2121,
> >                 "PURLS OP[%01x] size %d xchg addr 0x%x portid %06x\n",
> > -              item->iocb.iocb[3], item->size, uctx->exchange_address,
> > +              item->iocb[3], item->size, uctx->exchange_address,
> >                 fcport->d_id.b24);
> >          /* +48    0  1  2  3  4  5  6  7  8  9  A  B  C  D  E  F
> >           * ----- -----------------------------------------------
> > diff --git a/drivers/scsi/qla2xxx/qla_os.c b/drivers/scsi/qla2xxx/qla_os.c
> > index d4b484c0fd9d..253f802605d6 100644
> > --- a/drivers/scsi/qla2xxx/qla_os.c
> > +++ b/drivers/scsi/qla2xxx/qla_os.c
> > @@ -6459,9 +6459,10 @@ void qla24xx_process_purex_rdp(struct scsi_qla_host *vha,
> >   void
> >   qla24xx_free_purex_item(struct purex_item *item)
> >   {
> > -       if (item == &item->vha->default_item)
> > +       if (item == &item->vha->default_item) {
> >                  memset(&item->vha->default_item, 0, sizeof(struct purex_item));
> > -       else
> > +               memset(&item->vha->__default_item_iocb, 0, QLA_DEFAULT_PAYLOAD_SIZE);
> > +       } else
> >                  kfree(item);
> >
> > Thanks
> > -Gustavo
>

Re: [PATCH v2 1/1] scsi: qla2xxx: replace non-standard flexible array purex_item.iocb

[Gustavo A. R. Silva]

On 30/07/25 09:43, Bryan Gurney wrote:
> Hi Gustavo,
> 
> Yes, it passes.  When I test this on top of the NVMe FPIN link
> integrity v9 patchset, I only see the "kernel: qla2xxx... : FPIN ELS"
> event.
> 
> Tested-by: Bryan Gurney <bgurney@redhat.com>

Awesome. :)

Thanks!
-Gustavo

> 
> 
> Thanks,
> 
> Bryan
> 
> On Tue, Jul 29, 2025 at 11:45 PM Gustavo A. R. Silva
> <gustavo@embeddedor.com> wrote:
>>
>> Hi Bryan,
>>
>> I wonder if you could run your tests on the patch below and let me
>> know if it passes. If it does, I'll go ahead and submit it as a
>> proper patch (including your Tested-by tag) to the SCSI list.
>>
>> Thank you!
>> -Gustavo
>>
>>>
>>> The (untested) patch below avoids the use of `struct_group_tagged()`
>>> and the casts to `struct purex_item *`:
>>>
>>> diff --git a/drivers/scsi/qla2xxx/qla_def.h b/drivers/scsi/qla2xxx/qla_def.h
>>> index cb95b7b12051..4bdf8adf04ed 100644
>>> --- a/drivers/scsi/qla2xxx/qla_def.h
>>> +++ b/drivers/scsi/qla2xxx/qla_def.h
>>> @@ -4890,9 +4890,7 @@ struct purex_item {
>>>                                struct purex_item *pkt);
>>>           atomic_t in_use;
>>>           uint16_t size;
>>> -       struct {
>>> -               uint8_t iocb[64];
>>> -       } iocb;
>>> +       uint8_t iocb[] __counted_by(size);
>>>    };
>>>
>>>    #include "qla_edif.h"
>>> @@ -5101,7 +5099,6 @@ typedef struct scsi_qla_host {
>>>                   struct list_head head;
>>>                   spinlock_t lock;
>>>           } purex_list;
>>> -       struct purex_item default_item;
>>>
>>>           struct name_list_extended gnl;
>>>           /* Count of active session/fcport */
>>> @@ -5130,6 +5127,10 @@ typedef struct scsi_qla_host {
>>>    #define DPORT_DIAG_IN_PROGRESS                 BIT_0
>>>    #define DPORT_DIAG_CHIP_RESET_IN_PROGRESS      BIT_1
>>>           uint16_t dport_status;
>>> +
>>> +       TRAILING_OVERLAP(struct purex_item, default_item, iocb,
>>> +               uint8_t __default_item_iocb[QLA_DEFAULT_PAYLOAD_SIZE];
>>> +       );
>>>    } scsi_qla_host_t;
>>>
>>>    struct qla27xx_image_status {
>>> diff --git a/drivers/scsi/qla2xxx/qla_isr.c b/drivers/scsi/qla2xxx/qla_isr.c
>>> index c4c6b5c6658c..4559b490614d 100644
>>> --- a/drivers/scsi/qla2xxx/qla_isr.c
>>> +++ b/drivers/scsi/qla2xxx/qla_isr.c
>>> @@ -1077,17 +1077,17 @@ static struct purex_item *
>>>    qla24xx_alloc_purex_item(scsi_qla_host_t *vha, uint16_t size)
>>>    {
>>>           struct purex_item *item = NULL;
>>> -       uint8_t item_hdr_size = sizeof(*item);
>>>
>>>           if (size > QLA_DEFAULT_PAYLOAD_SIZE) {
>>> -               item = kzalloc(item_hdr_size +
>>> -                   (size - QLA_DEFAULT_PAYLOAD_SIZE), GFP_ATOMIC);
>>> +               item = kzalloc(struct_size(item, iocb, size), GFP_ATOMIC);
>>>           } else {
>>>                   if (atomic_inc_return(&vha->default_item.in_use) == 1) {
>>>                           item = &vha->default_item;
>>>                           goto initialize_purex_header;
>>>                   } else {
>>> -                       item = kzalloc(item_hdr_size, GFP_ATOMIC);
>>> +                       item = kzalloc(
>>> +                               struct_size(item, iocb, QLA_DEFAULT_PAYLOAD_SIZE),
>>> +                               GFP_ATOMIC);
>>>                   }
>>>           }
>>>           if (!item) {
>>> @@ -1127,17 +1127,16 @@ qla24xx_queue_purex_item(scsi_qla_host_t *vha, struct purex_item *pkt,
>>>     * @vha: SCSI driver HA context
>>>     * @pkt: ELS packet
>>>     */
>>> -static struct purex_item
>>> -*qla24xx_copy_std_pkt(struct scsi_qla_host *vha, void *pkt)
>>> +static struct purex_item *
>>> +qla24xx_copy_std_pkt(struct scsi_qla_host *vha, void *pkt)
>>>    {
>>>           struct purex_item *item;
>>>
>>> -       item = qla24xx_alloc_purex_item(vha,
>>> -                                       QLA_DEFAULT_PAYLOAD_SIZE);
>>> +       item = qla24xx_alloc_purex_item(vha, QLA_DEFAULT_PAYLOAD_SIZE);
>>>           if (!item)
>>>                   return item;
>>>
>>> -       memcpy(&item->iocb, pkt, sizeof(item->iocb));
>>> +       memcpy(&item->iocb, pkt, QLA_DEFAULT_PAYLOAD_SIZE);
>>>           return item;
>>>    }
>>>
>>> diff --git a/drivers/scsi/qla2xxx/qla_nvme.c b/drivers/scsi/qla2xxx/qla_nvme.c
>>> index 8ee2e337c9e1..92488890bc04 100644
>>> --- a/drivers/scsi/qla2xxx/qla_nvme.c
>>> +++ b/drivers/scsi/qla2xxx/qla_nvme.c
>>> @@ -1308,7 +1308,7 @@ void qla2xxx_process_purls_iocb(void **pkt, struct rsp_que **rsp)
>>>
>>>           ql_dbg(ql_dbg_unsol, vha, 0x2121,
>>>                  "PURLS OP[%01x] size %d xchg addr 0x%x portid %06x\n",
>>> -              item->iocb.iocb[3], item->size, uctx->exchange_address,
>>> +              item->iocb[3], item->size, uctx->exchange_address,
>>>                  fcport->d_id.b24);
>>>           /* +48    0  1  2  3  4  5  6  7  8  9  A  B  C  D  E  F
>>>            * ----- -----------------------------------------------
>>> diff --git a/drivers/scsi/qla2xxx/qla_os.c b/drivers/scsi/qla2xxx/qla_os.c
>>> index d4b484c0fd9d..253f802605d6 100644
>>> --- a/drivers/scsi/qla2xxx/qla_os.c
>>> +++ b/drivers/scsi/qla2xxx/qla_os.c
>>> @@ -6459,9 +6459,10 @@ void qla24xx_process_purex_rdp(struct scsi_qla_host *vha,
>>>    void
>>>    qla24xx_free_purex_item(struct purex_item *item)
>>>    {
>>> -       if (item == &item->vha->default_item)
>>> +       if (item == &item->vha->default_item) {
>>>                   memset(&item->vha->default_item, 0, sizeof(struct purex_item));
>>> -       else
>>> +               memset(&item->vha->__default_item_iocb, 0, QLA_DEFAULT_PAYLOAD_SIZE);
>>> +       } else
>>>                   kfree(item);
>>>
>>> Thanks
>>> -Gustavo
>>
> 

[PATCH 2/2] scsi: qla2xxx: unwrap purex_item.iocb.iocb so that __counted_by can be used

[Chris Leech]

Remove the outer wrapper from the iocb flex array, so that it can be
linked to purex_item.size with __counted_by.

This removes the default minimum 64-byte allocation, requiring further
changes.

  In struct scsi_qla_host embedded default_item is now followed by
  __default_item_iocb[QLA_DEFAULT_PAYLOAD_SIZE] to reserve space that
  will be used as default_item.iocb

  qla24xx_alloc_purex_item is adjusted to no longer expect the default
  minimum size to be part of sizeof(struct purex_item), the entire
  flexible array size is added to the structure size for allocation.

Tested-by: Bryan Gurney <bgurney@redhat.com>
Signed-off-by: Chris Leech <cleech@redhat.com>
---
 drivers/scsi/qla2xxx/qla_def.h  |  6 ++----
 drivers/scsi/qla2xxx/qla_isr.c  | 15 +++++++--------
 drivers/scsi/qla2xxx/qla_nvme.c |  2 +-
 drivers/scsi/qla2xxx/qla_os.c   |  5 +++--
 4 files changed, 13 insertions(+), 15 deletions(-)

diff --git a/drivers/scsi/qla2xxx/qla_def.h b/drivers/scsi/qla2xxx/qla_def.h
index 6237fefeca149..0a79ef2429f50 100644
--- a/drivers/scsi/qla2xxx/qla_def.h
+++ b/drivers/scsi/qla2xxx/qla_def.h
@@ -4890,10 +4890,7 @@ struct purex_item {
 			     struct purex_item *pkt);
 	atomic_t in_use;
 	uint16_t size;
-	union {
-		uint8_t __padding[QLA_DEFAULT_PAYLOAD_SIZE];
-		DECLARE_FLEX_ARRAY(uint8_t, iocb);
-	} iocb;
+	uint8_t iocb[] __counted_by(size);
 };
 
 #include "qla_edif.h"
@@ -5103,6 +5100,7 @@ typedef struct scsi_qla_host {
 		spinlock_t lock;
 	} purex_list;
 	struct purex_item default_item;
+	uint8_t __default_item_iocb[QLA_DEFAULT_PAYLOAD_SIZE];
 
 	struct name_list_extended gnl;
 	/* Count of active session/fcport */
diff --git a/drivers/scsi/qla2xxx/qla_isr.c b/drivers/scsi/qla2xxx/qla_isr.c
index fe98c76e9be32..83677f74fd8e6 100644
--- a/drivers/scsi/qla2xxx/qla_isr.c
+++ b/drivers/scsi/qla2xxx/qla_isr.c
@@ -1080,14 +1080,14 @@ qla24xx_alloc_purex_item(scsi_qla_host_t *vha, uint16_t size)
 	uint8_t item_hdr_size = sizeof(*item);
 
 	if (size > QLA_DEFAULT_PAYLOAD_SIZE) {
-		item = kzalloc(item_hdr_size +
-		    (size - QLA_DEFAULT_PAYLOAD_SIZE), GFP_ATOMIC);
+		item = kzalloc(item_hdr_size + size, GFP_ATOMIC);
 	} else {
 		if (atomic_inc_return(&vha->default_item.in_use) == 1) {
 			item = &vha->default_item;
 			goto initialize_purex_header;
 		} else {
-			item = kzalloc(item_hdr_size, GFP_ATOMIC);
+			item = kzalloc(item_hdr_size + QLA_DEFAULT_PAYLOAD_SIZE,
+					GFP_ATOMIC);
 		}
 	}
 	if (!item) {
@@ -1127,17 +1127,16 @@ qla24xx_queue_purex_item(scsi_qla_host_t *vha, struct purex_item *pkt,
  * @vha: SCSI driver HA context
  * @pkt: ELS packet
  */
-static struct purex_item
-*qla24xx_copy_std_pkt(struct scsi_qla_host *vha, void *pkt)
+static struct purex_item *
+qla24xx_copy_std_pkt(struct scsi_qla_host *vha, void *pkt)
 {
 	struct purex_item *item;
 
-	item = qla24xx_alloc_purex_item(vha,
-					QLA_DEFAULT_PAYLOAD_SIZE);
+	item = qla24xx_alloc_purex_item(vha, QLA_DEFAULT_PAYLOAD_SIZE);
 	if (!item)
 		return item;
 
-	memcpy(&item->iocb, pkt, sizeof(item->iocb));
+	memcpy(&item->iocb, pkt, QLA_DEFAULT_PAYLOAD_SIZE);
 	return item;
 }
 
diff --git a/drivers/scsi/qla2xxx/qla_nvme.c b/drivers/scsi/qla2xxx/qla_nvme.c
index 8ee2e337c9e1b..92488890bc04e 100644
--- a/drivers/scsi/qla2xxx/qla_nvme.c
+++ b/drivers/scsi/qla2xxx/qla_nvme.c
@@ -1308,7 +1308,7 @@ void qla2xxx_process_purls_iocb(void **pkt, struct rsp_que **rsp)
 
 	ql_dbg(ql_dbg_unsol, vha, 0x2121,
 	       "PURLS OP[%01x] size %d xchg addr 0x%x portid %06x\n",
-	       item->iocb.iocb[3], item->size, uctx->exchange_address,
+	       item->iocb[3], item->size, uctx->exchange_address,
 	       fcport->d_id.b24);
 	/* +48    0  1  2  3  4  5  6  7  8  9  A  B  C  D  E  F
 	 * ----- -----------------------------------------------
diff --git a/drivers/scsi/qla2xxx/qla_os.c b/drivers/scsi/qla2xxx/qla_os.c
index d4b484c0fd9d7..253f802605d63 100644
--- a/drivers/scsi/qla2xxx/qla_os.c
+++ b/drivers/scsi/qla2xxx/qla_os.c
@@ -6459,9 +6459,10 @@ void qla24xx_process_purex_rdp(struct scsi_qla_host *vha,
 void
 qla24xx_free_purex_item(struct purex_item *item)
 {
-	if (item == &item->vha->default_item)
+	if (item == &item->vha->default_item) {
 		memset(&item->vha->default_item, 0, sizeof(struct purex_item));
-	else
+		memset(&item->vha->__default_item_iocb, 0, QLA_DEFAULT_PAYLOAD_SIZE);
+	} else
 		kfree(item);
 }
 
-- 
2.50.1

Re: [PATCH 2/2] scsi: qla2xxx: unwrap purex_item.iocb.iocb so that __counted_by can be used

[Kees Cook]

On Fri, Jul 25, 2025 at 02:27:32PM -0700, Chris Leech wrote:
> Remove the outer wrapper from the iocb flex array, so that it can be
> linked to purex_item.size with __counted_by.

Oh nevermind about my suggestion, this is WAY better. Perhaps better to
just squash these two patches together?

-Kees

-- 
Kees Cook