Commit 57385b51 authored by Alex Elder's avatar Alex Elder Committed by Sage Weil

rbd: have rbd_obj_method_sync() return transfer count

Callers of rbd_obj_method_sync() don't know how many bytes of data
got returned by the class method call.  As a result, they have been
assuming enough got returned to decode whatever was expected.

This isn't safe.  We know how many bytes got transferred, so have
rbd_obj_method_sync() return that amount (rather than just 0) if
the call is successful.

Change all callers to use this return value to ensure decoding of
the results is done safely.

On the other hand, most callers of rbd_obj_method_sync() only
indicate success or failure, so all of *their* callers can simply
test for non-zero result.

This resolves:
    http://tracker.ceph.com/issues/4773Signed-off-by: default avatarAlex Elder <elder@inktank.com>
Reviewed-by: default avatarJosh Durgin <josh.durgin@inktank.com>
parent 4157976b
...@@ -2642,7 +2642,7 @@ static int rbd_obj_method_sync(struct rbd_device *rbd_dev, ...@@ -2642,7 +2642,7 @@ static int rbd_obj_method_sync(struct rbd_device *rbd_dev,
* method. Currently if this is present it will be a * method. Currently if this is present it will be a
* snapshot id. * snapshot id.
*/ */
page_count = (u32) calc_pages_for(0, inbound_size); page_count = (u32)calc_pages_for(0, inbound_size);
pages = ceph_alloc_page_vector(page_count, GFP_KERNEL); pages = ceph_alloc_page_vector(page_count, GFP_KERNEL);
if (IS_ERR(pages)) if (IS_ERR(pages))
return PTR_ERR(pages); return PTR_ERR(pages);
...@@ -2689,7 +2689,9 @@ static int rbd_obj_method_sync(struct rbd_device *rbd_dev, ...@@ -2689,7 +2689,9 @@ static int rbd_obj_method_sync(struct rbd_device *rbd_dev,
ret = obj_request->result; ret = obj_request->result;
if (ret < 0) if (ret < 0)
goto out; goto out;
ret = 0;
rbd_assert(obj_request->xferred < (u64)INT_MAX);
ret = (int)obj_request->xferred;
ceph_copy_from_page_vector(pages, inbound, 0, obj_request->xferred); ceph_copy_from_page_vector(pages, inbound, 0, obj_request->xferred);
if (version) if (version)
*version = obj_request->version; *version = obj_request->version;
...@@ -3583,13 +3585,15 @@ static int _rbd_dev_v2_snap_size(struct rbd_device *rbd_dev, u64 snap_id, ...@@ -3583,13 +3585,15 @@ static int _rbd_dev_v2_snap_size(struct rbd_device *rbd_dev, u64 snap_id,
dout("%s: rbd_obj_method_sync returned %d\n", __func__, ret); dout("%s: rbd_obj_method_sync returned %d\n", __func__, ret);
if (ret < 0) if (ret < 0)
return ret; return ret;
if (ret < sizeof (size_buf))
return -ERANGE;
*order = size_buf.order; *order = size_buf.order;
*snap_size = le64_to_cpu(size_buf.size); *snap_size = le64_to_cpu(size_buf.size);
dout(" snap_id 0x%016llx order = %u, snap_size = %llu\n", dout(" snap_id 0x%016llx order = %u, snap_size = %llu\n",
(unsigned long long) snap_id, (unsigned int) *order, (unsigned long long)snap_id, (unsigned int)*order,
(unsigned long long) *snap_size); (unsigned long long)*snap_size);
return 0; return 0;
} }
...@@ -3620,8 +3624,8 @@ static int rbd_dev_v2_object_prefix(struct rbd_device *rbd_dev) ...@@ -3620,8 +3624,8 @@ static int rbd_dev_v2_object_prefix(struct rbd_device *rbd_dev)
p = reply_buf; p = reply_buf;
rbd_dev->header.object_prefix = ceph_extract_encoded_string(&p, rbd_dev->header.object_prefix = ceph_extract_encoded_string(&p,
p + RBD_OBJ_PREFIX_LEN_MAX, p + ret, NULL, GFP_NOIO);
NULL, GFP_NOIO); ret = 0;
if (IS_ERR(rbd_dev->header.object_prefix)) { if (IS_ERR(rbd_dev->header.object_prefix)) {
ret = PTR_ERR(rbd_dev->header.object_prefix); ret = PTR_ERR(rbd_dev->header.object_prefix);
...@@ -3629,7 +3633,6 @@ static int rbd_dev_v2_object_prefix(struct rbd_device *rbd_dev) ...@@ -3629,7 +3633,6 @@ static int rbd_dev_v2_object_prefix(struct rbd_device *rbd_dev)
} else { } else {
dout(" object_prefix = %s\n", rbd_dev->header.object_prefix); dout(" object_prefix = %s\n", rbd_dev->header.object_prefix);
} }
out: out:
kfree(reply_buf); kfree(reply_buf);
...@@ -3654,6 +3657,8 @@ static int _rbd_dev_v2_snap_features(struct rbd_device *rbd_dev, u64 snap_id, ...@@ -3654,6 +3657,8 @@ static int _rbd_dev_v2_snap_features(struct rbd_device *rbd_dev, u64 snap_id,
dout("%s: rbd_obj_method_sync returned %d\n", __func__, ret); dout("%s: rbd_obj_method_sync returned %d\n", __func__, ret);
if (ret < 0) if (ret < 0)
return ret; return ret;
if (ret < sizeof (features_buf))
return -ERANGE;
incompat = le64_to_cpu(features_buf.incompat); incompat = le64_to_cpu(features_buf.incompat);
if (incompat & ~RBD_FEATURES_SUPPORTED) if (incompat & ~RBD_FEATURES_SUPPORTED)
...@@ -3662,9 +3667,9 @@ static int _rbd_dev_v2_snap_features(struct rbd_device *rbd_dev, u64 snap_id, ...@@ -3662,9 +3667,9 @@ static int _rbd_dev_v2_snap_features(struct rbd_device *rbd_dev, u64 snap_id,
*snap_features = le64_to_cpu(features_buf.features); *snap_features = le64_to_cpu(features_buf.features);
dout(" snap_id 0x%016llx features = 0x%016llx incompat = 0x%016llx\n", dout(" snap_id 0x%016llx features = 0x%016llx incompat = 0x%016llx\n",
(unsigned long long) snap_id, (unsigned long long)snap_id,
(unsigned long long) *snap_features, (unsigned long long)*snap_features,
(unsigned long long) le64_to_cpu(features_buf.incompat)); (unsigned long long)le64_to_cpu(features_buf.incompat));
return 0; return 0;
} }
...@@ -3710,9 +3715,9 @@ static int rbd_dev_v2_parent_info(struct rbd_device *rbd_dev) ...@@ -3710,9 +3715,9 @@ static int rbd_dev_v2_parent_info(struct rbd_device *rbd_dev)
if (ret < 0) if (ret < 0)
goto out_err; goto out_err;
ret = -ERANGE;
p = reply_buf; p = reply_buf;
end = reply_buf + size; end = reply_buf + ret;
ret = -ERANGE;
ceph_decode_64_safe(&p, end, parent_spec->pool_id, out_err); ceph_decode_64_safe(&p, end, parent_spec->pool_id, out_err);
if (parent_spec->pool_id == CEPH_NOPOOL) if (parent_spec->pool_id == CEPH_NOPOOL)
goto out; /* No parent? No problem. */ goto out; /* No parent? No problem. */
...@@ -3720,8 +3725,8 @@ static int rbd_dev_v2_parent_info(struct rbd_device *rbd_dev) ...@@ -3720,8 +3725,8 @@ static int rbd_dev_v2_parent_info(struct rbd_device *rbd_dev)
/* The ceph file layout needs to fit pool id in 32 bits */ /* The ceph file layout needs to fit pool id in 32 bits */
ret = -EIO; ret = -EIO;
if (WARN_ON(parent_spec->pool_id > (u64) U32_MAX)) if (WARN_ON(parent_spec->pool_id > (u64)U32_MAX))
goto out; goto out_err;
image_id = ceph_extract_encoded_string(&p, end, NULL, GFP_KERNEL); image_id = ceph_extract_encoded_string(&p, end, NULL, GFP_KERNEL);
if (IS_ERR(image_id)) { if (IS_ERR(image_id)) {
...@@ -3766,7 +3771,7 @@ static char *rbd_dev_image_name(struct rbd_device *rbd_dev) ...@@ -3766,7 +3771,7 @@ static char *rbd_dev_image_name(struct rbd_device *rbd_dev)
p = image_id; p = image_id;
end = image_id + image_id_size; end = image_id + image_id_size;
ceph_encode_string(&p, end, rbd_dev->spec->image_id, (u32) len); ceph_encode_string(&p, end, rbd_dev->spec->image_id, (u32)len);
size = sizeof (__le32) + RBD_IMAGE_NAME_LEN_MAX; size = sizeof (__le32) + RBD_IMAGE_NAME_LEN_MAX;
reply_buf = kmalloc(size, GFP_KERNEL); reply_buf = kmalloc(size, GFP_KERNEL);
...@@ -3886,9 +3891,9 @@ static int rbd_dev_v2_snap_context(struct rbd_device *rbd_dev, u64 *ver) ...@@ -3886,9 +3891,9 @@ static int rbd_dev_v2_snap_context(struct rbd_device *rbd_dev, u64 *ver)
if (ret < 0) if (ret < 0)
goto out; goto out;
ret = -ERANGE;
p = reply_buf; p = reply_buf;
end = reply_buf + size; end = reply_buf + ret;
ret = -ERANGE;
ceph_decode_64_safe(&p, end, seq, out); ceph_decode_64_safe(&p, end, seq, out);
ceph_decode_32_safe(&p, end, snap_count, out); ceph_decode_32_safe(&p, end, snap_count, out);
...@@ -3913,6 +3918,7 @@ static int rbd_dev_v2_snap_context(struct rbd_device *rbd_dev, u64 *ver) ...@@ -3913,6 +3918,7 @@ static int rbd_dev_v2_snap_context(struct rbd_device *rbd_dev, u64 *ver)
ret = -ENOMEM; ret = -ENOMEM;
goto out; goto out;
} }
ret = 0;
atomic_set(&snapc->nref, 1); atomic_set(&snapc->nref, 1);
snapc->seq = seq; snapc->seq = seq;
...@@ -3923,12 +3929,11 @@ static int rbd_dev_v2_snap_context(struct rbd_device *rbd_dev, u64 *ver) ...@@ -3923,12 +3929,11 @@ static int rbd_dev_v2_snap_context(struct rbd_device *rbd_dev, u64 *ver)
rbd_dev->header.snapc = snapc; rbd_dev->header.snapc = snapc;
dout(" snap context seq = %llu, snap_count = %u\n", dout(" snap context seq = %llu, snap_count = %u\n",
(unsigned long long) seq, (unsigned int) snap_count); (unsigned long long)seq, (unsigned int)snap_count);
out: out:
kfree(reply_buf); kfree(reply_buf);
return 0; return ret;
} }
static char *rbd_dev_v2_snap_name(struct rbd_device *rbd_dev, u32 which) static char *rbd_dev_v2_snap_name(struct rbd_device *rbd_dev, u32 which)
...@@ -3963,7 +3968,7 @@ static char *rbd_dev_v2_snap_name(struct rbd_device *rbd_dev, u32 which) ...@@ -3963,7 +3968,7 @@ static char *rbd_dev_v2_snap_name(struct rbd_device *rbd_dev, u32 which)
goto out; goto out;
} else { } else {
dout(" snap_id 0x%016llx snap_name = %s\n", dout(" snap_id 0x%016llx snap_name = %s\n",
(unsigned long long) le64_to_cpu(snap_id), snap_name); (unsigned long long)le64_to_cpu(snap_id), snap_name);
} }
kfree(reply_buf); kfree(reply_buf);
...@@ -4560,8 +4565,10 @@ static int rbd_dev_image_id(struct rbd_device *rbd_dev) ...@@ -4560,8 +4565,10 @@ static int rbd_dev_image_id(struct rbd_device *rbd_dev)
p = response; p = response;
rbd_dev->spec->image_id = ceph_extract_encoded_string(&p, rbd_dev->spec->image_id = ceph_extract_encoded_string(&p,
p + RBD_IMAGE_ID_LEN_MAX, p + ret,
NULL, GFP_NOIO); NULL, GFP_NOIO);
ret = 0;
if (IS_ERR(rbd_dev->spec->image_id)) { if (IS_ERR(rbd_dev->spec->image_id)) {
ret = PTR_ERR(rbd_dev->spec->image_id); ret = PTR_ERR(rbd_dev->spec->image_id);
rbd_dev->spec->image_id = NULL; rbd_dev->spec->image_id = NULL;
...@@ -4642,28 +4649,27 @@ static int rbd_dev_v2_probe(struct rbd_device *rbd_dev) ...@@ -4642,28 +4649,27 @@ static int rbd_dev_v2_probe(struct rbd_device *rbd_dev)
RBD_HEADER_PREFIX, rbd_dev->spec->image_id); RBD_HEADER_PREFIX, rbd_dev->spec->image_id);
/* Get the size and object order for the image */ /* Get the size and object order for the image */
ret = rbd_dev_v2_image_size(rbd_dev); ret = rbd_dev_v2_image_size(rbd_dev);
if (ret < 0) if (ret)
goto out_err; goto out_err;
/* Get the object prefix (a.k.a. block_name) for the image */ /* Get the object prefix (a.k.a. block_name) for the image */
ret = rbd_dev_v2_object_prefix(rbd_dev); ret = rbd_dev_v2_object_prefix(rbd_dev);
if (ret < 0) if (ret)
goto out_err; goto out_err;
/* Get the and check features for the image */ /* Get the and check features for the image */
ret = rbd_dev_v2_features(rbd_dev); ret = rbd_dev_v2_features(rbd_dev);
if (ret < 0) if (ret)
goto out_err; goto out_err;
/* If the image supports layering, get the parent info */ /* If the image supports layering, get the parent info */
if (rbd_dev->header.features & RBD_FEATURE_LAYERING) { if (rbd_dev->header.features & RBD_FEATURE_LAYERING) {
ret = rbd_dev_v2_parent_info(rbd_dev); ret = rbd_dev_v2_parent_info(rbd_dev);
if (ret < 0) if (ret)
goto out_err; goto out_err;
} }
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment