Commit ccece235 authored by Alex Elder's avatar Alex Elder Committed by Sage Weil
Browse files

rbd: fixes in rbd_header_from_disk()



This fixes a few issues in rbd_header_from_disk():
    - There is a check intended to catch overflow, but it's wrong in
      two ways.
	- First, the type we don't want to overflow is size_t, not
	  unsigned int, and there is now a SIZE_MAX we can use for
	  use with that type.
	- Second, we're allocating the snapshot ids and snapshot
	  image sizes separately (each has type u64; on disk they
          grouped together as a rbd_image_header_ondisk structure).
	  So we can use the size of u64 in this overflow check.
    - If there are no snapshots, then there should be no snapshot
      names.  Enforce this, and issue a warning if we encounter a
      header with no snapshots but a non-zero snap_names_len.
    - When saving the snapshot names into the header, be more direct
      in defining the offset in the on-disk structure from which
      they're being copied by using "snap_count" rather than "i"
      in the array index.
    - If an error occurs, the "snapc" and "snap_names" fields are
      freed at the end of the function.  Make those fields be null
      pointers after they're freed, to be explicit that they are
      no longer valid.
    - Finally, move the definition of the local variable "i" to the
      innermost scope in which it's needed.
Signed-off-by: default avatarAlex Elder <elder@inktank.com>
Reviewed-by: Josh Durgin <josh.durgin@inktank.com> 
parent 913d2fdc
...@@ -494,14 +494,14 @@ static int rbd_header_from_disk(struct rbd_image_header *header, ...@@ -494,14 +494,14 @@ static int rbd_header_from_disk(struct rbd_image_header *header,
struct rbd_image_header_ondisk *ondisk, struct rbd_image_header_ondisk *ondisk,
u32 allocated_snaps) u32 allocated_snaps)
{ {
u32 i, snap_count; u32 snap_count;
if (!rbd_dev_ondisk_valid(ondisk)) if (!rbd_dev_ondisk_valid(ondisk))
return -ENXIO; return -ENXIO;
snap_count = le32_to_cpu(ondisk->snap_count); snap_count = le32_to_cpu(ondisk->snap_count);
if (snap_count > (UINT_MAX - sizeof(struct ceph_snap_context)) if (snap_count > (SIZE_MAX - sizeof(struct ceph_snap_context))
/ sizeof (*ondisk)) / sizeof (u64))
return -EINVAL; return -EINVAL;
header->snapc = kmalloc(sizeof(struct ceph_snap_context) + header->snapc = kmalloc(sizeof(struct ceph_snap_context) +
snap_count * sizeof(u64), snap_count * sizeof(u64),
...@@ -509,8 +509,8 @@ static int rbd_header_from_disk(struct rbd_image_header *header, ...@@ -509,8 +509,8 @@ static int rbd_header_from_disk(struct rbd_image_header *header,
if (!header->snapc) if (!header->snapc)
return -ENOMEM; return -ENOMEM;
header->snap_names_len = le64_to_cpu(ondisk->snap_names_len);
if (snap_count) { if (snap_count) {
header->snap_names_len = le64_to_cpu(ondisk->snap_names_len);
header->snap_names = kmalloc(header->snap_names_len, header->snap_names = kmalloc(header->snap_names_len,
GFP_KERNEL); GFP_KERNEL);
if (!header->snap_names) if (!header->snap_names)
...@@ -520,6 +520,8 @@ static int rbd_header_from_disk(struct rbd_image_header *header, ...@@ -520,6 +520,8 @@ static int rbd_header_from_disk(struct rbd_image_header *header,
if (!header->snap_sizes) if (!header->snap_sizes)
goto err_names; goto err_names;
} else { } else {
WARN_ON(ondisk->snap_names_len);
header->snap_names_len = 0;
header->snap_names = NULL; header->snap_names = NULL;
header->snap_sizes = NULL; header->snap_sizes = NULL;
} }
...@@ -544,6 +546,8 @@ static int rbd_header_from_disk(struct rbd_image_header *header, ...@@ -544,6 +546,8 @@ static int rbd_header_from_disk(struct rbd_image_header *header,
header->total_snaps = snap_count; header->total_snaps = snap_count;
if (snap_count && allocated_snaps == snap_count) { if (snap_count && allocated_snaps == snap_count) {
int i;
for (i = 0; i < snap_count; i++) { for (i = 0; i < snap_count; i++) {
header->snapc->snaps[i] = header->snapc->snaps[i] =
le64_to_cpu(ondisk->snaps[i].id); le64_to_cpu(ondisk->snaps[i].id);
...@@ -552,7 +556,7 @@ static int rbd_header_from_disk(struct rbd_image_header *header, ...@@ -552,7 +556,7 @@ static int rbd_header_from_disk(struct rbd_image_header *header,
} }
/* copy snapshot names */ /* copy snapshot names */
memcpy(header->snap_names, &ondisk->snaps[i], memcpy(header->snap_names, &ondisk->snaps[snap_count],
header->snap_names_len); header->snap_names_len);
} }
...@@ -560,10 +564,14 @@ static int rbd_header_from_disk(struct rbd_image_header *header, ...@@ -560,10 +564,14 @@ static int rbd_header_from_disk(struct rbd_image_header *header,
err_sizes: err_sizes:
kfree(header->snap_sizes); kfree(header->snap_sizes);
header->snap_sizes = NULL;
err_names: err_names:
kfree(header->snap_names); kfree(header->snap_names);
header->snap_names = NULL;
err_snapc: err_snapc:
kfree(header->snapc); kfree(header->snapc);
header->snapc = NULL;
return -ENOMEM; return -ENOMEM;
} }
......
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