On 09/13/2017 03:45 PM, Milan Broz wrote:
> If a crypt mapping uses optional sector_size feature, additional
> restrictions to mapped device segment size must be applied in constructor,
> otherwise the device activation will fail later.
Hi,
we had some discussion with Mikulas if this check should be better in generic DM code.
I think that for this case it is not a good idea - dm-crypt can increase
encryption sector size during load (it is stupid to do, but I see no reason why to block it).
And then only constructor of the target itself know what is possible and what should be rejected.
Anyway, there is a short reproducer what this patch solves:
Create simple mapping with 4096 encryption sector:
# dmsetup create test --table "0 8 crypt cipher_null - 0 /dev/sdb 0 1 sector_size:4096"
Now load new unaligned-length table (this should fail!)
# dmsetup load test --table "0 9 crypt cipher_null - 0 /dev/sdb 0 1 sector_size:4096"
Inactive table is apparently accepted:
# dmsetup table --inactive
test: 0 9 crypt cipher_null 0 0 8:16 0 1 sector_size:4096
And now, resume fails, keeping the device in suspended state afterward:
# dmsetup resume test
device-mapper: resume ioctl on test failed: Invalid argument
Command failed
kernel: device-mapper: table: 254:0: len=9 not aligned to h/w logical block size 4096 of sdb
# dmsetup info -c
Name Maj Min Stat Open Targ Event UUID
test 254 0 L-sw 0 1 0
With the patch applied, the load step correctly fails:
# dmsetup load test --table "0 9 crypt cipher_null - 0 /dev/sdb 0 1 sector_size:4096"
device-mapper: reload ioctl on test failed: Invalid argument
kernel: device-mapper: table: 254:0: crypt: Device size is not multiple of sector_size feature
Please consider this for 4.14 (and stable 4.12+ perhaps).
Thanks,
Milan
>
> Signed-off-by: Milan Broz <gmazyland@xxxxxxxxx>
> ---
> drivers/md/dm-crypt.c | 4 ++++
> 1 file changed, 4 insertions(+)
>
> diff --git a/drivers/md/dm-crypt.c b/drivers/md/dm-crypt.c
> index 54aef8ed97db..488ecd0b1bd0 100644
> --- a/drivers/md/dm-crypt.c
> +++ b/drivers/md/dm-crypt.c
> @@ -2584,6 +2584,10 @@ static int crypt_ctr_optional(struct dm_target *ti, unsigned int argc, char **ar
> ti->error = "Invalid feature value for sector_size";
> return -EINVAL;
> }
> + if (ti->len & ((cc->sector_size >> SECTOR_SHIFT) - 1)) {
> + ti->error = "Device size is not multiple of sector_size feature";
> + return -EINVAL;
> + }
> cc->sector_shift = __ffs(cc->sector_size) - SECTOR_SHIFT;
> } else if (!strcasecmp(opt_string, "iv_large_sectors"))
> set_bit(CRYPT_IV_LARGE_SECTORS, &cc->cipher_flags);
>
--
dm-devel mailing list
dm-devel@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/dm-devel
