On 02-May-19 16:55, Jason Gunthorpe wrote:
> On Wed, May 01, 2019 at 01:48:20PM +0300, Gal Pressman wrote:
>
>> +static struct efa_comp_ctx *efa_com_submit_admin_cmd(struct efa_com_admin_queue *aq,
>> + struct efa_admin_aq_entry *cmd,
>> + size_t cmd_size_in_bytes,
>> + struct efa_admin_acq_entry *comp,
>> + size_t comp_size_in_bytes)
>> +{
>> + struct efa_comp_ctx *comp_ctx;
>> +
>> + spin_lock(&aq->sq.lock);
>> + if (!test_bit(EFA_AQ_STATE_RUNNING_BIT, &aq->state)) {
>> + ibdev_err(aq->efa_dev, "Admin queue is closed\n");
>> + spin_unlock(&aq->sq.lock);
>> + return ERR_PTR(-ENODEV);
>> + }
>> +
>> + comp_ctx = __efa_com_submit_admin_cmd(aq, cmd, cmd_size_in_bytes, comp,
>> + comp_size_in_bytes);
>> + spin_unlock(&aq->sq.lock);
>> + if (IS_ERR(comp_ctx))
>> + clear_bit(EFA_AQ_STATE_RUNNING_BIT, &aq->state);
>> +
>> + return comp_ctx;
>> +}
>
> [..]
>
>> +static void efa_com_admin_flush(struct efa_com_dev *edev)
>> +{
>> + struct efa_com_admin_queue *aq = &edev->aq;
>> +
>> + clear_bit(EFA_AQ_STATE_RUNNING_BIT, &aq->state);
>
> This scheme looks use after free racey to me..
The running bit stops new admin commands from being submitted, clearly the exact
moment in which the bit is cleared is "racy" to submission of admin commands but
that is taken care of.
The submission of an admin command is atomic as it is guarded by an admin queue
lock.
The same lock is acquired by this flow as well when flushing the admin queue.
After all admin commands have been aborted and we know for sure that no new
admin commands will be submitted (the bit test is also done inside the lock) we
wait for all commands completions and only then, when all consumers are done, we
safely disable the admin queue.
