Commit fd7c092e authored by Mikulas Patocka's avatar Mikulas Patocka Committed by Alasdair G Kergon
Browse files

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: default avatarMikulas Patocka <mpatocka@redhat.com>
Signed-off-by: default avatarAlasdair G Kergon <agk@redhat.com>
parent 16245bdc
Loading
Loading
Loading
Loading
+9 −30
Original line number Original line Diff line number Diff line
@@ -1234,20 +1234,6 @@ static int crypt_decode_key(u8 *key, char *hex, unsigned int size)
	return 0;
	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)
static void crypt_free_tfms(struct crypt_config *cc)
{
{
	unsigned i;
	unsigned i;
@@ -1717,11 +1703,11 @@ static int crypt_map(struct dm_target *ti, struct bio *bio)
	return DM_MAPIO_SUBMITTED;
	return DM_MAPIO_SUBMITTED;
}
}


static int crypt_status(struct dm_target *ti, status_type_t type,
static void crypt_status(struct dm_target *ti, status_type_t type,
			 unsigned status_flags, char *result, unsigned maxlen)
			 unsigned status_flags, char *result, unsigned maxlen)
{
{
	struct crypt_config *cc = ti->private;
	struct crypt_config *cc = ti->private;
	unsigned int sz = 0;
	unsigned i, sz = 0;


	switch (type) {
	switch (type) {
	case STATUSTYPE_INFO:
	case STATUSTYPE_INFO:
@@ -1731,17 +1717,11 @@ static int crypt_status(struct dm_target *ti, status_type_t type,
	case STATUSTYPE_TABLE:
	case STATUSTYPE_TABLE:
		DMEMIT("%s ", cc->cipher_string);
		DMEMIT("%s ", cc->cipher_string);


		if (cc->key_size > 0) {
		if (cc->key_size > 0)
			if ((maxlen - sz) < ((cc->key_size << 1) + 1))
			for (i = 0; i < cc->key_size; i++)
				return -ENOMEM;
				DMEMIT("%02x", cc->key[i]);

		else
			crypt_encode_key(result + sz, cc->key, cc->key_size);
			DMEMIT("-");
			sz += cc->key_size << 1;
		} else {
			if (sz >= maxlen)
				return -ENOMEM;
			result[sz++] = '-';
		}


		DMEMIT(" %llu %s %llu", (unsigned long long)cc->iv_offset,
		DMEMIT(" %llu %s %llu", (unsigned long long)cc->iv_offset,
				cc->dev->name, (unsigned long long)cc->start);
				cc->dev->name, (unsigned long long)cc->start);
@@ -1751,7 +1731,6 @@ static int crypt_status(struct dm_target *ti, status_type_t type,


		break;
		break;
	}
	}
	return 0;
}
}


static void crypt_postsuspend(struct dm_target *ti)
static void crypt_postsuspend(struct dm_target *ti)
@@ -1845,7 +1824,7 @@ static int crypt_iterate_devices(struct dm_target *ti,


static struct target_type crypt_target = {
static struct target_type crypt_target = {
	.name   = "crypt",
	.name   = "crypt",
	.version = {1, 12, 0},
	.version = {1, 12, 1},
	.module = THIS_MODULE,
	.module = THIS_MODULE,
	.ctr    = crypt_ctr,
	.ctr    = crypt_ctr,
	.dtr    = crypt_dtr,
	.dtr    = crypt_dtr,
+3 −5
Original line number Original line Diff line number Diff line
@@ -293,7 +293,7 @@ static int delay_map(struct dm_target *ti, struct bio *bio)
	return delay_bio(dc, dc->read_delay, bio);
	return delay_bio(dc, dc->read_delay, bio);
}
}


static int delay_status(struct dm_target *ti, status_type_t type,
static void delay_status(struct dm_target *ti, status_type_t type,
			 unsigned status_flags, char *result, unsigned maxlen)
			 unsigned status_flags, char *result, unsigned maxlen)
{
{
	struct delay_c *dc = ti->private;
	struct delay_c *dc = ti->private;
@@ -314,8 +314,6 @@ static int delay_status(struct dm_target *ti, status_type_t type,
			       dc->write_delay);
			       dc->write_delay);
		break;
		break;
	}
	}

	return 0;
}
}


static int delay_iterate_devices(struct dm_target *ti,
static int delay_iterate_devices(struct dm_target *ti,
@@ -337,7 +335,7 @@ out:


static struct target_type delay_target = {
static struct target_type delay_target = {
	.name	     = "delay",
	.name	     = "delay",
	.version     = {1, 2, 0},
	.version     = {1, 2, 1},
	.module      = THIS_MODULE,
	.module      = THIS_MODULE,
	.ctr	     = delay_ctr,
	.ctr	     = delay_ctr,
	.dtr	     = delay_dtr,
	.dtr	     = delay_dtr,
+3 −4
Original line number Original line Diff line number Diff line
@@ -337,7 +337,7 @@ static int flakey_end_io(struct dm_target *ti, struct bio *bio, int error)
	return error;
	return error;
}
}


static int flakey_status(struct dm_target *ti, status_type_t type,
static void flakey_status(struct dm_target *ti, status_type_t type,
			  unsigned status_flags, char *result, unsigned maxlen)
			  unsigned status_flags, char *result, unsigned maxlen)
{
{
	unsigned sz = 0;
	unsigned sz = 0;
@@ -368,7 +368,6 @@ static int flakey_status(struct dm_target *ti, status_type_t type,


		break;
		break;
	}
	}
	return 0;
}
}


static int flakey_ioctl(struct dm_target *ti, unsigned int cmd, unsigned long arg)
static int flakey_ioctl(struct dm_target *ti, unsigned int cmd, unsigned long arg)
@@ -411,7 +410,7 @@ static int flakey_iterate_devices(struct dm_target *ti, iterate_devices_callout_


static struct target_type flakey_target = {
static struct target_type flakey_target = {
	.name   = "flakey",
	.name   = "flakey",
	.version = {1, 3, 0},
	.version = {1, 3, 1},
	.module = THIS_MODULE,
	.module = THIS_MODULE,
	.ctr    = flakey_ctr,
	.ctr    = flakey_ctr,
	.dtr    = flakey_dtr,
	.dtr    = flakey_dtr,
+9 −5
Original line number Original line Diff line number Diff line
@@ -1067,6 +1067,7 @@ static void retrieve_status(struct dm_table *table,
	num_targets = dm_table_get_num_targets(table);
	num_targets = dm_table_get_num_targets(table);
	for (i = 0; i < num_targets; i++) {
	for (i = 0; i < num_targets; i++) {
		struct dm_target *ti = dm_table_get_target(table, i);
		struct dm_target *ti = dm_table_get_target(table, i);
		size_t l;


		remaining = len - (outptr - outbuf);
		remaining = len - (outptr - outbuf);
		if (remaining <= sizeof(struct dm_target_spec)) {
		if (remaining <= sizeof(struct dm_target_spec)) {
@@ -1093,14 +1094,17 @@ static void retrieve_status(struct dm_table *table,
		if (ti->type->status) {
		if (ti->type->status) {
			if (param->flags & DM_NOFLUSH_FLAG)
			if (param->flags & DM_NOFLUSH_FLAG)
				status_flags |= DM_STATUS_NOFLUSH_FLAG;
				status_flags |= DM_STATUS_NOFLUSH_FLAG;
			if (ti->type->status(ti, type, status_flags, outptr, remaining)) {
			ti->type->status(ti, type, status_flags, outptr, remaining);
		} else
			outptr[0] = '\0';

		l = strlen(outptr) + 1;
		if (l == remaining) {
			param->flags |= DM_BUFFER_FULL_FLAG;
			param->flags |= DM_BUFFER_FULL_FLAG;
			break;
			break;
		}
		}
		} else
			outptr[0] = '\0';


		outptr += strlen(outptr) + 1;
		outptr += l;
		used = param->data_start + (outptr - outbuf);
		used = param->data_start + (outptr - outbuf);


		outptr = align_ptr(outptr);
		outptr = align_ptr(outptr);
+3 −4
Original line number Original line Diff line number Diff line
@@ -95,7 +95,7 @@ static int linear_map(struct dm_target *ti, struct bio *bio)
	return DM_MAPIO_REMAPPED;
	return DM_MAPIO_REMAPPED;
}
}


static int linear_status(struct dm_target *ti, status_type_t type,
static void linear_status(struct dm_target *ti, status_type_t type,
			  unsigned status_flags, char *result, unsigned maxlen)
			  unsigned status_flags, char *result, unsigned maxlen)
{
{
	struct linear_c *lc = (struct linear_c *) ti->private;
	struct linear_c *lc = (struct linear_c *) ti->private;
@@ -110,7 +110,6 @@ static int linear_status(struct dm_target *ti, status_type_t type,
				(unsigned long long)lc->start);
				(unsigned long long)lc->start);
		break;
		break;
	}
	}
	return 0;
}
}


static int linear_ioctl(struct dm_target *ti, unsigned int cmd,
static int linear_ioctl(struct dm_target *ti, unsigned int cmd,
@@ -155,7 +154,7 @@ static int linear_iterate_devices(struct dm_target *ti,


static struct target_type linear_target = {
static struct target_type linear_target = {
	.name   = "linear",
	.name   = "linear",
	.version = {1, 2, 0},
	.version = {1, 2, 1},
	.module = THIS_MODULE,
	.module = THIS_MODULE,
	.ctr    = linear_ctr,
	.ctr    = linear_ctr,
	.dtr    = linear_dtr,
	.dtr    = linear_dtr,
Loading