| Message ID | 20190109105714.3606-1-gmazyland@gmail.com (mailing list archive) |
|---|---|
| State | Accepted, archived |
| Delegated to: | Mike Snitzer |
| Headers | show |
| Series | dm-crypt: Fix parsing of extended IV arguments. | expand |
On Wed, Jan 09 2019 at 5:57am -0500, Milan Broz <gmazyland@gmail.com> wrote: > The dm-crypt cipher specification in a mapping table is defined as > cipher[:keycount]-chainmode-ivmode[:ivopts] or with the new crypt API format > capi:cipher_api_spec-ivmode[:ivopts]. > > For ESSIV, the parameter includes hash specification, for example aes-cbc-essiv:sha256. > > The implementation expected that additional IV option never includes another dash '-' character > > Unfortunately, with SHA3, we have now names like sha3-256, so the mapping table parser fails: > > dmsetup create test --table "0 8 crypt aes-cbc-essiv:sha3-256 9c1185a5c5e9fc54612808977ee8f5b9e 0 /dev/sdb 0" > or (new format) > dmsetup create test --table "0 8 crypt capi:cbc(aes)-essiv:sha3-256 9c1185a5c5e9fc54612808977ee8f5b9e 0 /dev/sdb 0" > > device-mapper: crypt: Ignoring unexpected additional cipher options > device-mapper: table: 253:0: crypt: Error creating IV > device-mapper: ioctl: error adding target to table > > This patch fixes the dm-crypt constructor to ignore additional dash in IV options and also removes > bogus warning (that is ignored anyway). > > [This patch should go into stable tree as well.] Rather than this it'd be useful to just be more explicit, e.g.: Fixes: XXXXXXXX ("commit subject") Cc: stable@vger.kernel.org # > 4.x? Once I know which commit exposed us to this problem I can take care of getting this fix staged for 5.0-rcX inclussion. Thanks, Mike -- dm-devel mailing list dm-devel@redhat.com https://www.redhat.com/mailman/listinfo/dm-devel
On 10/01/2019 20:18, Mike Snitzer wrote: > On Wed, Jan 09 2019 at 5:57am -0500, > Milan Broz <gmazyland@gmail.com> wrote: >> This patch fixes the dm-crypt constructor to ignore additional dash in IV options and also removes >> bogus warning (that is ignored anyway). >> >> [This patch should go into stable tree as well.] > > Rather than this it'd be useful to just be more explicit, e.g.: There is no single reference commit. The new capi: was introduced in https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/drivers/md/dm-crypt.c?id=33d2f09fcb357fd1861c4959d1d3505492bf91f8 but I think the second part applies even to older kernel, but not sure it makes sense to fix it there (I guess not)... So perhaps the best is to apply it to stable kernel >= 4.12 Milan -- dm-devel mailing list dm-devel@redhat.com https://www.redhat.com/mailman/listinfo/dm-devel
On Thu, Jan 10 2019 at 2:50pm -0500, Milan Broz <gmazyland@gmail.com> wrote: > On 10/01/2019 20:18, Mike Snitzer wrote: > > On Wed, Jan 09 2019 at 5:57am -0500, > > Milan Broz <gmazyland@gmail.com> wrote: > > >> This patch fixes the dm-crypt constructor to ignore additional dash in IV options and also removes > >> bogus warning (that is ignored anyway). > >> > >> [This patch should go into stable tree as well.] > > > > Rather than this it'd be useful to just be more explicit, e.g.: > > There is no single reference commit. > > The new capi: was introduced in > https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/drivers/md/dm-crypt.c?id=33d2f09fcb357fd1861c4959d1d3505492bf91f8 > > but I think the second part applies even to older kernel, but not sure it makes sense to fix it there (I guess not)... > > So perhaps the best is to apply it to stable kernel >= 4.12 OK, now staged, I tweaked your header though because (like your email reply) it was lacking proper wrapping. See: https://git.kernel.org/pub/scm/linux/kernel/git/device-mapper/linux-dm.git/commit/?h=dm-5.0&id=1856b9f7bcc8e9bdcccc360aabb56fbd4dd6c565 Thanks, Mike -- dm-devel mailing list dm-devel@redhat.com https://www.redhat.com/mailman/listinfo/dm-devel
diff --git a/drivers/md/dm-crypt.c b/drivers/md/dm-crypt.c index 0ff22159a0ca..71bfb85f9652 100644 --- a/drivers/md/dm-crypt.c +++ b/drivers/md/dm-crypt.c @@ -2414,9 +2414,21 @@ static int crypt_ctr_cipher_new(struct dm_target *ti, char *cipher_in, char *key * capi:cipher_api_spec-iv:ivopts */ tmp = &cipher_in[strlen("capi:")]; - cipher_api = strsep(&tmp, "-"); - *ivmode = strsep(&tmp, ":"); - *ivopts = tmp; + + /* Separate IV options if present, it can contain another '-' in hash name */ + *ivopts = strrchr(tmp, ':'); + if (*ivopts) { + **ivopts = '\0'; + (*ivopts)++; + } + /* Parse IV mode */ + *ivmode = strrchr(tmp, '-'); + if (*ivmode) { + **ivmode = '\0'; + (*ivmode)++; + } + /* The rest is crypto API spec */ + cipher_api = tmp; if (*ivmode && !strcmp(*ivmode, "lmk")) cc->tfms_count = 64; @@ -2486,11 +2498,8 @@ static int crypt_ctr_cipher_old(struct dm_target *ti, char *cipher_in, char *key goto bad_mem; chainmode = strsep(&tmp, "-"); - *ivopts = strsep(&tmp, "-"); - *ivmode = strsep(&*ivopts, ":"); - - if (tmp) - DMWARN("Ignoring unexpected additional cipher options"); + *ivmode = strsep(&tmp, ":"); + *ivopts = tmp; /* * For compatibility with the original dm-crypt mapping format, if
The dm-crypt cipher specification in a mapping table is defined as cipher[:keycount]-chainmode-ivmode[:ivopts] or with the new crypt API format capi:cipher_api_spec-ivmode[:ivopts]. For ESSIV, the parameter includes hash specification, for example aes-cbc-essiv:sha256. The implementation expected that additional IV option never includes another dash '-' character Unfortunately, with SHA3, we have now names like sha3-256, so the mapping table parser fails: dmsetup create test --table "0 8 crypt aes-cbc-essiv:sha3-256 9c1185a5c5e9fc54612808977ee8f5b9e 0 /dev/sdb 0" or (new format) dmsetup create test --table "0 8 crypt capi:cbc(aes)-essiv:sha3-256 9c1185a5c5e9fc54612808977ee8f5b9e 0 /dev/sdb 0" device-mapper: crypt: Ignoring unexpected additional cipher options device-mapper: table: 253:0: crypt: Error creating IV device-mapper: ioctl: error adding target to table This patch fixes the dm-crypt constructor to ignore additional dash in IV options and also removes bogus warning (that is ignored anyway). [This patch should go into stable tree as well.] Signed-off-by: Milan Broz <gmazyland@gmail.com> --- drivers/md/dm-crypt.c | 25 +++++++++++++++++-------- 1 file changed, 17 insertions(+), 8 deletions(-)