[PATCH v2 5/7] keyctl_dh_compute_kdf: use a void* for opaque data

[PATCH v2 5/7] keyctl_dh_compute_kdf: use a void* for opaque data

[Ben Boeckel]

Signed-off-by: Ben Boeckel <mathstuf@gmail.com>
---
 keyutils.c              | 2 +-
 keyutils.h              | 4 ++--
 man/keyctl_dh_compute.3 | 2 +-
 3 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/keyutils.c b/keyutils.c
index a087a33..5cc90cb 100644
--- a/keyutils.c
+++ b/keyutils.c
@@ -246,7 +246,7 @@ long keyctl_dh_compute(key_serial_t priv, key_serial_t prime,
 
 long keyctl_dh_compute_kdf(key_serial_t private, key_serial_t prime,
 			   key_serial_t base, const char *hashname,
-			   const char *otherinfo, size_t otherinfolen,
+			   const void *otherinfo, size_t otherinfolen,
 			   char *buffer, size_t buflen)
 {
 	struct keyctl_dh_params params = { .priv = private,
diff --git a/keyutils.h b/keyutils.h
index 208c175..9b3848e 100644
--- a/keyutils.h
+++ b/keyutils.h
@@ -116,7 +116,7 @@ struct keyctl_dh_params {
 
 struct keyctl_kdf_params {
 	const char *hashname;
-	const char *otherinfo;
+	const void *otherinfo;
 	uint32_t otherinfolen;
 	uint32_t __spare[8];
 };
@@ -204,7 +204,7 @@ extern long keyctl_dh_compute(key_serial_t priv, key_serial_t prime,
 			      key_serial_t base, char *buffer, size_t buflen);
 extern long keyctl_dh_compute_kdf(key_serial_t private, key_serial_t prime,
 				  key_serial_t base, const char *hashname,
-				  const char *otherinfo, size_t otherinfolen,
+				  const void *otherinfo, size_t otherinfolen,
 				  char *buffer, size_t buflen);
 extern long keyctl_restrict_keyring(key_serial_t keyring, const char *type,
 				    const char *restriction);
diff --git a/man/keyctl_dh_compute.3 b/man/keyctl_dh_compute.3
index 58ad5df..7aaf4a6 100644
--- a/man/keyctl_dh_compute.3
+++ b/man/keyctl_dh_compute.3
@@ -25,7 +25,7 @@ keyctl_dh_compute_kdf \- Derive key from a Diffie-Hellman shared secret
 .BI "key_serial_t " prime ", key_serial_t " base ", void **" _buffer ");"
 .sp
 .BI "long keyctl_dh_compute_kdf(key_serial_t " private ", key_serial_t " prime ,
-.BI "key_serial_t " base ", const char *" hashname ", const char *" otherinfo ",
+.BI "key_serial_t " base ", const char *" hashname ", const void *" otherinfo ",
 .BI "size_t " otherinfolen ", char *" buffer ", size_t " buflen ");"
 .\"""""""""""""""""""""""""""""""""""""""""""""""""""""""""""""""""""""""""""""
 .SH DESCRIPTION
-- 
2.19.0.221.g150f307afc

Re: [PATCH v2 5/7] keyctl_dh_compute_kdf: use a void* for opaque data

[David Howells]

Ben Boeckel <mathstuf@gmail.com> wrote:

>  struct keyctl_kdf_params {
>  	const char *hashname;
> -	const char *otherinfo;
> +	const void *otherinfo;
>  	uint32_t otherinfolen;
>  	uint32_t __spare[8];
>  };

Hmmm...  This no longer matches the definition in the kernel's UAPI.

Also note that this *is* a significant change in C++.

David

Re: [PATCH v2 5/7] keyctl_dh_compute_kdf: use a void* for opaque data

[Ben Boeckel]

On Fri, Nov 02, 2018 at 14:45:45 +0000, David Howells wrote:
> Hmmm...  This no longer matches the definition in the kernel's UAPI.
> 
> Also note that this *is* a significant change in C++.

Well, I had a patch to also change the UAPI which you rejected because
it didn't match the code in libkeyutils. How are people other than you
supposed to fix things in both places before they land in a kernel
release? Which I suppose has now happened even though patches to just
fix things were available.

Note that I'm now on the ML to do direct review, but without a way to
reply to emails that happened before I was subscribed…

--Ben

Re: [PATCH v2 5/7] keyctl_dh_compute_kdf: use a void* for opaque data

[David Howells]

Ben Boeckel <mathstuf@gmail.com> wrote:

> Well, I had a patch to also change the UAPI which you rejected because
> it didn't match the code in libkeyutils.

That wasn't the only reason I rejected it.  I also said:

	The UAPI change might potentially cause userspace to fail to compile
	if someone was assigning the char * pointer from the struct to another
	char * pointer.

The structs need to match - something which we failed to do with
keyctl_dh_params.  I've ended up putting some magic with a cpp-conditional and
a union in the kernel for that :-/

But you shouldn't also *change* the UAPI if that will break existing
userspace.  I think that's Linus's rule #1.  And your change has the potential
to do just that.

David

Re: [PATCH v2 5/7] keyctl_dh_compute_kdf: use a void* for opaque data

[Ben Boeckel]

On Fri, Nov 02, 2018 at 16:30:21 +0000, David Howells wrote:
> But you shouldn't also *change* the UAPI if that will break existing
> userspace.  I think that's Linus's rule #1.  And your change has the potential
> to do just that.

It seems that my git archaeology missed that the struct was added in
4.12. Sorry about that. I guess late night coding didn't help there.

--Ben