[BUG] dmsetup table output changed from 2.6.18 to 2.6.19-rc3 and breaks yaird.

[BUG] dmsetup table output changed from 2.6.18 to 2.6.19-rc3 and breaks yaird.

[Stefan Schmidt]

[-- Attachment #1: Type: text/plain, Size: 833 bytes --]

Hello.

Today the build of an initrd from a 2.6.19-rc3 kernel failed.

fairlight:/boot# yaird -o /tmp/foo.initrd 2.6.19-rc3-fairlight
yaird error: Could not read output for /sbin/modprobe -v -n --show-depends --set-version 2.6.19-rc3-fairlight cbc(aes) (fatal)

dmsetup table has changed the report for luks cipher. 

dmsetup table on 2.6.18 reports: aes-cbc-essiv:sha256

dmsetup table on 2.6.19-rc3 reports: cbc(aes)-cbc-essiv:sha256

The problem seems to be on the kernel side here. Herbert Xu changed
the output with d1806f6a97a536b043fe50e6d8a25b061755cf50

http://kernel.org/git/?p=linux/kernel/git/torvalds/linux-2.6.git;a=commit;h=d1806f6a97a536b043fe50e6d8a25b061755cf50

The question is if this change was intentional and yaird should be
fixed, or it's a kernel API breakage.

regards
Stefan Schmidt

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 241 bytes --]

Re: [BUG] dmsetup table output changed from 2.6.18 to 2.6.19-rc3 and breaks yaird.

[Alasdair G Kergon]

On Mon, Oct 30, 2006 at 04:19:30PM +0100, Stefan Schmidt wrote:
> dmsetup table on 2.6.18 reports: aes-cbc-essiv:sha256
> dmsetup table on 2.6.19-rc3 reports: cbc(aes)-cbc-essiv:sha256

> The problem seems to be on the kernel side here. Herbert Xu changed
> the output with d1806f6a97a536b043fe50e6d8a25b061755cf50
> http://kernel.org/git/?p=linux/kernel/git/torvalds/linux-2.6.git;a=commit;h=d1806f6a97a536b043fe50e6d8a25b061755cf50

> The question is if this change was intentional and yaird should be
> fixed, or it's a kernel API breakage.
 
It cannot have been intentional as there was no mention of the change to the
userspace interface in the git changelog (and the interface version number
was not changed).

A new patch is needed to revert the part of the patch that changed the
userspace interface.

Please don't forget to copy in the appropriate maintainers when you send
messages like this one:
  http://marc.theaimsgroup.com/?l=linux-netdev&m=115547174417490&w=2
so they can provide acks:-)

Alasdair
-- 
agk@redhat.com

Re: [BUG] dmsetup table output changed from 2.6.18 to 2.6.19-rc3 and breaks yaird.

[Linus Torvalds]

On Mon, 30 Oct 2006, Alasdair G Kergon wrote:
>  
> It cannot have been intentional as there was no mention of the change to the
> userspace interface in the git changelog (and the interface version number
> was not changed).
> 
> A new patch is needed to revert the part of the patch that changed the
> userspace interface.
> 
> Please don't forget to copy in the appropriate maintainers when you send
> messages like this one:
>   http://marc.theaimsgroup.com/?l=linux-netdev&m=115547174417490&w=2
> so they can provide acks:-)

Yeah.

Herbert, the breakage _seems_ to be due to the STATUSTYPE_TABLE case 
change:

-		cipher = crypto_tfm_alg_name(cc->tfm);
+		cipher = crypto_blkcipher_name(cc->tfm);

which effectively changes "aes" into "cbc(aes)", which is wrong, since we 
show the chainmode separately.

Please, somebody who knows this area, send me a fix,

		Linus

----
(maybe something like this trivial one? Totally untested, but it would 
seem to be the sane approach)

diff --git a/drivers/md/dm-crypt.c b/drivers/md/dm-crypt.c
index a625576..645e3ce 100644
--- a/drivers/md/dm-crypt.c
+++ b/drivers/md/dm-crypt.c
@@ -925,8 +925,7 @@ static int crypt_status(struct dm_target
 		break;
 
 	case STATUSTYPE_TABLE:
-		cipher = crypto_blkcipher_name(cc->tfm);
-
+		cipher = cc->cipher;
 		chainmode = cc->chainmode;
 
 		if (cc->iv_mode)

Re: [BUG] dmsetup table output changed from 2.6.18 to 2.6.19-rc3 and breaks yaird.

[Alasdair G Kergon]

On Mon, Oct 30, 2006 at 11:00:29AM -0800, Linus Torvalds wrote:
> (maybe something like this trivial one? Totally untested, but it would 
> seem to be the sane approach)
 
> diff --git a/drivers/md/dm-crypt.c b/drivers/md/dm-crypt.c
> index a625576..645e3ce 100644
> --- a/drivers/md/dm-crypt.c
> +++ b/drivers/md/dm-crypt.c
> @@ -925,8 +925,7 @@ static int crypt_status(struct dm_target
>  		break;
>  
>  	case STATUSTYPE_TABLE:
> -		cipher = crypto_blkcipher_name(cc->tfm);
> -
> +		cipher = cc->cipher;
>  		chainmode = cc->chainmode;
>  
>  		if (cc->iv_mode)
> 
> 

Looks correct.

The point of STATUSTYPE_TABLE is to return (readable) output to userspace in
a format that the crypt_ctr() function would accept back in.

So crypt_ctr() now stores a private copy of cipher and chainmode for
crypt_status() to regurgitate when requested.

Alasdair
-- 
agk@redhat.com

Re: [BUG] dmsetup table output changed from 2.6.18 to 2.6.19-rc3 and breaks yaird.

[Christophe Saout]

[-- Attachment #1: Type: text/plain, Size: 2526 bytes --]

Am Montag, den 30.10.2006, 11:00 -0800 schrieb Linus Torvalds:
> On Mon, 30 Oct 2006, Alasdair G Kergon wrote:
> >  
> > It cannot have been intentional as there was no mention of the change to the
> > userspace interface in the git changelog (and the interface version number
> > was not changed).
> > 
> > A new patch is needed to revert the part of the patch that changed the
> > userspace interface.
> > 
> > Please don't forget to copy in the appropriate maintainers when you send
> > messages like this one:
> >   http://marc.theaimsgroup.com/?l=linux-netdev&m=115547174417490&w=2
> > so they can provide acks:-)
> 
> Yeah.
> 
> Herbert, the breakage _seems_ to be due to the STATUSTYPE_TABLE case 
> change:
> 
> -		cipher = crypto_tfm_alg_name(cc->tfm);
> +		cipher = crypto_blkcipher_name(cc->tfm);
> 
> which effectively changes "aes" into "cbc(aes)", which is wrong, since we 
> show the chainmode separately.
> 
> Please, somebody who knows this area, send me a fix,
> 
> (maybe something like this trivial one? Totally untested, but it would 
> seem to be the sane approach)

Yes, this works just fine. It can also be cleaned up a little further as
the temporary variables are unnecessary at that point.

----

Fix dm-crypt after the block cipher API changes to correctly return the
backwards compatible cipher-chainmode[-ivmode] format for "dmsetup
table".

Signed-off-by: Christophe Saout <christophe@saout.de>

diff linux-2.6.19-rc3.orig/drivers/md/dm-crypt.c linux-2.6.19-rc3/drivers/md/dm-crypt.c
--- linux-2.6.19-rc3.orig/drivers/md/dm-crypt.c	2006-10-26 13:17:58.000000000 +0200
+++ linux-2.6.19-rc3/drivers/md/dm-crypt.c	2006-10-30 20:26:37.000000000 +0100
@@ -915,8 +915,6 @@ static int crypt_status(struct dm_target
 			char *result, unsigned int maxlen)
 {
 	struct crypt_config *cc = (struct crypt_config *) ti->private;
-	const char *cipher;
-	const char *chainmode = NULL;
 	unsigned int sz = 0;
 
 	switch (type) {
@@ -925,14 +923,11 @@ static int crypt_status(struct dm_target
 		break;
 
 	case STATUSTYPE_TABLE:
-		cipher = crypto_blkcipher_name(cc->tfm);
-
-		chainmode = cc->chainmode;
-
 		if (cc->iv_mode)
-			DMEMIT("%s-%s-%s ", cipher, chainmode, cc->iv_mode);
+			DMEMIT("%s-%s-%s ", cc->cipher, cc->chainmode,
+			       cc->iv_mode);
 		else
-			DMEMIT("%s-%s ", cipher, chainmode);
+			DMEMIT("%s-%s ", cc->cipher, cc->chainmode);
 
 		if (cc->key_size > 0) {
 			if ((maxlen - sz) < ((cc->key_size << 1) + 1))


[-- Attachment #2: Dies ist ein digital signierter Nachrichtenteil --]
[-- Type: application/pgp-signature, Size: 189 bytes --]

Re: [BUG] dmsetup table output changed from 2.6.18 to 2.6.19-rc3 and breaks yaird.

[Stefan Schmidt]

[-- Attachment #1: Type: text/plain, Size: 241 bytes --]

Hello.

On Mon, 2006-10-30 at 11:00, Linus Torvalds wrote:
> 
> (maybe something like this trivial one? Totally untested, but it would 
> seem to be the sane approach)

I just tested it. Works fine for me.

regards
Stefan Schmidt

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 241 bytes --]

Re: [BUG] dmsetup table output changed from 2.6.18 to 2.6.19-rc3 and breaks yaird.

[Herbert Xu]

On Mon, Oct 30, 2006 at 08:39:08PM +0100, Christophe Saout wrote:
>
> > Herbert, the breakage _seems_ to be due to the STATUSTYPE_TABLE case 
> > change:
> > 
> > -		cipher = crypto_tfm_alg_name(cc->tfm);
> > +		cipher = crypto_blkcipher_name(cc->tfm);
> > 
> > which effectively changes "aes" into "cbc(aes)", which is wrong, since we 
> > show the chainmode separately.

Yes that's wrong.

However, the bigger problem is that dmcrypt's algorithm specification
does not allow all algorithms to be specified.  In particular, algorithms
with dashes in their names cannot be represented in an unambiguous way.

The separation of chain modes and algorithm names is in fact unnecessary
and only complicates matters.

For now it's no big deal since the algorithms people actually use
can be represented.  But it would be nice to get this fixed.
 
> ----
> 
> Fix dm-crypt after the block cipher API changes to correctly return the
> backwards compatible cipher-chainmode[-ivmode] format for "dmsetup
> table".
> 
> Signed-off-by: Christophe Saout <christophe@saout.de>

Looks good to me.

Acked-by: Herbert Xu <herbert@gondor.apana.org.au>

Thanks,
-- 
Visit Openswan at http://www.openswan.org/
Email: Herbert Xu ~{PmV>HI~} <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt