On Thu, Sep 11, 2014 at 8:24 PM, Alex Elder <elder@xxxxxxxx> wrote:
> On 09/11/2014 11:17 AM, Ilya Dryomov wrote:
>> On Thu, Sep 11, 2014 at 7:23 PM, Alex Elder <elder@xxxxxxxx> wrote:
>>> On 09/11/2014 10:10 AM, Ilya Dryomov wrote:
>>>> Trying to map an image out of a pool for which we don't have an 'x'
>>>> permission bit fails with -ERANGE from ceph_extract_encoded_string()
>>>> due to an unsigned vs signed bug. Fix it and get rid of the -ENIVAL
>>>> sink, thus exposing rbd::get_id cls method return value. (I've seen
>>>> a bunch of unexplained -ERANGE reports, I bet this is it).
>>>>
>>>> Signed-off-by: Ilya Dryomov <ilya.dryomov@xxxxxxxxxxx>
>>>
>>> I often think people are annoyed by my explicit type casts
>>> all over the place. This (missed) one matters a lot.
>>>
>>> I think the -EINVAL was to ensure an error code that was
>>> expected by a write() call would be returned.
>>
>> Yeah, the way it's written it's possible in theory to get a positive
>> return value from rbd_dev_image_id(). Looking deeper, this sizeof() is
>> not needed at all - ceph_extract_encoded_string() deals with short
>> buffers as it should. As for the ret == sizeof(u32) (i.e. an empty
>> string), neither userspace nor us check against empty strings in
>> similar cases (object prefix, snapshot name, etc).
>
> I should have asked this before. Why is a permission error
> leading to ceph_extract_encoded_string() finding a short
> buffer? I didn't take the time to trace the error path
> you're talking about here all the way back.
rbd_obj_method_sync() returns -EPERM, which when compared with size_t
from sizeof() ends up a big positive. ceph_extract_encoded_string() is
then called and the safe decode macro kicks in..
Thanks,
Ilya
--
To unsubscribe from this list: send the line "unsubscribe ceph-devel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
