FAQ Search Today's Posts Mark Forums Read
» Video Reviews

» Linux Archive

Linux-archive is a website aiming to archive linux email lists and to make them easily accessible for linux users/developers.


» Sponsor

» Partners

» Sponsor

Go Back   Linux Archive > Redhat > Device-mapper Development

 
 
LinkBack Thread Tools
 
Old 02-22-2012, 02:53 PM
Mikulas Patocka
 
Default dm: Better number validation in sscanf

dm: Better number validation in sscanf

Device mapper uses sscanf to convert arguments to numbers. The problem is that
sscanf ignores additional unmatched characters in the scanned string.

For example, this `if (sscanf(string, "%d", &number) == 1)' will match a number,
but also it will match number with some garbage appended, like "123abc".

sscanf is used this way at a lot of places in the device mapper and
as a result, device mapper accepts garbage after some numbers, for example
the command `dmsetup create vg1-new --table "0 16384 linear 254:1bla 34816bla"'
will pass without an error.

This patch fixes all sscanf uses in device mapper. The patch appends "%c" with
a pointer to a dummy character variable to every sscanf statement.

The construct `if (sscanf(string, "%d%c", &number, &dummy) == 1)' succeeds
only if string is a null-terminated number (optinally preceeded by some
whitespace characters). If there is some character appended after the number,
sscanf matches "%c", writes the character to the dummy variable and returns 2.
We check the return value for 1, consequently we reject numbers with some
garbage appended.

Signed-off-by: Mikulas Patocka <mpatocka@redhat.com>

---
drivers/md/dm-crypt.c | 8 +++++---
drivers/md/dm-delay.c | 9 +++++----
drivers/md/dm-flakey.c | 3 ++-
drivers/md/dm-ioctl.c | 5 +++--
drivers/md/dm-linear.c | 3 ++-
drivers/md/dm-log.c | 3 ++-
drivers/md/dm-mpath.c | 6 ++++--
drivers/md/dm-queue-length.c | 3 ++-
drivers/md/dm-raid1.c | 12 ++++++++----
drivers/md/dm-round-robin.c | 3 ++-
drivers/md/dm-service-time.c | 5 +++--
drivers/md/dm-stripe.c | 3 ++-
drivers/md/dm-table.c | 6 ++++--
13 files changed, 44 insertions(+), 25 deletions(-)

Index: linux-3.2.7-fast/drivers/md/dm-crypt.c
================================================== =================
--- linux-3.2.7-fast.orig/drivers/md/dm-crypt.c 2012-02-22 16:31:43.000000000 +0100
+++ linux-3.2.7-fast/drivers/md/dm-crypt.c 2012-02-22 16:33:39.000000000 +0100
@@ -1413,6 +1413,7 @@ static int crypt_ctr_cipher(struct dm_ta
char *tmp, *cipher, *chainmode, *ivmode, *ivopts, *keycount;
char *cipher_api = NULL;
int cpu, ret = -EINVAL;
+ char dummy;

/* Convert to crypto api definition? */
if (strchr(cipher_in, '(')) {
@@ -1434,7 +1435,7 @@ static int crypt_ctr_cipher(struct dm_ta

if (!keycount)
cc->tfms_count = 1;
- else if (sscanf(keycount, "%u", &cc->tfms_count) != 1 ||
+ else if (sscanf(keycount, "%u%c", &cc->tfms_count, &dummy) != 1 ||
!is_power_of_2(cc->tfms_count)) {
ti->error = "Bad cipher key count specification";
return -EINVAL;
@@ -1579,6 +1580,7 @@ static int crypt_ctr(struct dm_target *t
int ret;
struct dm_arg_set as;
const char *opt_string;
+ char dummy;

static struct dm_arg _args[] = {
{0, 1, "Invalid number of feature args"},
@@ -1636,7 +1638,7 @@ static int crypt_ctr(struct dm_target *t
}

ret = -EINVAL;
- if (sscanf(argv[2], "%llu", &tmpll) != 1) {
+ if (sscanf(argv[2], "%llu%c", &tmpll, &dummy) != 1) {
ti->error = "Invalid iv_offset sector";
goto bad;
}
@@ -1647,7 +1649,7 @@ static int crypt_ctr(struct dm_target *t
goto bad;
}

- if (sscanf(argv[4], "%llu", &tmpll) != 1) {
+ if (sscanf(argv[4], "%llu%c", &tmpll, &dummy) != 1) {
ti->error = "Invalid device sector";
goto bad;
}
Index: linux-3.2.7-fast/drivers/md/dm-delay.c
================================================== =================
--- linux-3.2.7-fast.orig/drivers/md/dm-delay.c 2012-02-22 16:31:43.000000000 +0100
+++ linux-3.2.7-fast/drivers/md/dm-delay.c 2012-02-22 16:33:39.000000000 +0100
@@ -131,6 +131,7 @@ static int delay_ctr(struct dm_target *t
{
struct delay_c *dc;
unsigned long long tmpll;
+ char dummy;

if (argc != 3 && argc != 6) {
ti->error = "requires exactly 3 or 6 arguments";
@@ -145,13 +146,13 @@ static int delay_ctr(struct dm_target *t

dc->reads = dc->writes = 0;

- if (sscanf(argv[1], "%llu", &tmpll) != 1) {
+ if (sscanf(argv[1], "%llu%c", &tmpll, &dummy) != 1) {
ti->error = "Invalid device sector";
goto bad;
}
dc->start_read = tmpll;

- if (sscanf(argv[2], "%u", &dc->read_delay) != 1) {
+ if (sscanf(argv[2], "%u%c", &dc->read_delay, &dummy) != 1) {
ti->error = "Invalid delay";
goto bad;
}
@@ -166,13 +167,13 @@ static int delay_ctr(struct dm_target *t
if (argc == 3)
goto out;

- if (sscanf(argv[4], "%llu", &tmpll) != 1) {
+ if (sscanf(argv[4], "%llu%c", &tmpll, &dummy) != 1) {
ti->error = "Invalid write device sector";
goto bad_dev_read;
}
dc->start_write = tmpll;

- if (sscanf(argv[5], "%u", &dc->write_delay) != 1) {
+ if (sscanf(argv[5], "%u%c", &dc->write_delay, &dummy) != 1) {
ti->error = "Invalid write delay";
goto bad_dev_read;
}
Index: linux-3.2.7-fast/drivers/md/dm-flakey.c
================================================== =================
--- linux-3.2.7-fast.orig/drivers/md/dm-flakey.c 2012-02-22 16:31:43.000000000 +0100
+++ linux-3.2.7-fast/drivers/md/dm-flakey.c 2012-02-22 16:33:39.000000000 +0100
@@ -160,6 +160,7 @@ static int flakey_ctr(struct dm_target *
unsigned long long tmpll;
struct dm_arg_set as;
const char *devname;
+ char dummy;

as.argc = argc;
as.argv = argv;
@@ -178,7 +179,7 @@ static int flakey_ctr(struct dm_target *

devname = dm_shift_arg(&as);

- if (sscanf(dm_shift_arg(&as), "%llu", &tmpll) != 1) {
+ if (sscanf(dm_shift_arg(&as), "%llu%c", &tmpll, &dummy) != 1) {
ti->error = "Invalid device sector";
goto bad;
}
Index: linux-3.2.7-fast/drivers/md/dm-ioctl.c
================================================== =================
--- linux-3.2.7-fast.orig/drivers/md/dm-ioctl.c 2012-02-22 16:31:43.000000000 +0100
+++ linux-3.2.7-fast/drivers/md/dm-ioctl.c 2012-02-22 16:33:39.000000000 +0100
@@ -880,6 +880,7 @@ static int dev_set_geometry(struct dm_io
struct hd_geometry geometry;
unsigned long indata[4];
char *geostr = (char *) param + param->data_start;
+ char dummy;

md = find_device(param);
if (!md)
@@ -891,8 +892,8 @@ static int dev_set_geometry(struct dm_io
goto out;
}

- x = sscanf(geostr, "%lu %lu %lu %lu", indata,
- indata + 1, indata + 2, indata + 3);
+ x = sscanf(geostr, "%lu %lu %lu %lu%c", indata,
+ indata + 1, indata + 2, indata + 3, &dummy);

if (x != 4) {
DMWARN("Unable to interpret geometry settings.");
Index: linux-3.2.7-fast/drivers/md/dm-linear.c
================================================== =================
--- linux-3.2.7-fast.orig/drivers/md/dm-linear.c 2012-02-22 16:31:43.000000000 +0100
+++ linux-3.2.7-fast/drivers/md/dm-linear.c 2012-02-22 16:33:39.000000000 +0100
@@ -29,6 +29,7 @@ static int linear_ctr(struct dm_target *
{
struct linear_c *lc;
unsigned long long tmp;
+ char dummy;

if (argc != 2) {
ti->error = "Invalid argument count";
@@ -41,7 +42,7 @@ static int linear_ctr(struct dm_target *
return -ENOMEM;
}

- if (sscanf(argv[1], "%llu", &tmp) != 1) {
+ if (sscanf(argv[1], "%llu%c", &tmp, &dummy) != 1) {
ti->error = "dm-linear: Invalid device sector";
goto bad;
}
Index: linux-3.2.7-fast/drivers/md/dm-log.c
================================================== =================
--- linux-3.2.7-fast.orig/drivers/md/dm-log.c 2012-02-22 16:31:43.000000000 +0100
+++ linux-3.2.7-fast/drivers/md/dm-log.c 2012-02-22 16:33:39.000000000 +0100
@@ -369,6 +369,7 @@ static int create_log_context(struct dm_
unsigned int region_count;
size_t bitset_size, buf_size;
int r;
+ char dummy;

if (argc < 1 || argc > 2) {
DMWARN("wrong number of arguments to dirty region log");
@@ -387,7 +388,7 @@ static int create_log_context(struct dm_
}
}

- if (sscanf(argv[0], "%u", &region_size) != 1 ||
+ if (sscanf(argv[0], "%u%c", &region_size, &dummy) != 1 ||
!_check_region_size(ti, region_size)) {
DMWARN("invalid region size %s", argv[0]);
return -EINVAL;
Index: linux-3.2.7-fast/drivers/md/dm-mpath.c
================================================== =================
--- linux-3.2.7-fast.orig/drivers/md/dm-mpath.c 2012-02-22 16:31:43.000000000 +0100
+++ linux-3.2.7-fast/drivers/md/dm-mpath.c 2012-02-22 16:33:39.000000000 +0100
@@ -1054,8 +1054,9 @@ static int switch_pg_num(struct multipat
struct priority_group *pg;
unsigned pgnum;
unsigned long flags;
+ char dummy;

- if (!pgstr || (sscanf(pgstr, "%u", &pgnum) != 1) || !pgnum ||
+ if (!pgstr || (sscanf(pgstr, "%u%c", &pgnum, &dummy) != 1) || !pgnum ||
(pgnum > m->nr_priority_groups)) {
DMWARN("invalid PG number supplied to switch_pg_num");
return -EINVAL;
@@ -1085,8 +1086,9 @@ static int bypass_pg_num(struct multipat
{
struct priority_group *pg;
unsigned pgnum;
+ char dummy;

- if (!pgstr || (sscanf(pgstr, "%u", &pgnum) != 1) || !pgnum ||
+ if (!pgstr || (sscanf(pgstr, "%u%c", &pgnum, &dummy) != 1) || !pgnum ||
(pgnum > m->nr_priority_groups)) {
DMWARN("invalid PG number supplied to bypass_pg");
return -EINVAL;
Index: linux-3.2.7-fast/drivers/md/dm-queue-length.c
================================================== =================
--- linux-3.2.7-fast.orig/drivers/md/dm-queue-length.c 2012-02-22 16:31:43.000000000 +0100
+++ linux-3.2.7-fast/drivers/md/dm-queue-length.c 2012-02-22 16:33:39.000000000 +0100
@@ -112,6 +112,7 @@ static int ql_add_path(struct path_selec
struct selector *s = ps->context;
struct path_info *pi;
unsigned repeat_count = QL_MIN_IO;
+ char dummy;

/*
* Arguments: [<repeat_count>]
@@ -123,7 +124,7 @@ static int ql_add_path(struct path_selec
return -EINVAL;
}

- if ((argc == 1) && (sscanf(argv[0], "%u", &repeat_count) != 1)) {
+ if ((argc == 1) && (sscanf(argv[0], "%u%c", &repeat_count, &dummy) != 1)) {
*error = "queue-length ps: invalid repeat count";
return -EINVAL;
}
Index: linux-3.2.7-fast/drivers/md/dm-raid1.c
================================================== =================
--- linux-3.2.7-fast.orig/drivers/md/dm-raid1.c 2012-02-22 16:31:43.000000000 +0100
+++ linux-3.2.7-fast/drivers/md/dm-raid1.c 2012-02-22 16:33:39.000000000 +0100
@@ -927,8 +927,9 @@ static int get_mirror(struct mirror_set
unsigned int mirror, char **argv)
{
unsigned long long offset;
+ char dummy;

- if (sscanf(argv[1], "%llu", &offset) != 1) {
+ if (sscanf(argv[1], "%llu%c", &offset, &dummy) != 1) {
ti->error = "Invalid offset";
return -EINVAL;
}
@@ -956,13 +957,14 @@ static struct dm_dirty_log *create_dirty
{
unsigned param_count;
struct dm_dirty_log *dl;
+ char dummy;

if (argc < 2) {
ti->error = "Insufficient mirror log arguments";
return NULL;
}

- if (sscanf(argv[1], "%u", &param_count) != 1) {
+ if (sscanf(argv[1], "%u%c", &param_count, &dummy) != 1) {
ti->error = "Invalid mirror log argument count";
return NULL;
}
@@ -989,13 +991,14 @@ static int parse_features(struct mirror_
{
unsigned num_features;
struct dm_target *ti = ms->ti;
+ char dummy;

*args_used = 0;

if (!argc)
return 0;

- if (sscanf(argv[0], "%u", &num_features) != 1) {
+ if (sscanf(argv[0], "%u%c", &num_features, &dummy) != 1) {
ti->error = "Invalid number of features";
return -EINVAL;
}
@@ -1039,6 +1042,7 @@ static int mirror_ctr(struct dm_target *
unsigned int nr_mirrors, m, args_used;
struct mirror_set *ms;
struct dm_dirty_log *dl;
+ char dummy;

dl = create_dirty_log(ti, argc, argv, &args_used);
if (!dl)
@@ -1047,7 +1051,7 @@ static int mirror_ctr(struct dm_target *
argv += args_used;
argc -= args_used;

- if (!argc || sscanf(argv[0], "%u", &nr_mirrors) != 1 ||
+ if (!argc || sscanf(argv[0], "%u%c", &nr_mirrors, &dummy) != 1 ||
nr_mirrors < 2 || nr_mirrors > DM_KCOPYD_MAX_REGIONS + 1) {
ti->error = "Invalid number of mirrors";
dm_dirty_log_destroy(dl);
Index: linux-3.2.7-fast/drivers/md/dm-round-robin.c
================================================== =================
--- linux-3.2.7-fast.orig/drivers/md/dm-round-robin.c 2012-02-22 16:31:43.000000000 +0100
+++ linux-3.2.7-fast/drivers/md/dm-round-robin.c 2012-02-22 16:33:39.000000000 +0100
@@ -114,6 +114,7 @@ static int rr_add_path(struct path_selec
struct selector *s = (struct selector *) ps->context;
struct path_info *pi;
unsigned repeat_count = RR_MIN_IO;
+ char dummy;

if (argc > 1) {
*error = "round-robin ps: incorrect number of arguments";
@@ -121,7 +122,7 @@ static int rr_add_path(struct path_selec
}

/* First path argument is number of I/Os before switching path */
- if ((argc == 1) && (sscanf(argv[0], "%u", &repeat_count) != 1)) {
+ if ((argc == 1) && (sscanf(argv[0], "%u%c", &repeat_count, &dummy) != 1)) {
*error = "round-robin ps: invalid repeat count";
return -EINVAL;
}
Index: linux-3.2.7-fast/drivers/md/dm-service-time.c
================================================== =================
--- linux-3.2.7-fast.orig/drivers/md/dm-service-time.c 2012-02-22 16:31:43.000000000 +0100
+++ linux-3.2.7-fast/drivers/md/dm-service-time.c 2012-02-22 16:33:39.000000000 +0100
@@ -110,6 +110,7 @@ static int st_add_path(struct path_selec
struct path_info *pi;
unsigned repeat_count = ST_MIN_IO;
unsigned relative_throughput = 1;
+ char dummy;

/*
* Arguments: [<repeat_count> [<relative_throughput>]]
@@ -128,13 +129,13 @@ static int st_add_path(struct path_selec
return -EINVAL;
}

- if (argc && (sscanf(argv[0], "%u", &repeat_count) != 1)) {
+ if (argc && (sscanf(argv[0], "%u%c", &repeat_count, &dummy) != 1)) {
*error = "service-time ps: invalid repeat count";
return -EINVAL;
}

if ((argc == 2) &&
- (sscanf(argv[1], "%u", &relative_throughput) != 1 ||
+ (sscanf(argv[1], "%u%c", &relative_throughput, &dummy) != 1 ||
relative_throughput > ST_MAX_RELATIVE_THROUGHPUT)) {
*error = "service-time ps: invalid relative_throughput value";
return -EINVAL;
Index: linux-3.2.7-fast/drivers/md/dm-stripe.c
================================================== =================
--- linux-3.2.7-fast.orig/drivers/md/dm-stripe.c 2012-02-22 16:31:43.000000000 +0100
+++ linux-3.2.7-fast/drivers/md/dm-stripe.c 2012-02-22 16:33:39.000000000 +0100
@@ -75,8 +75,9 @@ static int get_stripe(struct dm_target *
unsigned int stripe, char **argv)
{
unsigned long long start;
+ char dummy;

- if (sscanf(argv[1], "%llu", &start) != 1)
+ if (sscanf(argv[1], "%llu%c", &start, &dummy) != 1)
return -EINVAL;

if (dm_get_device(ti, argv[0], dm_table_get_mode(ti->table),
Index: linux-3.2.7-fast/drivers/md/dm-table.c
================================================== =================
--- linux-3.2.7-fast.orig/drivers/md/dm-table.c 2012-02-22 16:31:44.000000000 +0100
+++ linux-3.2.7-fast/drivers/md/dm-table.c 2012-02-22 16:33:39.000000000 +0100
@@ -464,10 +464,11 @@ int dm_get_device(struct dm_target *ti,
struct dm_dev_internal *dd;
unsigned int major, minor;
struct dm_table *t = ti->table;
+ char dummy;

BUG_ON(!t);

- if (sscanf(path, "%u:%u", &major, &minor) == 2) {
+ if (sscanf(path, "%u:%u%c", &major, &minor, &dummy) == 2) {
/* Extract the major/minor numbers */
dev = MKDEV(major, minor);
if (MAJOR(dev) != major || MINOR(dev) != minor)
@@ -842,9 +843,10 @@ static int validate_next_arg(struct dm_a
unsigned *value, char **error, unsigned grouped)
{
const char *arg_str = dm_shift_arg(arg_set);
+ char dummy;

if (!arg_str ||
- (sscanf(arg_str, "%u", value) != 1) ||
+ (sscanf(arg_str, "%u%c", value, &dummy) != 1) ||
(*value < arg->min) ||
(*value > arg->max) ||
(grouped && arg_set->argc < *value)) {

--
dm-devel mailing list
dm-devel@redhat.com
https://www.redhat.com/mailman/listinfo/dm-devel
 
Old 02-22-2012, 03:11 PM
Mike Snitzer
 
Default dm: Better number validation in sscanf

On Wed, Feb 22 2012 at 10:53am -0500,
Mikulas Patocka <mpatocka@redhat.com> wrote:

> dm: Better number validation in sscanf
>
> Device mapper uses sscanf to convert arguments to numbers. The problem is that
> sscanf ignores additional unmatched characters in the scanned string.
>
> For example, this `if (sscanf(string, "%d", &number) == 1)' will match a number,
> but also it will match number with some garbage appended, like "123abc".
>
> sscanf is used this way at a lot of places in the device mapper and
> as a result, device mapper accepts garbage after some numbers, for example
> the command `dmsetup create vg1-new --table "0 16384 linear 254:1bla 34816bla"'
> will pass without an error.
>
> This patch fixes all sscanf uses in device mapper. The patch appends "%c" with
> a pointer to a dummy character variable to every sscanf statement.
>
> The construct `if (sscanf(string, "%d%c", &number, &dummy) == 1)' succeeds
> only if string is a null-terminated number (optinally preceeded by some
> whitespace characters). If there is some character appended after the number,
> sscanf matches "%c", writes the character to the dummy variable and returns 2.
> We check the return value for 1, consequently we reject numbers with some
> garbage appended.
>
> Signed-off-by: Mikulas Patocka <mpatocka@redhat.com>

Looks reasonable to me.

Acked-by: Mike Snitzer <snitzer@redhat.com>

--
dm-devel mailing list
dm-devel@redhat.com
https://www.redhat.com/mailman/listinfo/dm-devel
 

Thread Tools




All times are GMT. The time now is 10:45 AM.

VBulletin, Copyright ©2000 - 2014, Jelsoft Enterprises Ltd.
Content Relevant URLs by vBSEO ©2007, Crawlability, Inc.
Copyright 2007 - 2008, www.linux-archive.org