Skip to content

Commit

Permalink
dm: reject trailing characters in sccanf input
Browse files Browse the repository at this point in the history
Device mapper uses sscanf to convert arguments to numbers. The problem is that
the way we use it 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".

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. It 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 (optionally preceded 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 and consequently reject numbers with some
garbage appended.

Signed-off-by: Mikulas Patocka <[email protected]>
Acked-by: Mike Snitzer <[email protected]>
Signed-off-by: Alasdair G Kergon <[email protected]>
  • Loading branch information
Mikulas Patocka authored and kergon committed Mar 28, 2012
1 parent 0447568 commit 31998ef
Show file tree
Hide file tree
Showing 13 changed files with 44 additions and 25 deletions.
8 changes: 5 additions & 3 deletions drivers/md/dm-crypt.c
Original file line number Diff line number Diff line change
Expand Up @@ -1415,6 +1415,7 @@ static int crypt_ctr_cipher(struct dm_target *ti,
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, '(')) {
Expand All @@ -1436,7 +1437,7 @@ static int crypt_ctr_cipher(struct dm_target *ti,

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;
Expand Down Expand Up @@ -1581,6 +1582,7 @@ static int crypt_ctr(struct dm_target *ti, unsigned int argc, char **argv)
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"},
Expand Down Expand Up @@ -1638,7 +1640,7 @@ static int crypt_ctr(struct dm_target *ti, unsigned int argc, char **argv)
}

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;
}
Expand All @@ -1649,7 +1651,7 @@ static int crypt_ctr(struct dm_target *ti, unsigned int argc, char **argv)
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;
}
Expand Down
9 changes: 5 additions & 4 deletions drivers/md/dm-delay.c
Original file line number Diff line number Diff line change
Expand Up @@ -131,6 +131,7 @@ static int delay_ctr(struct dm_target *ti, unsigned int argc, char **argv)
{
struct delay_c *dc;
unsigned long long tmpll;
char dummy;

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

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;
}
Expand All @@ -166,13 +167,13 @@ static int delay_ctr(struct dm_target *ti, unsigned int argc, char **argv)
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;
}
Expand Down
3 changes: 2 additions & 1 deletion drivers/md/dm-flakey.c
Original file line number Diff line number Diff line change
Expand Up @@ -160,6 +160,7 @@ static int flakey_ctr(struct dm_target *ti, unsigned int argc, char **argv)
unsigned long long tmpll;
struct dm_arg_set as;
const char *devname;
char dummy;

as.argc = argc;
as.argv = argv;
Expand All @@ -178,7 +179,7 @@ static int flakey_ctr(struct dm_target *ti, unsigned int argc, char **argv)

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;
}
Expand Down
5 changes: 3 additions & 2 deletions drivers/md/dm-ioctl.c
Original file line number Diff line number Diff line change
Expand Up @@ -880,6 +880,7 @@ static int dev_set_geometry(struct dm_ioctl *param, size_t param_size)
struct hd_geometry geometry;
unsigned long indata[4];
char *geostr = (char *) param + param->data_start;
char dummy;

md = find_device(param);
if (!md)
Expand All @@ -891,8 +892,8 @@ static int dev_set_geometry(struct dm_ioctl *param, size_t param_size)
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.");
Expand Down
3 changes: 2 additions & 1 deletion drivers/md/dm-linear.c
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@ static int linear_ctr(struct dm_target *ti, unsigned int argc, char **argv)
{
struct linear_c *lc;
unsigned long long tmp;
char dummy;

if (argc != 2) {
ti->error = "Invalid argument count";
Expand All @@ -41,7 +42,7 @@ static int linear_ctr(struct dm_target *ti, unsigned int argc, char **argv)
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;
}
Expand Down
3 changes: 2 additions & 1 deletion drivers/md/dm-log.c
Original file line number Diff line number Diff line change
Expand Up @@ -369,6 +369,7 @@ static int create_log_context(struct dm_dirty_log *log, struct dm_target *ti,
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");
Expand All @@ -387,7 +388,7 @@ static int create_log_context(struct dm_dirty_log *log, struct dm_target *ti,
}
}

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;
Expand Down
6 changes: 4 additions & 2 deletions drivers/md/dm-mpath.c
Original file line number Diff line number Diff line change
Expand Up @@ -1070,8 +1070,9 @@ static int switch_pg_num(struct multipath *m, const char *pgstr)
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;
Expand Down Expand Up @@ -1101,8 +1102,9 @@ static int bypass_pg_num(struct multipath *m, const char *pgstr, int bypassed)
{
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;
Expand Down
3 changes: 2 additions & 1 deletion drivers/md/dm-queue-length.c
Original file line number Diff line number Diff line change
Expand Up @@ -112,6 +112,7 @@ static int ql_add_path(struct path_selector *ps, struct dm_path *path,
struct selector *s = ps->context;
struct path_info *pi;
unsigned repeat_count = QL_MIN_IO;
char dummy;

/*
* Arguments: [<repeat_count>]
Expand All @@ -123,7 +124,7 @@ static int ql_add_path(struct path_selector *ps, struct dm_path *path,
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;
}
Expand Down
12 changes: 8 additions & 4 deletions drivers/md/dm-raid1.c
Original file line number Diff line number Diff line change
Expand Up @@ -924,8 +924,9 @@ static int get_mirror(struct mirror_set *ms, struct dm_target *ti,
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;
}
Expand Down Expand Up @@ -953,13 +954,14 @@ static struct dm_dirty_log *create_dirty_log(struct dm_target *ti,
{
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;
}
Expand All @@ -986,13 +988,14 @@ static int parse_features(struct mirror_set *ms, unsigned argc, char **argv,
{
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;
}
Expand Down Expand Up @@ -1036,6 +1039,7 @@ static int mirror_ctr(struct dm_target *ti, unsigned int argc, char **argv)
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)
Expand All @@ -1044,7 +1048,7 @@ static int mirror_ctr(struct dm_target *ti, unsigned int argc, char **argv)
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);
Expand Down
3 changes: 2 additions & 1 deletion drivers/md/dm-round-robin.c
Original file line number Diff line number Diff line change
Expand Up @@ -114,14 +114,15 @@ static int rr_add_path(struct path_selector *ps, struct dm_path *path,
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";
return -EINVAL;
}

/* 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;
}
Expand Down
5 changes: 3 additions & 2 deletions drivers/md/dm-service-time.c
Original file line number Diff line number Diff line change
Expand Up @@ -110,6 +110,7 @@ static int st_add_path(struct path_selector *ps, struct dm_path *path,
struct path_info *pi;
unsigned repeat_count = ST_MIN_IO;
unsigned relative_throughput = 1;
char dummy;

/*
* Arguments: [<repeat_count> [<relative_throughput>]]
Expand All @@ -128,13 +129,13 @@ static int st_add_path(struct path_selector *ps, struct dm_path *path,
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;
Expand Down
3 changes: 2 additions & 1 deletion drivers/md/dm-stripe.c
Original file line number Diff line number Diff line change
Expand Up @@ -75,8 +75,9 @@ static int get_stripe(struct dm_target *ti, struct stripe_c *sc,
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),
Expand Down
6 changes: 4 additions & 2 deletions drivers/md/dm-table.c
Original file line number Diff line number Diff line change
Expand Up @@ -463,10 +463,11 @@ int dm_get_device(struct dm_target *ti, const char *path, fmode_t mode,
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)
Expand Down Expand Up @@ -841,9 +842,10 @@ static int validate_next_arg(struct dm_arg *arg, struct dm_arg_set *arg_set,
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)) {
Expand Down

0 comments on commit 31998ef

Please sign in to comment.