From fd7c092e711ebab55b2688d3859d95dfd0301f73 Mon Sep 17 00:00:00 2001 From: Mikulas Patocka Date: Fri, 1 Mar 2013 22:45:44 +0000 Subject: dm: fix truncated status strings Avoid returning a truncated table or status string instead of setting the DM_BUFFER_FULL_FLAG when the last target of a table fills the buffer. When processing a table or status request, the function retrieve_status calls ti->type->status. If ti->type->status returns non-zero, retrieve_status assumes that the buffer overflowed and sets DM_BUFFER_FULL_FLAG. However, targets don't return non-zero values from their status method on overflow. Most targets returns always zero. If a buffer overflow happens in a target that is not the last in the table, it gets noticed during the next iteration of the loop in retrieve_status; but if a buffer overflow happens in the last target, it goes unnoticed and erroneously truncated data is returned. In the current code, the targets behave in the following way: * dm-crypt returns -ENOMEM if there is not enough space to store the key, but it returns 0 on all other overflows. * dm-thin returns errors from the status method if a disk error happened. This is incorrect because retrieve_status doesn't check the error code, it assumes that all non-zero values mean buffer overflow. * all the other targets always return 0. This patch changes the ti->type->status function to return void (because most targets don't use the return code). Overflow is detected in retrieve_status: if the status method fills up the remaining space completely, it is assumed that buffer overflow happened. Cc: stable@vger.kernel.org Signed-off-by: Mikulas Patocka Signed-off-by: Alasdair G Kergon --- drivers/md/dm-thin.c | 80 ++++++++++++++++++++++++++++++++-------------------- 1 file changed, 49 insertions(+), 31 deletions(-) (limited to 'drivers/md/dm-thin.c') diff --git a/drivers/md/dm-thin.c b/drivers/md/dm-thin.c index 5409607d487..7a66d73148e 100644 --- a/drivers/md/dm-thin.c +++ b/drivers/md/dm-thin.c @@ -2299,8 +2299,8 @@ static void emit_flags(struct pool_features *pf, char *result, * / * / */ -static int pool_status(struct dm_target *ti, status_type_t type, - unsigned status_flags, char *result, unsigned maxlen) +static void pool_status(struct dm_target *ti, status_type_t type, + unsigned status_flags, char *result, unsigned maxlen) { int r; unsigned sz = 0; @@ -2326,32 +2326,41 @@ static int pool_status(struct dm_target *ti, status_type_t type, if (!(status_flags & DM_STATUS_NOFLUSH_FLAG) && !dm_suspended(ti)) (void) commit_or_fallback(pool); - r = dm_pool_get_metadata_transaction_id(pool->pmd, - &transaction_id); - if (r) - return r; + r = dm_pool_get_metadata_transaction_id(pool->pmd, &transaction_id); + if (r) { + DMERR("dm_pool_get_metadata_transaction_id returned %d", r); + goto err; + } - r = dm_pool_get_free_metadata_block_count(pool->pmd, - &nr_free_blocks_metadata); - if (r) - return r; + r = dm_pool_get_free_metadata_block_count(pool->pmd, &nr_free_blocks_metadata); + if (r) { + DMERR("dm_pool_get_free_metadata_block_count returned %d", r); + goto err; + } r = dm_pool_get_metadata_dev_size(pool->pmd, &nr_blocks_metadata); - if (r) - return r; + if (r) { + DMERR("dm_pool_get_metadata_dev_size returned %d", r); + goto err; + } - r = dm_pool_get_free_block_count(pool->pmd, - &nr_free_blocks_data); - if (r) - return r; + r = dm_pool_get_free_block_count(pool->pmd, &nr_free_blocks_data); + if (r) { + DMERR("dm_pool_get_free_block_count returned %d", r); + goto err; + } r = dm_pool_get_data_dev_size(pool->pmd, &nr_blocks_data); - if (r) - return r; + if (r) { + DMERR("dm_pool_get_data_dev_size returned %d", r); + goto err; + } r = dm_pool_get_metadata_snap(pool->pmd, &held_root); - if (r) - return r; + if (r) { + DMERR("dm_pool_get_metadata_snap returned %d", r); + goto err; + } DMEMIT("%llu %llu/%llu %llu/%llu ", (unsigned long long)transaction_id, @@ -2388,8 +2397,10 @@ static int pool_status(struct dm_target *ti, status_type_t type, emit_flags(&pt->requested_pf, result, sz, maxlen); break; } + return; - return 0; +err: + DMEMIT("Error"); } static int pool_iterate_devices(struct dm_target *ti, @@ -2468,7 +2479,7 @@ static struct target_type pool_target = { .name = "thin-pool", .features = DM_TARGET_SINGLETON | DM_TARGET_ALWAYS_WRITEABLE | DM_TARGET_IMMUTABLE, - .version = {1, 6, 0}, + .version = {1, 6, 1}, .module = THIS_MODULE, .ctr = pool_ctr, .dtr = pool_dtr, @@ -2676,8 +2687,8 @@ static void thin_postsuspend(struct dm_target *ti) /* * */ -static int thin_status(struct dm_target *ti, status_type_t type, - unsigned status_flags, char *result, unsigned maxlen) +static void thin_status(struct dm_target *ti, status_type_t type, + unsigned status_flags, char *result, unsigned maxlen) { int r; ssize_t sz = 0; @@ -2687,7 +2698,7 @@ static int thin_status(struct dm_target *ti, status_type_t type, if (get_pool_mode(tc->pool) == PM_FAIL) { DMEMIT("Fail"); - return 0; + return; } if (!tc->td) @@ -2696,12 +2707,16 @@ static int thin_status(struct dm_target *ti, status_type_t type, switch (type) { case STATUSTYPE_INFO: r = dm_thin_get_mapped_count(tc->td, &mapped); - if (r) - return r; + if (r) { + DMERR("dm_thin_get_mapped_count returned %d", r); + goto err; + } r = dm_thin_get_highest_mapped_block(tc->td, &highest); - if (r < 0) - return r; + if (r < 0) { + DMERR("dm_thin_get_highest_mapped_block returned %d", r); + goto err; + } DMEMIT("%llu ", mapped * tc->pool->sectors_per_block); if (r) @@ -2721,7 +2736,10 @@ static int thin_status(struct dm_target *ti, status_type_t type, } } - return 0; + return; + +err: + DMEMIT("Error"); } static int thin_iterate_devices(struct dm_target *ti, @@ -2748,7 +2766,7 @@ static int thin_iterate_devices(struct dm_target *ti, static struct target_type thin_target = { .name = "thin", - .version = {1, 7, 0}, + .version = {1, 7, 1}, .module = THIS_MODULE, .ctr = thin_ctr, .dtr = thin_dtr, -- cgit v1.2.3-18-g5258