linux-next: manual merge of the crypto tree with the kbuild tree

linux-next: manual merge of the crypto tree with the kbuild tree

[Stephen Rothwell]

Hi all,

Today's linux-next merge of the crypto tree got a conflict in:

  include/linux/crypto.h

between commit:

  c394d1683e7c ("cryptouser.h: fix include from userland")

from the kbuild tree and commit:

  f437a3f477cc ("crypto: api - Extend algorithm name limit to 128 bytes")

from the crypto tree.

I fixed it up (I used the former version of this file and added the
following merge fix patch) and can carry the fix as necessary. This
is now fixed as far as linux-next is concerned, but any non trivial
conflicts should be mentioned to your upstream maintainer when your tree
is submitted for merging.  You may also want to consider cooperating
with the maintainer of the conflicting tree to minimise any particularly
complex conflicts.

Someone needs to remember to send this to Linus when these trees get
merged.

From: Stephen Rothwell <sfr@canb.auug.org.au>
Date: Tue, 11 Apr 2017 10:16:43 +1000
Subject: [PATCH] crypto: merge fix for CRYPTO_MAX_ALG_NAME move

Signed-off-by: Stephen Rothwell <sfr@canb.auug.org.au>
---
 include/uapi/linux/crypto.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/include/uapi/linux/crypto.h b/include/uapi/linux/crypto.h
index e342c5a5ac50..cfebf1d23d6a 100644
--- a/include/uapi/linux/crypto.h
+++ b/include/uapi/linux/crypto.h
@@ -9,6 +9,6 @@
 #ifndef _UAPI_CRYPTO_H
 #define _UAPI_CRYPTO_H
 
-#define CRYPTO_MAX_ALG_NAME		64
+#define CRYPTO_MAX_ALG_NAME		128
 
 #endif /* _UAPI_CRYPTO_H */
-- 
2.11.0

-- 
Cheers,
Stephen Rothwell

Re: linux-next: manual merge of the crypto tree with the kbuild tree

[Herbert Xu]

On Tue, Apr 11, 2017 at 10:21:19AM +1000, Stephen Rothwell wrote:
> Hi all,
> 
> Today's linux-next merge of the crypto tree got a conflict in:
> 
>   include/linux/crypto.h
> 
> between commit:
> 
>   c394d1683e7c ("cryptouser.h: fix include from userland")
> 
> from the kbuild tree and commit:
> 
>   f437a3f477cc ("crypto: api - Extend algorithm name limit to 128 bytes")
> 
> from the crypto tree.
> 
> I fixed it up (I used the former version of this file and added the
> following merge fix patch) and can carry the fix as necessary. This
> is now fixed as far as linux-next is concerned, but any non trivial
> conflicts should be mentioned to your upstream maintainer when your tree
> is submitted for merging.  You may also want to consider cooperating
> with the maintainer of the conflicting tree to minimise any particularly
> complex conflicts.

Actually the patch in the kbuild tree should be reverted because
we have now increased the in-kernel length limit and this must not
be directly exposed to user-space or it'll break compatibility.

Thanks,
-- 
Email: Herbert Xu <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

Re: linux-next: manual merge of the crypto tree with the kbuild tree

[Stephen Rothwell]

Hi Herbert,

On Tue, 11 Apr 2017 10:42:15 +0800 Herbert Xu <herbert@gondor.apana.org.au> wrote:
>
> Actually the patch in the kbuild tree should be reverted because
> we have now increased the in-kernel length limit and this must not
> be directly exposed to user-space or it'll break compatibility.

So basically we need CRYPTO_MAX_ALG_NAME to be 64 in the exported
header but 128 in the kernel header?  In which case the kbuild patch
needs to be changed not removed.  Or the merge resolution needs to be
cleverer.

-- 
Cheers,
Stephen Rothwell

Re: linux-next: manual merge of the crypto tree with the kbuild tree

[Herbert Xu]

On Tue, Apr 11, 2017 at 03:02:50PM +1000, Stephen Rothwell wrote:
>
> So basically we need CRYPTO_MAX_ALG_NAME to be 64 in the exported
> header but 128 in the kernel header?  In which case the kbuild patch
> needs to be changed not removed.  Or the merge resolution needs to be
> cleverer.

Actually the kbuild patch just needs to be reverted.  We should
not export CRYPTO_MAX_ALG_NAME at all.  Each user-space interface
that uses it already has its own limit and should not refer to the
in-kernel value.

Cheers,
-- 
Email: Herbert Xu <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

Re: linux-next: manual merge of the crypto tree with the kbuild tree

[Masahiro Yamada]

Hi Herbert,


2017-04-11 14:12 GMT+09:00 Herbert Xu <herbert@gondor.apana.org.au>:
> On Tue, Apr 11, 2017 at 03:02:50PM +1000, Stephen Rothwell wrote:
>>
>> So basically we need CRYPTO_MAX_ALG_NAME to be 64 in the exported
>> header but 128 in the kernel header?  In which case the kbuild patch
>> needs to be changed not removed.  Or the merge resolution needs to be
>> cleverer.
>
> Actually the kbuild patch just needs to be reverted.  We should
> not export CRYPTO_MAX_ALG_NAME at all.  Each user-space interface
> that uses it already has its own limit and should not refer to the
> in-kernel value.



CRYPTO_MAX_ALG_NAME is referenced from
include/uapi/linux/cryptouser.h

If you exporting CRYPTO_MAX_ALG_NAME is wrong,
please move cryptouser.h out of the include/uapi directory.




-- 
Best Regards
Masahiro Yamada

Re: linux-next: manual merge of the crypto tree with the kbuild tree

[Herbert Xu]

On Tue, Apr 11, 2017 at 03:34:43PM +0900, Masahiro Yamada wrote:
> Hi Herbert,
> 
> 
> 2017-04-11 14:12 GMT+09:00 Herbert Xu <herbert@gondor.apana.org.au>:
> > On Tue, Apr 11, 2017 at 03:02:50PM +1000, Stephen Rothwell wrote:
> >>
> >> So basically we need CRYPTO_MAX_ALG_NAME to be 64 in the exported
> >> header but 128 in the kernel header?  In which case the kbuild patch
> >> needs to be changed not removed.  Or the merge resolution needs to be
> >> cleverer.
> >
> > Actually the kbuild patch just needs to be reverted.  We should
> > not export CRYPTO_MAX_ALG_NAME at all.  Each user-space interface
> > that uses it already has its own limit and should not refer to the
> > in-kernel value.
> 
> CRYPTO_MAX_ALG_NAME is referenced from
> include/uapi/linux/cryptouser.h
> 
> If you exporting CRYPTO_MAX_ALG_NAME is wrong,
> please move cryptouser.h out of the include/uapi directory.

It doesn't reference it anymore in the latest cryptodev tree.

So please revert the patch from the kbuild tree.

Thanks,
-- 
Email: Herbert Xu <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

Re: linux-next: manual merge of the crypto tree with the kbuild tree

[Masahiro Yamada]

Hi Herbert,


2017-04-11 15:40 GMT+09:00 Herbert Xu <herbert@gondor.apana.org.au>:
> On Tue, Apr 11, 2017 at 03:34:43PM +0900, Masahiro Yamada wrote:
>> Hi Herbert,
>>
>>
>> 2017-04-11 14:12 GMT+09:00 Herbert Xu <herbert@gondor.apana.org.au>:
>> > On Tue, Apr 11, 2017 at 03:02:50PM +1000, Stephen Rothwell wrote:
>> >>
>> >> So basically we need CRYPTO_MAX_ALG_NAME to be 64 in the exported
>> >> header but 128 in the kernel header?  In which case the kbuild patch
>> >> needs to be changed not removed.  Or the merge resolution needs to be
>> >> cleverer.
>> >
>> > Actually the kbuild patch just needs to be reverted.  We should
>> > not export CRYPTO_MAX_ALG_NAME at all.  Each user-space interface
>> > that uses it already has its own limit and should not refer to the
>> > in-kernel value.
>>
>> CRYPTO_MAX_ALG_NAME is referenced from
>> include/uapi/linux/cryptouser.h
>>
>> If you exporting CRYPTO_MAX_ALG_NAME is wrong,
>> please move cryptouser.h out of the include/uapi directory.
>
> It doesn't reference it anymore in the latest cryptodev tree.
>
> So please revert the patch from the kbuild tree.
>

Ah, right.

Commit 4473710df1f8 fixed the problem,
but reverting the patch in kbuild will break the bisect'ability
of the kbuild tree.


I will consult people about how to handle this case.



-- 
Best Regards
Masahiro Yamada

Re: linux-next: manual merge of the crypto tree with the kbuild tree

[Herbert Xu]

On Tue, Apr 11, 2017 at 04:00:45PM +0900, Masahiro Yamada wrote:
>
> Commit 4473710df1f8 fixed the problem,
> but reverting the patch in kbuild will break the bisect'ability
> of the kbuild tree.

All you will be doing is reverting to the status quo before you
applied that patch.  So I don't see how this can break anything
that wasn't already broken.

Cheers,
-- 
Email: Herbert Xu <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

Re: linux-next: manual merge of the crypto tree with the kbuild tree

[Masahiro Yamada]

Hi Linus, Stephen,


2017-04-11 14:02 GMT+09:00 Stephen Rothwell <sfr@canb.auug.org.au>:
> Hi Herbert,
>
> On Tue, 11 Apr 2017 10:42:15 +0800 Herbert Xu <herbert@gondor.apana.org.au> wrote:
>>
>> Actually the patch in the kbuild tree should be reverted because
>> we have now increased the in-kernel length limit and this must not
>> be directly exposed to user-space or it'll break compatibility.
>
> So basically we need CRYPTO_MAX_ALG_NAME to be 64 in the exported
> header but 128 in the kernel header?  In which case the kbuild patch
> needs to be changed not removed.  Or the merge resolution needs to be
> cleverer.


In the development cycle for 4.12-rc1, some patches
from Kbuild cause conflicts in linux-next from time to time.

The patches in linux-kbuild/uapi branch touched some files
in other subsystems because they are prerequisites
to export the uapi directory as-is.

Most of the conflicts are trivial to fix-up,
and they are handled nicely thanks to Stephen.

But, today's one is hard:
https://lkml.org/lkml/2017/4/10/1208

As Herbert suggested, the easiest way is to revert
c394d1683, but reverting it will cause an error in Kbuild tree:
.../linux/cryptouser.h:58:16: error: ‘CRYPTO_MAX_ALG_NAME’ undeclared
here (not in a function)


So, I will rebase the linux-kbuild/uapi branch onto Linus's tree
(resolving all conflicts) after crypto changes are pulled
during the next merge window.

Then, I will send the kbuild/uapi pull request so that Linus can pull
it with no (less) conflicts.

The commit c394d1683 will effectively be dropped.

I think this is the cleanest way to fix the issue.

Please let me know if you see problems in this plan.

Thanks.


-- 
Best Regards
Masahiro Yamada

Re: linux-next: manual merge of the crypto tree with the kbuild tree

[Herbert Xu]

On Tue, Apr 11, 2017 at 04:51:08PM +0900, Masahiro Yamada wrote:
> 
> But, today's one is hard:
> https://lkml.org/lkml/2017/4/10/1208
> 
> As Herbert suggested, the easiest way is to revert
> c394d1683, but reverting it will cause an error in Kbuild tree:
> .../linux/cryptouser.h:58:16: error: ‘CRYPTO_MAX_ALG_NAME’ undeclared
> here (not in a function)
> 
> 
> So, I will rebase the linux-kbuild/uapi branch onto Linus's tree
> (resolving all conflicts) after crypto changes are pulled
> during the next merge window.
> 
> Then, I will send the kbuild/uapi pull request so that Linus can pull
> it with no (less) conflicts.
> 
> The commit c394d1683 will effectively be dropped.
> 
> I think this is the cleanest way to fix the issue.

Another solution is to adopt the same cryptouser change in the
kbuild tree, something like this:

diff --git a/include/linux/crypto.h b/include/linux/crypto.h
index cc2425b..c0b0cf3 100644
--- a/include/linux/crypto.h
+++ b/include/linux/crypto.h
@@ -24,7 +24,6 @@
 #include <linux/slab.h>
 #include <linux/string.h>
 #include <linux/uaccess.h>
-#include <uapi/linux/crypto.h>
 
 /*
  * Autoloaded crypto modules should only use a prefixed name to avoid allowing
@@ -124,6 +123,7 @@
 /*
  * Miscellaneous stuff.
  */
+#define CRYPTO_MAX_ALG_NAME		64
 
 /*
  * The macro CRYPTO_MINALIGN_ATTR (along with the void * type in the actual
diff --git a/include/uapi/linux/crypto.h b/include/uapi/linux/crypto.h
deleted file mode 100644
index e342c5a..0000000
--- a/include/uapi/linux/crypto.h
+++ /dev/null
@@ -1,14 +0,0 @@
-/*
- * Copyright (c) 2017 Nicolas Dichtel <nicolas.dichtel@6wind.com>
- *
- * This program is free software; you can redistribute it and/or modify
- * it under the terms of the GNU General Public License version 2
- * as published by the Free Software Foundation.
- */
-
-#ifndef _UAPI_CRYPTO_H
-#define _UAPI_CRYPTO_H
-
-#define CRYPTO_MAX_ALG_NAME		64
-
-#endif /* _UAPI_CRYPTO_H */
diff --git a/include/uapi/linux/cryptouser.h b/include/uapi/linux/cryptouser.h
index 751e7da..d041fbb 100644
--- a/include/uapi/linux/cryptouser.h
+++ b/include/uapi/linux/cryptouser.h
@@ -21,7 +21,6 @@
 #ifndef _UAPI_CRYPTOUSER_H
 #define _UAPI_CRYPTOUSER_H
 
-#include <linux/crypto.h>
 #include <linux/types.h>
 
 /* Netlink configuration messages.  */
@@ -37,7 +36,7 @@ enum {
 #define CRYPTO_MSG_MAX (__CRYPTO_MSG_MAX - 1)
 #define CRYPTO_NR_MSGTYPES (CRYPTO_MSG_MAX + 1 - CRYPTO_MSG_BASE)
 
-#define CRYPTO_MAX_NAME CRYPTO_MAX_ALG_NAME
+#define CRYPTO_MAX_NAME 64
 
 /* Netlink message attributes.  */
 enum crypto_attr_type_t {
@@ -59,9 +58,9 @@ enum crypto_attr_type_t {
 };
 
 struct crypto_user_alg {
-	char cru_name[CRYPTO_MAX_ALG_NAME];
-	char cru_driver_name[CRYPTO_MAX_ALG_NAME];
-	char cru_module_name[CRYPTO_MAX_ALG_NAME];
+	char cru_name[CRYPTO_MAX_NAME];
+	char cru_driver_name[CRYPTO_MAX_NAME];
+	char cru_module_name[CRYPTO_MAX_NAME];
 	__u32 cru_type;
 	__u32 cru_mask;
 	__u32 cru_refcnt;
@@ -79,7 +78,7 @@ struct crypto_report_hash {
 };
 
 struct crypto_report_cipher {
-	char type[CRYPTO_MAX_ALG_NAME];
+	char type[CRYPTO_MAX_NAME];
 	unsigned int blocksize;
 	unsigned int min_keysize;
 	unsigned int max_keysize;
-- 
Email: Herbert Xu <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt