Re: [PATCH 2/2] block-rbd: One function call less in rbd_dev_probe_parent() after error detection

[SF Markus Elfring]

>> @@ -5157,14 +5157,14 @@ static int rbd_dev_probe_parent(struct rbd_device *rbd_dev, int depth)
>>         if (++depth > RBD_MAX_PARENT_CHAIN_LEN) {
>>                 pr_info("parent chain is too long (%d)\n", depth);
>>                 ret = -EINVAL;
>> -               goto out_err;
>> +               goto unparent_device;
>>         }
>>
>>         parent = rbd_dev_create(rbd_dev->rbd_client, rbd_dev->parent_spec,
>>                                 NULL);
>>         if (!parent) {
>>                 ret = -ENOMEM;
>> -               goto out_err;
>> +               goto unparent_device;
>>         }
>>
>>         /*
>> @@ -5176,15 +5176,15 @@ static int rbd_dev_probe_parent(struct rbd_device *rbd_dev, int depth)
>>
>>         ret = rbd_dev_image_probe(parent, depth);
>>         if (ret < 0)
>> -               goto out_err;
>> +               goto destroy_device;
>>
>>         rbd_dev->parent = parent;
>>         atomic_set(&rbd_dev->parent_ref, 1);
>>         return 0;
>> -
>> -out_err:
>> -       rbd_dev_unparent(rbd_dev);
>> +destroy_device:
>>         rbd_dev_destroy(parent);
>> +unparent_device:
>> +       rbd_dev_unparent(rbd_dev);
>>         return ret;
>>  }
> 
> Cleanup here is (and should be) done in reverse order.

I have got an other impression about the appropriate order for the corresponding
clean-up function calls.


> We allocate parent rbd_device and then link it with what we already have,

I guess that we have got a different understanding about the relevant "linking".


> so the order in which we cleanup is unlink ("unparent"), destroy.

I interpreted the eventual passing of a null pointer to the rbd_dev_destroy()
function as an indication for further source code adjustments.


> Changing it is just asking for use-after-free bugs.

Do the affected implementation details need a bit more clarification?

Regards,
Markus
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/