From: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
To: linux-kernel@vger.kernel.org
Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
stable@vger.kernel.org, Mikulas Patocka <mpatocka@redhat.com>,
Alasdair G Kergon <agk@redhat.com>
Subject: [PATCH 3.4 31/32] dm: fix truncated status strings
Date: Fri, 6 Dec 2013 13:52:41 -0800 [thread overview]
Message-ID: <20131206214959.683233859@linuxfoundation.org> (raw)
In-Reply-To: <20131206214956.830407026@linuxfoundation.org>
3.4-stable review patch. If anyone has any objections, please let me know.
------------------
From: Mikulas Patocka <mpatocka@redhat.com>
commit fd7c092e711ebab55b2688d3859d95dfd0301f73 upstream.
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.
Signed-off-by: Mikulas Patocka <mpatocka@redhat.com>
Signed-off-by: Alasdair G Kergon <agk@redhat.com>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
---
drivers/md/dm-crypt.c | 37 ++++----------------
drivers/md/dm-delay.c | 6 +--
drivers/md/dm-flakey.c | 5 +-
drivers/md/dm-ioctl.c | 18 ++++++---
drivers/md/dm-linear.c | 5 +-
drivers/md/dm-mpath.c | 6 +--
drivers/md/dm-raid.c | 6 +--
drivers/md/dm-raid1.c | 6 +--
drivers/md/dm-snap.c | 12 ++----
drivers/md/dm-stripe.c | 5 +-
drivers/md/dm-thin.c | 76 +++++++++++++++++++++++++-----------------
drivers/md/dm-verity.c | 6 +--
include/linux/device-mapper.h | 4 +-
13 files changed, 88 insertions(+), 104 deletions(-)
--- a/drivers/md/dm-crypt.c
+++ b/drivers/md/dm-crypt.c
@@ -1262,20 +1262,6 @@ static int crypt_decode_key(u8 *key, cha
return 0;
}
-/*
- * Encode key into its hex representation
- */
-static void crypt_encode_key(char *hex, u8 *key, unsigned int size)
-{
- unsigned int i;
-
- for (i = 0; i < size; i++) {
- sprintf(hex, "%02x", *key);
- hex += 2;
- key++;
- }
-}
-
static void crypt_free_tfms(struct crypt_config *cc, int cpu)
{
struct crypt_cpu *cpu_cc = per_cpu_ptr(cc->cpu, cpu);
@@ -1741,11 +1727,11 @@ static int crypt_map(struct dm_target *t
return DM_MAPIO_SUBMITTED;
}
-static int crypt_status(struct dm_target *ti, status_type_t type,
- char *result, unsigned int maxlen)
+static void crypt_status(struct dm_target *ti, status_type_t type,
+ char *result, unsigned int maxlen)
{
struct crypt_config *cc = ti->private;
- unsigned int sz = 0;
+ unsigned i, sz = 0;
switch (type) {
case STATUSTYPE_INFO:
@@ -1755,17 +1741,11 @@ static int crypt_status(struct dm_target
case STATUSTYPE_TABLE:
DMEMIT("%s ", cc->cipher_string);
- if (cc->key_size > 0) {
- if ((maxlen - sz) < ((cc->key_size << 1) + 1))
- return -ENOMEM;
-
- crypt_encode_key(result + sz, cc->key, cc->key_size);
- sz += cc->key_size << 1;
- } else {
- if (sz >= maxlen)
- return -ENOMEM;
- result[sz++] = '-';
- }
+ if (cc->key_size > 0)
+ for (i = 0; i < cc->key_size; i++)
+ DMEMIT("%02x", cc->key[i]);
+ else
+ DMEMIT("-");
DMEMIT(" %llu %s %llu", (unsigned long long)cc->iv_offset,
cc->dev->name, (unsigned long long)cc->start);
@@ -1775,7 +1755,6 @@ static int crypt_status(struct dm_target
break;
}
- return 0;
}
static void crypt_postsuspend(struct dm_target *ti)
--- a/drivers/md/dm-delay.c
+++ b/drivers/md/dm-delay.c
@@ -294,8 +294,8 @@ static int delay_map(struct dm_target *t
return delay_bio(dc, dc->read_delay, bio);
}
-static int delay_status(struct dm_target *ti, status_type_t type,
- char *result, unsigned maxlen)
+static void delay_status(struct dm_target *ti, status_type_t type,
+ char *result, unsigned maxlen)
{
struct delay_c *dc = ti->private;
int sz = 0;
@@ -315,8 +315,6 @@ static int delay_status(struct dm_target
dc->write_delay);
break;
}
-
- return 0;
}
static int delay_iterate_devices(struct dm_target *ti,
--- a/drivers/md/dm-flakey.c
+++ b/drivers/md/dm-flakey.c
@@ -332,8 +332,8 @@ static int flakey_end_io(struct dm_targe
return error;
}
-static int flakey_status(struct dm_target *ti, status_type_t type,
- char *result, unsigned int maxlen)
+static void flakey_status(struct dm_target *ti, status_type_t type,
+ char *result, unsigned int maxlen)
{
unsigned sz = 0;
struct flakey_c *fc = ti->private;
@@ -363,7 +363,6 @@ static int flakey_status(struct dm_targe
break;
}
- return 0;
}
static int flakey_ioctl(struct dm_target *ti, unsigned int cmd, unsigned long arg)
--- a/drivers/md/dm-ioctl.c
+++ b/drivers/md/dm-ioctl.c
@@ -1066,6 +1066,7 @@ static void retrieve_status(struct dm_ta
num_targets = dm_table_get_num_targets(table);
for (i = 0; i < num_targets; i++) {
struct dm_target *ti = dm_table_get_target(table, i);
+ size_t l;
remaining = len - (outptr - outbuf);
if (remaining <= sizeof(struct dm_target_spec)) {
@@ -1089,15 +1090,18 @@ static void retrieve_status(struct dm_ta
}
/* Get the status/table string from the target driver */
- if (ti->type->status) {
- if (ti->type->status(ti, type, outptr, remaining)) {
- param->flags |= DM_BUFFER_FULL_FLAG;
- break;
- }
- } else
+ if (ti->type->status)
+ ti->type->status(ti, type, outptr, remaining);
+ else
outptr[0] = '\0';
- outptr += strlen(outptr) + 1;
+ l = strlen(outptr) + 1;
+ if (l == remaining) {
+ param->flags |= DM_BUFFER_FULL_FLAG;
+ break;
+ }
+
+ outptr += l;
used = param->data_start + (outptr - outbuf);
outptr = align_ptr(outptr);
--- a/drivers/md/dm-linear.c
+++ b/drivers/md/dm-linear.c
@@ -95,8 +95,8 @@ static int linear_map(struct dm_target *
return DM_MAPIO_REMAPPED;
}
-static int linear_status(struct dm_target *ti, status_type_t type,
- char *result, unsigned int maxlen)
+static void linear_status(struct dm_target *ti, status_type_t type,
+ char *result, unsigned int maxlen)
{
struct linear_c *lc = (struct linear_c *) ti->private;
@@ -110,7 +110,6 @@ static int linear_status(struct dm_targe
(unsigned long long)lc->start);
break;
}
- return 0;
}
static int linear_ioctl(struct dm_target *ti, unsigned int cmd,
--- a/drivers/md/dm-mpath.c
+++ b/drivers/md/dm-mpath.c
@@ -1343,8 +1343,8 @@ static void multipath_resume(struct dm_t
* [priority selector-name num_ps_args [ps_args]*
* num_paths num_selector_args [path_dev [selector_args]* ]+ ]+
*/
-static int multipath_status(struct dm_target *ti, status_type_t type,
- char *result, unsigned int maxlen)
+static void multipath_status(struct dm_target *ti, status_type_t type,
+ char *result, unsigned int maxlen)
{
int sz = 0;
unsigned long flags;
@@ -1447,8 +1447,6 @@ static int multipath_status(struct dm_ta
}
spin_unlock_irqrestore(&m->lock, flags);
-
- return 0;
}
static int multipath_message(struct dm_target *ti, unsigned argc, char **argv)
--- a/drivers/md/dm-raid.c
+++ b/drivers/md/dm-raid.c
@@ -1067,8 +1067,8 @@ static int raid_map(struct dm_target *ti
return DM_MAPIO_SUBMITTED;
}
-static int raid_status(struct dm_target *ti, status_type_t type,
- char *result, unsigned maxlen)
+static void raid_status(struct dm_target *ti, status_type_t type,
+ char *result, unsigned maxlen)
{
struct raid_set *rs = ti->private;
unsigned raid_param_cnt = 1; /* at least 1 for chunksize */
@@ -1203,8 +1203,6 @@ static int raid_status(struct dm_target
DMEMIT(" -");
}
}
-
- return 0;
}
static int raid_iterate_devices(struct dm_target *ti, iterate_devices_callout_fn fn, void *data)
--- a/drivers/md/dm-raid1.c
+++ b/drivers/md/dm-raid1.c
@@ -1362,8 +1362,8 @@ static char device_status_char(struct mi
}
-static int mirror_status(struct dm_target *ti, status_type_t type,
- char *result, unsigned int maxlen)
+static void mirror_status(struct dm_target *ti, status_type_t type,
+ char *result, unsigned int maxlen)
{
unsigned int m, sz = 0;
struct mirror_set *ms = (struct mirror_set *) ti->private;
@@ -1398,8 +1398,6 @@ static int mirror_status(struct dm_targe
if (ms->features & DM_RAID1_HANDLE_ERRORS)
DMEMIT(" 1 handle_errors");
}
-
- return 0;
}
static int mirror_iterate_devices(struct dm_target *ti,
--- a/drivers/md/dm-snap.c
+++ b/drivers/md/dm-snap.c
@@ -1845,8 +1845,8 @@ static void snapshot_merge_resume(struct
start_merge(s);
}
-static int snapshot_status(struct dm_target *ti, status_type_t type,
- char *result, unsigned int maxlen)
+static void snapshot_status(struct dm_target *ti, status_type_t type,
+ char *result, unsigned int maxlen)
{
unsigned sz = 0;
struct dm_snapshot *snap = ti->private;
@@ -1892,8 +1892,6 @@ static int snapshot_status(struct dm_tar
maxlen - sz);
break;
}
-
- return 0;
}
static int snapshot_iterate_devices(struct dm_target *ti,
@@ -2148,8 +2146,8 @@ static void origin_resume(struct dm_targ
ti->split_io = get_origin_minimum_chunksize(dev->bdev);
}
-static int origin_status(struct dm_target *ti, status_type_t type, char *result,
- unsigned int maxlen)
+static void origin_status(struct dm_target *ti, status_type_t type, char *result,
+ unsigned int maxlen)
{
struct dm_dev *dev = ti->private;
@@ -2162,8 +2160,6 @@ static int origin_status(struct dm_targe
snprintf(result, maxlen, "%s", dev->name);
break;
}
-
- return 0;
}
static int origin_merge(struct dm_target *ti, struct bvec_merge_data *bvm,
--- a/drivers/md/dm-stripe.c
+++ b/drivers/md/dm-stripe.c
@@ -302,8 +302,8 @@ static int stripe_map(struct dm_target *
*
*/
-static int stripe_status(struct dm_target *ti,
- status_type_t type, char *result, unsigned int maxlen)
+static void stripe_status(struct dm_target *ti,
+ status_type_t type, char *result, unsigned int maxlen)
{
struct stripe_c *sc = (struct stripe_c *) ti->private;
char buffer[sc->stripes + 1];
@@ -330,7 +330,6 @@ static int stripe_status(struct dm_targe
(unsigned long long)sc->stripe[i].physical_start);
break;
}
- return 0;
}
static int stripe_end_io(struct dm_target *ti, struct bio *bio,
--- a/drivers/md/dm-thin.c
+++ b/drivers/md/dm-thin.c
@@ -2325,8 +2325,8 @@ static int pool_message(struct dm_target
* <transaction id> <used metadata sectors>/<total metadata sectors>
* <used data sectors>/<total data sectors> <held metadata root>
*/
-static int pool_status(struct dm_target *ti, status_type_t type,
- char *result, unsigned maxlen)
+static void pool_status(struct dm_target *ti, status_type_t type,
+ char *result, unsigned maxlen)
{
int r, count;
unsigned sz = 0;
@@ -2343,32 +2343,41 @@ static int pool_status(struct dm_target
switch (type) {
case STATUSTYPE_INFO:
- r = dm_pool_get_metadata_transaction_id(pool->pmd,
- &transaction_id);
- if (r)
- return r;
-
- r = dm_pool_get_free_metadata_block_count(pool->pmd,
- &nr_free_blocks_metadata);
- 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) {
+ 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_held_metadata_root(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,
@@ -2406,8 +2415,10 @@ static int pool_status(struct dm_target
break;
}
+ return;
- return 0;
+err:
+ DMEMIT("Error");
}
static int pool_iterate_devices(struct dm_target *ti,
@@ -2659,8 +2670,8 @@ static void thin_postsuspend(struct dm_t
/*
* <nr mapped sectors> <highest mapped sector>
*/
-static int thin_status(struct dm_target *ti, status_type_t type,
- char *result, unsigned maxlen)
+static void thin_status(struct dm_target *ti, status_type_t type,
+ char *result, unsigned maxlen)
{
int r;
ssize_t sz = 0;
@@ -2674,12 +2685,16 @@ static int thin_status(struct dm_target
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)
@@ -2699,7 +2714,10 @@ static int thin_status(struct dm_target
}
}
- return 0;
+ return;
+
+err:
+ DMEMIT("Error");
}
static int thin_iterate_devices(struct dm_target *ti,
--- a/drivers/md/dm-verity.c
+++ b/drivers/md/dm-verity.c
@@ -514,8 +514,8 @@ static int verity_map(struct dm_target *
/*
* Status: V (valid) or C (corruption found)
*/
-static int verity_status(struct dm_target *ti, status_type_t type,
- char *result, unsigned maxlen)
+static void verity_status(struct dm_target *ti, status_type_t type,
+ char *result, unsigned maxlen)
{
struct dm_verity *v = ti->private;
unsigned sz = 0;
@@ -546,8 +546,6 @@ static int verity_status(struct dm_targe
DMEMIT("%02x", v->salt[x]);
break;
}
-
- return 0;
}
static int verity_ioctl(struct dm_target *ti, unsigned cmd,
--- a/include/linux/device-mapper.h
+++ b/include/linux/device-mapper.h
@@ -72,8 +72,8 @@ typedef void (*dm_postsuspend_fn) (struc
typedef int (*dm_preresume_fn) (struct dm_target *ti);
typedef void (*dm_resume_fn) (struct dm_target *ti);
-typedef int (*dm_status_fn) (struct dm_target *ti, status_type_t status_type,
- char *result, unsigned int maxlen);
+typedef void (*dm_status_fn) (struct dm_target *ti, status_type_t status_type,
+ char *result, unsigned int maxlen);
typedef int (*dm_message_fn) (struct dm_target *ti, unsigned argc, char **argv);
next prev parent reply other threads:[~2013-12-06 21:57 UTC|newest]
Thread overview: 38+ messages / expand[flat|nested] mbox.gz Atom feed top
2013-12-06 21:52 [PATCH 3.4 00/32] 3.4.73-stable review Greg Kroah-Hartman
2013-12-06 21:52 ` [PATCH 3.4 01/32] net: Fix "ip rule delete table 256" Greg Kroah-Hartman
2013-12-06 21:52 ` [PATCH 3.4 02/32] ipv6: use rt6_get_dflt_router to get default router in rt6_route_rcv Greg Kroah-Hartman
2013-12-06 21:52 ` [PATCH 3.4 03/32] random32: fix off-by-one in seeding requirement Greg Kroah-Hartman
2013-12-06 21:52 ` [PATCH 3.4 04/32] bonding: dont permit to use ARP monitoring in 802.3ad mode Greg Kroah-Hartman
2013-12-06 21:52 ` [PATCH 3.4 05/32] 6lowpan: Uncompression of traffic class field was incorrect Greg Kroah-Hartman
2013-12-06 21:52 ` [PATCH 3.4 06/32] bonding: fix two race conditions in bond_store_updelay/downdelay Greg Kroah-Hartman
2013-12-06 21:52 ` [PATCH 3.4 07/32] isdnloop: use strlcpy() instead of strcpy() Greg Kroah-Hartman
2013-12-06 21:52 ` [PATCH 3.4 08/32] connector: improved unaligned access error fix Greg Kroah-Hartman
2013-12-06 21:52 ` [PATCH 3.4 09/32] ipv4: fix possible seqlock deadlock Greg Kroah-Hartman
2013-12-06 21:52 ` [PATCH 3.4 10/32] inet: prevent leakage of uninitialized memory to user in recv syscalls Greg Kroah-Hartman
2013-12-06 21:52 ` [PATCH 3.4 11/32] net: rework recvmsg handler msg_name and msg_namelen logic Greg Kroah-Hartman
2013-12-06 21:52 ` [PATCH 3.4 12/32] net: add BUG_ON if kernel advertises msg_namelen > sizeof(struct sockaddr_storage) Greg Kroah-Hartman
2013-12-06 21:52 ` [PATCH 3.4 13/32] inet: fix addr_len/msg->msg_namelen assignment in recv_error and rxpmtu functions Greg Kroah-Hartman
2013-12-06 21:52 ` [PATCH 3.4 14/32] net: clamp ->msg_namelen instead of returning an error Greg Kroah-Hartman
2013-12-06 21:52 ` [PATCH 3.4 15/32] ipv6: fix leaking uninitialized port number of offender sockaddr Greg Kroah-Hartman
2013-12-06 21:52 ` [PATCH 3.4 16/32] atm: idt77252: fix dev refcnt leak Greg Kroah-Hartman
2013-12-06 21:52 ` [PATCH 3.4 17/32] net: core: Always propagate flag changes to interfaces Greg Kroah-Hartman
2013-12-06 21:52 ` [PATCH 3.4 18/32] bridge: flush brs address entry in fdb when remove the bridge dev Greg Kroah-Hartman
2013-12-06 21:52 ` [PATCH 3.4 19/32] packet: fix use after free race in send path when dev is released Greg Kroah-Hartman
2013-12-06 21:52 ` [PATCH 3.4 20/32] af_packet: block BH in prb_shutdown_retire_blk_timer() Greg Kroah-Hartman
2013-12-06 21:52 ` [PATCH 3.4 21/32] net: update consumers of MSG_MORE to recognize MSG_SENDPAGE_NOTLAST Greg Kroah-Hartman
2013-12-06 21:52 ` [PATCH 3.4 22/32] inet: fix possible seqlock deadlocks Greg Kroah-Hartman
2013-12-06 21:52 ` [PATCH 3.4 23/32] ipv6: fix possible seqlock deadlock in ip6_finish_output2 Greg Kroah-Hartman
2013-12-06 21:52 ` [PATCH 3.4 24/32] {pktgen, xfrm} Update IPv4 header total len and checksum after tranformation Greg Kroah-Hartman
2013-12-06 21:52 ` [PATCH 3.4 25/32] HID: picolcd_core: validate output report details Greg Kroah-Hartman
2013-12-06 21:52 ` [PATCH 3.4 26/32] mmc: block: fix a bug of error handling in MMC driver Greg Kroah-Hartman
2013-12-06 21:52 ` [PATCH 3.4 27/32] nfsd: use "init_net" for portmapper Greg Kroah-Hartman
2013-12-06 21:52 ` [PATCH 3.4 28/32] video: kyro: fix incorrect sizes when copying to userspace Greg Kroah-Hartman
2013-12-06 21:52 ` [PATCH 3.4 29/32] iommu/vt-d: Fixed interaction of VFIO_IOMMU_MAP_DMA with IOMMU address limits Greg Kroah-Hartman
2013-12-06 21:52 ` [PATCH 3.4 30/32] elevator: acquire q->sysfs_lock in elevator_change() Greg Kroah-Hartman
2013-12-06 21:52 ` Greg Kroah-Hartman [this message]
2013-12-06 21:52 ` [PATCH 3.4 32/32] blk-core: Fix memory corruption if blkcg_init_queue fails Greg Kroah-Hartman
2013-12-07 6:45 ` [PATCH 3.4 00/32] 3.4.73-stable review Guenter Roeck
2013-12-07 17:02 ` Greg Kroah-Hartman
2013-12-07 22:16 ` Shuah Khan
2013-12-08 14:38 ` Satoru Takeuchi
2013-12-08 15:23 ` Greg Kroah-Hartman
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20131206214959.683233859@linuxfoundation.org \
--to=gregkh@linuxfoundation.org \
--cc=agk@redhat.com \
--cc=linux-kernel@vger.kernel.org \
--cc=mpatocka@redhat.com \
--cc=stable@vger.kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.