[RFC PATCH] dm: Check for device sector overflow if CONFIG_LBDAF is not set

* [RFC PATCH] dm: Check for device sector overflow if CONFIG_LBDAF is not set
@ 2018-11-04 13:42 Milan Broz
  2018-11-05 18:35 ` Mikulas Patocka
  0 siblings, 1 reply; 6+ messages in thread
From: Milan Broz @ 2018-11-04 13:42 UTC (permalink / raw)
  To: dm-devel; +Cc: mpatocka, Milan Broz, snitzer

Reference to a device in device-mapper table contains offset in sectors.

If the sector_t is 32bit integer (CONFIG_LBDAF is not set), then
several device-mapper targets can overflow this offset and validity
check is then performad on wrong offset and wrong table is activated.

See for example (on 32bit without CONFIG_LBDAF) this overflow:

  # dmsetup create test --table "0 2048 linear /dev/sdg 4294967297"
  # dmsetup table test
  0 2048 linear 8:96 1

In this patch I tried to add check for this problem to dm-linear and dm-crypt,
but I am sure there are more places and I am not sure this is the proper way.

Should we use uint64_t in DM internally for device offset instead?

There are probably some internal calculations in dm-table.c that
can overflow as well.

NOTE: it is a RFC patch that is incomplete (more targets need fixes).

Signed-off-by: Milan Broz <gmazyland@gmail.com>
---
 drivers/md/dm-crypt.c  | 4 ++++
 drivers/md/dm-linear.c | 4 ++++
 2 files changed, 8 insertions(+)

diff --git a/drivers/md/dm-crypt.c b/drivers/md/dm-crypt.c
index 49be7a6a2e81..008fc40ef84b 100644
--- a/drivers/md/dm-crypt.c
+++ b/drivers/md/dm-crypt.c
@@ -2786,6 +2786,10 @@ static int crypt_ctr(struct dm_target *ti, unsigned int argc, char **argv)
 		goto bad;
 	}
 	cc->start = tmpll;
+	if (sizeof(cc->start) < sizeof(tmpll) && cc->start != tmpll) {
+		ti->error = "Device sector overflow";
+		goto bad;
+	}
 
 	if (crypt_integrity_aead(cc) || cc->integrity_iv_size) {
 		ret = crypt_integrity_ctr(cc, ti);
diff --git a/drivers/md/dm-linear.c b/drivers/md/dm-linear.c
index 8d7ddee6ac4d..b5a0065d1436 100644
--- a/drivers/md/dm-linear.c
+++ b/drivers/md/dm-linear.c
@@ -50,6 +50,10 @@ static int linear_ctr(struct dm_target *ti, unsigned int argc, char **argv)
 		goto bad;
 	}
 	lc->start = tmp;
+	if (sizeof(lc->start) < sizeof(tmp) && lc->start != tmp) {
+		ti->error = "Device sector overflow";
+		goto bad;
+	}
 
 	ret = dm_get_device(ti, argv[0], dm_table_get_mode(ti->table), &lc->dev);
 	if (ret) {
-- 
2.19.1

^ permalink raw reply related	[flat|nested] 6+ messages in thread