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

Chris Leech
2025-07-28T21:15:18Z

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