Re: [PATCH v4 9/9] scsi/scsi_bus: fix races in REPORT LUNS

[Stefan Hajnoczi]

On Mon, Aug 31, 2020 at 06:01:24PM +0300, Maxim Levitsky wrote:
> Currently scsi_target_emulate_report_luns iterates
> over child devices list twice, and there is guarantee, that
> it will not be changed meanwhile.
> 
> This reason for two loops is that it needs to know how much memory
> to allocate.
> 
> Avoid this by iterating once, and allocating the memory for the output
> dynamically with reserving enought memory so that in practice it won't
> be reallocated often.
> 
> Bugzilla for reference: https://bugzilla.redhat.com/show_bug.cgi?id=1866707

"Buglink:" is the tag name documented in
https://wiki.qemu.org/Contribute/SubmitAPatch#Write_a_meaningful_commit_message

>  static bool scsi_target_emulate_report_luns(SCSITargetReq *r)
>  {
>      BusChild *kid;
> -    int i, len, n;
>      int channel, id;
> -    bool found_lun0;
> +    uint8_t tmp[8] = {0};
> +    int len = 0;
> +
> +    /* reserve space for 63 LUNs*/
> +    GByteArray *buf = g_byte_array_sized_new(512);
>  
>      if (r->req.cmd.xfer < 16) {
>          return false;

buf is leaked.

> @@ -460,46 +466,36 @@ static bool 
> scsi_target_emulate_report_luns(SCSITargetReq *r)
>      }
>      channel = r->req.dev->channel;
>      id = r->req.dev->id;
> -    found_lun0 = false;
> -    n = 0;
>  
> -    rcu_read_lock();
>  
> -    QTAILQ_FOREACH_RCU(kid, &r->req.bus->qbus.children, sibling) {
> -        DeviceState *qdev = kid->child;
> -        SCSIDevice *dev = SCSI_DEVICE(qdev);
> +    /* add size (will be updated later to correct value */
> +    g_byte_array_append(buf, tmp, 8);
> +    len += 8;

Can g_byte_array_size() be used instead of keeping a len local variable?

Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>

signature.asc
Description: PGP signature