Bug#935702: [pkg-cryptsetup-devel] Bug#935702: Wrong DM device size due to integer truncation
Control: tag -1 fixed-upstream On Mon, 26 Aug 2019 at 11:08:35 +0200, Milan Broz wrote: > Fixed here > https://gitlab.com/cryptsetup/cryptsetup/commit/8f8f0b3258152a260c6a40be89b485f943f81484 Thanks, Milan! > I'll do minor release soon, but perhaps it would be better to > cherrypick the patch directly. We'll likely have to backport your patch to 2.2.0-3 since we're a bit in a rush if we want the bugfix in the upcoming point release (10.1 is scheduled on Sep 07 after a freeze of one week): bug fixes should land to unstable first per Release Team policy. -- Guilhem. signature.asc Description: PGP signature
Bug#935702: [pkg-cryptsetup-devel] Bug#935702: Wrong DM device size due to integer truncation
On 26/08/2019 08:03, Milan Broz wrote: > No need to report it upstream, I'll fix it. This is really stupid bug, all > sizes in code > must be uint_64t. I just wonder why no static analysis screamed here, I run > it on 32bit... Fixed here https://gitlab.com/cryptsetup/cryptsetup/commit/8f8f0b3258152a260c6a40be89b485f943f81484 I'll do minor release soon, but perhaps it would be better to cherrypick the patch directly. (It should be possible to apply it to 2.1.0/2.2.0) m.
Bug#935702: [pkg-cryptsetup-devel] Bug#935702: Wrong DM device size due to integer truncation
On 26/08/2019 03:28, Guilhem Moulin wrote: > On Sun, 25 Aug 2019 at 12:43:26 +, n...@waifu.club wrote: >> Not only the access to protected data is lost, the integritysetup's "open" >> operation actually succeeds. All reads on the incorrectly created DM device >> will of course fail with I/O errors due to bad integrity tags, but all >> writes will happily write wrong tags at wrong places! This makes it very >> easy for the administrator to destroy the data while trying to recover with >> --integrity-recovery-mode. > > Would you mind sharing your test vector, either here or the upstream bug > tracker at https://gitlab.com/cryptsetup/cryptsetup/issues ? While it's > clear there is a bug, I was unable to reproduce your observations in the > arguably most common cases, namely block devices formatted as LUKS1 or > LUKS2 with the default parameters (and a payload size exceeding ≥2³² > 512-bits sectors). The only function of the dm_*_target_set() family > called is dm_crypt_target_set(), and always with 0 as segment offset and > size. No need to report it upstream, I'll fix it. This is really stupid bug, all sizes in code must be uint_64t. I just wonder why no static analysis screamed here, I run it on 32bit... Thanks for the report! m.
Bug#935702: [pkg-cryptsetup-devel] Bug#935702: Wrong DM device size due to integer truncation
Control: retitle -1 DM device size ≥2³² 512-bits sectors is truncated on 32-bits platforms Control: tag -1 + upstream Hi, On Sun, 25 Aug 2019 at 12:43:26 +, n...@waifu.club wrote: > Not only the access to protected data is lost, the integritysetup's "open" > operation actually succeeds. All reads on the incorrectly created DM device > will of course fail with I/O errors due to bad integrity tags, but all > writes will happily write wrong tags at wrong places! This makes it very > easy for the administrator to destroy the data while trying to recover with > --integrity-recovery-mode. Would you mind sharing your test vector, either here or the upstream bug tracker at https://gitlab.com/cryptsetup/cryptsetup/issues ? While it's clear there is a bug, I was unable to reproduce your observations in the arguably most common cases, namely block devices formatted as LUKS1 or LUKS2 with the default parameters (and a payload size exceeding ≥2³² 512-bits sectors). The only function of the dm_*_target_set() family called is dm_crypt_target_set(), and always with 0 as segment offset and size. `cryptsetup plainOpen -b $LARGE_NUMBER` does yield a call to dm_crypt_target_set() with a truncated dmd.size, but the proper size is used before opening the device thanks to device_block_adjust(). Or does the bug only applies to integrity targets? I'm indeed able to reproduce for these targets: $ uname -r 4.19.0-5-686 $ truncate -s3T /tmp/disk.img $ losetup /dev/loop0 /tmp/disk.img $ integritysetup format -s 512 --no-wipe -q /dev/loop0 $ integritysetup dump /dev/loop0 | grep provided_data_sectors provided_data_sectors 6392379512 $ integritysetup open /dev/loop0 integrity_test $ echo $? 0 $ dmsetup table integrity_test 0 2097412216 integrity … syslog contains (checksum errors are due to no-wipe): […] device-mapper: integrity: Checksum failed at sector 0x7d03f780 […] device-mapper: integrity: Checksum failed at sector 0x7d03f780 […] Buffer I/O error on dev dm-2, logical block 262176496, async page read ## experimental --integrity extension $ cryptsetup luksFormat -q --integrity hmac-sha256 --integrity-no-wipe \ --pbkdf-force-iterations 4 --pbkdf-memory 32 /dev/loop0 << signature.asc Description: PGP signature
