On 07/24/2015 04:58 PM, Christoph Hellwig wrote:
> On Wed, Jul 08, 2015 at 11:06:09AM +0200, Hannes Reinecke wrote:
>> + pg = kzalloc(sizeof(struct alua_port_group), GFP_KERNEL);
>> + if (!pg) {
>> + sdev_printk(KERN_WARNING, sdev,
>> + "%s: kzalloc port group failed\n",
>> + ALUA_DH_NAME);
>> + /* Temporary failure, bypass */
>> + return SCSI_DH_DEV_TEMP_BUSY;
>> + }
>> + pg->group_id = group_id;
>> + pg->buff = pg->inq;
>> + pg->bufflen = ALUA_INQUIRY_SIZE;
>> + pg->tpgs = h->tpgs;
>> + pg->state = TPGS_STATE_OPTIMIZED;
>> + kref_init(&pg->kref);
>> + spin_lock(&port_group_lock);
>> + list_add(&pg->node, &port_group_list);
>> + h->pg = pg;
>> + spin_unlock(&port_group_lock);
>
> Is there any high level protection against someone racing to allocate
> this structure, e.g. from a sysfs-initiated scan?
>
Not in this patch, as this is called during device scan only. It is
assumed that higher levels will protect against simultaneous scans.
Real protection against concurrent updates is done in patch
'scsi_dh_alua: parse device id', as with that we can easily hit existing
port groups.
>> - len = (h->buff[0] << 24) + (h->buff[1] << 16) +
>> - (h->buff[2] << 8) + h->buff[3] + 4;
>> + len = get_unaligned_be32(&pg->buff[0]) + 4;
>
> Andother spurious get/set_unaligned conversion. I'd really recommend doing
> all of them before the atual series.
>
Okay, will be doing so.
>> + rcu_read_lock();
>> + pg = rcu_dereference(h->pg);
>> + if (!pg) {
>> + rcu_read_unlock();
>> + return -ENXIO;
>> + }
>> + rcu_read_unlock();
>> +
>> if (optimize)
>> - h->flags |= ALUA_OPTIMIZE_STPG;
>> + pg->flags |= ALUA_OPTIMIZE_STPG;
>> else
>> - h->flags &= ~ALUA_OPTIMIZE_STPG;
>> + pg->flags |= ~ALUA_OPTIMIZE_STPG;
>
> You'll need to move the rcu_read_unlock here to be safe.
>
Ok.
Cheers,
Hannes
--
Dr. Hannes Reinecke zSeries & Storage
hare@xxxxxxx +49 911 74053 688
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg
GF: J. Hawn, J. Guild, F. Imendörffer, HRB 16746 (AG Nürnberg)
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
