Hi Guilhem,
>(And added unit tests for the use case.)
thanks! I was more interested in getting my system working and did the
fix on the installed system without looking at the source package at
first.
>Thanks for the patch! FWIW crypttab(5)'s ‘offset=’ passes the value to
>`cryptsetup -o` which is a 512 byte sectors count while `blkid -O` wants
>bytes
*OUCH!* Good catch.
>, so I completed your patch with 2373709bb461a71a7af46e7e9c59355fce63e52e.
-blkid="$(/sbin/blkid -o value -s TYPE -p ${offset:+-O "$offset"} -- "$dev")"
+blkid="$(/sbin/blkid -o value -s TYPE -p ${offset:+-O "$((offset*512))"} --
"$dev")"
That’s only good for offset ≤ 4194303 though and invokes C “Undefined
Behaviour” on larger values. (More on LP64 of course.)
Worse, blkid(8) supports multiplicators, but only KiB and more, not
(512-byte) sectors, so to fix this *properly* we’d have to rely on
bc(1) or dc(1), which unfortunately Debian does not install by default
(unlike Unix). I’d say go for it but I’m not sure how majority-capable
my opinion on this is ☻
Other options:
• ignore this, which will cause misdetection for large offsets (bah)
• document that offsets larger than 4194303 are not supported
(not worth much if the underlying cryptsetup does support it)
‣ option: check for that
• check for large offsets and either “defuse” (skip and permit)
or “refuse” (skip and deny) the blkid and un_blkid checks
Checking for large offsets is also not trivial, as you cannot
do just $((offset < 4194303) for the same reason. In POSIX sh
this would work:
case x$offset in
(x)
deny 'offset is empty' # or rather just skip the -O option
;;
(x*[!0-9]*)
deny 'offset is negative or contains non-digits'
;;
(*)
if test ${#offset} -gt 7 || test "$offset" -gt 4194303; then
deny 'offset too large'
fi
;;
esac
This code first checks that it’s indeed a positive number,
then its length, and only if that’s within safe bounds,
the actual value.
All of these other than the bc(1)/dc(1) solution, however, impose
32-bit limits on 64-bit platforms. This is, unfortunately, not
avoidable because in POSIX sh it’s impossible to find out whether
the shell has 32-bit or 64-bit arithmetics without triggering UB
on 32-bit. Blaming ISO C is appropriate.
I’m attaching a first cut at my favourite solution. It’s missing
a thing I’m not sure how to achieve: ensure that bc(1) is available
in the initrd. My initramfs-tools “fu” is not very high.
Thanks,
//mirabilos
--
“It is inappropriate to require that a time represented as
seconds since the Epoch precisely represent the number of
seconds between the referenced time and the Epoch.”
-- IEEE Std 1003.1b-1993 (POSIX) Section B.2.2.2
