On 04/09/2012 04:52 PM, Xi Wang wrote:
ondisk->snap_count is read from disk via rbd_req_sync_read() and thus
needs validation. Otherwise, a bogus `snap_count' could overflow the
kmalloc() size, leading to memory corruption.
Also use `u32' consistently for `snap_count'.
This looks good, however I have changed it to use UINT_MAX
rather than ULONG_MAX, because on some architectures size_t
(__kernel_size_t) is defined as type (unsigned int). It
is the more conservative value, and even on architectures
where __BITS_PER_LONG is 64, it still offers a sane upper
bound on the number of snapshots for a rbd device.
Reviewed-by: Alex Elder <elder@xxxxxxxxxxxxx>
Signed-off-by: Xi Wang<xi.wang@xxxxxxxxx>
---
drivers/block/rbd.c | 12 +++++++-----
1 files changed, 7 insertions(+), 5 deletions(-)
diff --git a/drivers/block/rbd.c b/drivers/block/rbd.c
index 013c7a5..d47f7e6 100644
--- a/drivers/block/rbd.c
+++ b/drivers/block/rbd.c
@@ -487,18 +487,20 @@ static void rbd_coll_release(struct kref *kref)
*/
static int rbd_header_from_disk(struct rbd_image_header *header,
struct rbd_image_header_ondisk *ondisk,
- int allocated_snaps,
+ u32 allocated_snaps,
gfp_t gfp_flags)
{
- int i;
- u32 snap_count;
+ u32 i, snap_count;
if (memcmp(ondisk, RBD_HEADER_TEXT, sizeof(RBD_HEADER_TEXT)))
return -ENXIO;
snap_count = le32_to_cpu(ondisk->snap_count);
+ if (snap_count> (ULONG_MAX - sizeof(struct ceph_snap_context))
+ / sizeof(*ondisk))
+ return -EINVAL;
header->snapc = kmalloc(sizeof(struct ceph_snap_context) +
- snap_count * sizeof (*ondisk),
+ snap_count * sizeof(*ondisk),
gfp_flags);
if (!header->snapc)
return -ENOMEM;
@@ -1592,7 +1594,7 @@ static int rbd_read_header(struct rbd_device *rbd_dev,
{
ssize_t rc;
struct rbd_image_header_ondisk *dh;
- int snap_count = 0;
+ u32 snap_count = 0;
u64 ver;
size_t len;
--
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
