>> @@ -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 kernel-janitors" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
