[LTP PATCH 0/2] ltp: fix af_alg02 to specify control data

[LTP PATCH 0/2] ltp: fix af_alg02 to specify control data

[Eric Biggers]

It isn't clearly defined what happens if you read from an AF_ALG request
socket without previously sending the control data to begin an
encryption or decryption operation.  On some kernels the read will
return 0, while on others it will block.

Testing this corner case isn't the purpose of af_alg02; it just wants to
try to encrypt a zero-length message.  So, change it to explicitly send
a zero-length message with control data.

This fixes the test failure reported at
https://lkml.kernel.org/r/CA+G9fYtebf78TH-XpqArunHc1L6s9mHdLEbpY1EY9tSyDjp=sg@mail.gmail.com

Fixing the test in this way was also previously suggested at
https://lkml.kernel.org/r/20200702033221.GA19367@gondor.apana.org.au

Note, this patch doesn't change the fact that the read() still blocks on
pre-4.14 kernels (which is a kernel bug), and thus the timeout logic in
the test is still needed.

Eric Biggers (2):
  lib/tst_af_alg: add tst_alg_sendmsg()
  crypto/af_alg02: send message with control data before reading

 include/tst_af_alg.h               | 32 +++++++++++++++
 lib/tst_af_alg.c                   | 64 ++++++++++++++++++++++++++++++
 testcases/kernel/crypto/af_alg02.c | 21 ++++++++--
 3 files changed, 114 insertions(+), 3 deletions(-)

-- 
2.28.0

[LTP] [LTP PATCH 0/2] ltp: fix af_alg02 to specify control data

[Eric Biggers]

It isn't clearly defined what happens if you read from an AF_ALG request
socket without previously sending the control data to begin an
encryption or decryption operation.  On some kernels the read will
return 0, while on others it will block.

Testing this corner case isn't the purpose of af_alg02; it just wants to
try to encrypt a zero-length message.  So, change it to explicitly send
a zero-length message with control data.

This fixes the test failure reported at
https://lkml.kernel.org/r/CA+G9fYtebf78TH-XpqArunHc1L6s9mHdLEbpY1EY9tSyDjp=sg@mail.gmail.com

Fixing the test in this way was also previously suggested at
https://lkml.kernel.org/r/20200702033221.GA19367@gondor.apana.org.au

Note, this patch doesn't change the fact that the read() still blocks on
pre-4.14 kernels (which is a kernel bug), and thus the timeout logic in
the test is still needed.

Eric Biggers (2):
  lib/tst_af_alg: add tst_alg_sendmsg()
  crypto/af_alg02: send message with control data before reading

 include/tst_af_alg.h               | 32 +++++++++++++++
 lib/tst_af_alg.c                   | 64 ++++++++++++++++++++++++++++++
 testcases/kernel/crypto/af_alg02.c | 21 ++++++++--
 3 files changed, 114 insertions(+), 3 deletions(-)

-- 
2.28.0

[LTP PATCH 1/2] lib/tst_af_alg: add tst_alg_sendmsg()

[Eric Biggers]

From: Eric Biggers <ebiggers@google.com>

Add a helper function which sends data to an AF_ALG request socket,
including control data.  This is needed by af_alg02, but it may also be
useful for other AF_ALG tests in the future.

Signed-off-by: Eric Biggers <ebiggers@google.com>
---
 include/tst_af_alg.h | 32 ++++++++++++++++++++++
 lib/tst_af_alg.c     | 64 ++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 96 insertions(+)

diff --git a/include/tst_af_alg.h b/include/tst_af_alg.h
index fc4b1989a..fd2ff0647 100644
--- a/include/tst_af_alg.h
+++ b/include/tst_af_alg.h
@@ -133,4 +133,36 @@ int tst_alg_setup(const char *algtype, const char *algname,
 int tst_alg_setup_reqfd(const char *algtype, const char *algname,
 			const uint8_t *key, unsigned int keylen);
 
+/** Specification of control data to send to an AF_ALG request socket */
+struct tst_alg_sendmsg_params {
+
+	/** If true, send ALG_SET_OP with ALG_OP_ENCRYPT */
+	bool encrypt;
+
+	/** If true, send ALG_SET_OP with ALG_OP_DECRYPT */
+	bool decrypt;
+
+	/** If ivlen != 0, send ALG_SET_IV */
+	const uint8_t *iv;
+	unsigned int ivlen;
+
+	/** If assoclen != 0, send ALG_SET_AEAD_ASSOCLEN */
+	unsigned int assoclen;
+
+	/* Value to use as msghdr::msg_flags */
+	uint32_t msg_flags;
+};
+
+/**
+ * Send some data to an AF_ALG request socket, including control data.
+ * @param reqfd An AF_ALG request socket
+ * @param data The data to send
+ * @param datalen The length of data in bytes
+ * @param params Specification of the control data to send
+ *
+ * On failure, tst_brk() is called with TBROK.
+ */
+void tst_alg_sendmsg(int reqfd, const void *data, size_t datalen,
+		     const struct tst_alg_sendmsg_params *params);
+
 #endif /* TST_AF_ALG_H */
diff --git a/lib/tst_af_alg.c b/lib/tst_af_alg.c
index 97be548b4..d3895a83d 100644
--- a/lib/tst_af_alg.c
+++ b/lib/tst_af_alg.c
@@ -146,3 +146,67 @@ int tst_alg_setup_reqfd(const char *algtype, const char *algname,
 	close(algfd);
 	return reqfd;
 }
+
+void tst_alg_sendmsg(int reqfd, const void *data, size_t datalen,
+		     const struct tst_alg_sendmsg_params *params)
+{
+	struct iovec iov = {
+		.iov_base = (void *)data,
+		.iov_len = datalen,
+	};
+	struct msghdr msg = {
+		.msg_iov = &iov,
+		.msg_iovlen = 1,
+		.msg_flags = params->msg_flags,
+	};
+	size_t controllen;
+	uint8_t *control;
+	struct cmsghdr *cmsg;
+	struct af_alg_iv *alg_iv;
+
+	if (params->encrypt && params->decrypt)
+		tst_brk(TBROK, "Both encrypt and decrypt are specified");
+
+	controllen = 0;
+	if (params->encrypt || params->decrypt)
+		controllen += CMSG_SPACE(sizeof(uint32_t));
+	if (params->ivlen)
+		controllen += CMSG_SPACE(sizeof(struct af_alg_iv) +
+					 params->ivlen);
+	if (params->assoclen)
+		controllen += CMSG_SPACE(sizeof(uint32_t));
+
+	control = SAFE_MALLOC(controllen);
+	memset(control, 0, controllen);
+	msg.msg_control = control;
+	msg.msg_controllen = controllen;
+	cmsg = CMSG_FIRSTHDR(&msg);
+
+	if (params->encrypt || params->decrypt) {
+		cmsg->cmsg_level = SOL_ALG;
+		cmsg->cmsg_type = ALG_SET_OP;
+		cmsg->cmsg_len = CMSG_LEN(sizeof(uint32_t));
+		*(uint32_t *)CMSG_DATA(cmsg) =
+			params->encrypt ? ALG_OP_ENCRYPT : ALG_OP_DECRYPT;
+		cmsg = CMSG_NXTHDR(&msg, cmsg);
+	}
+	if (params->ivlen) {
+		cmsg->cmsg_level = SOL_ALG;
+		cmsg->cmsg_type = ALG_SET_IV;
+		cmsg->cmsg_len = CMSG_LEN(sizeof(struct af_alg_iv) +
+					  params->ivlen);
+		alg_iv = (struct af_alg_iv *)CMSG_DATA(cmsg);
+		alg_iv->ivlen = params->ivlen;
+		memcpy(alg_iv->iv, params->iv, params->ivlen);
+		cmsg = CMSG_NXTHDR(&msg, cmsg);
+	}
+	if (params->assoclen) {
+		cmsg->cmsg_level = SOL_ALG;
+		cmsg->cmsg_type = ALG_SET_AEAD_ASSOCLEN;
+		cmsg->cmsg_len = CMSG_LEN(sizeof(uint32_t));
+		*(uint32_t *)CMSG_DATA(cmsg) = params->assoclen;
+		cmsg = CMSG_NXTHDR(&msg, cmsg);
+	}
+
+	SAFE_SENDMSG(datalen, reqfd, &msg, 0);
+}
-- 
2.28.0

[LTP] [LTP PATCH 1/2] lib/tst_af_alg: add tst_alg_sendmsg()

[Eric Biggers]

From: Eric Biggers <ebiggers@google.com>

Add a helper function which sends data to an AF_ALG request socket,
including control data.  This is needed by af_alg02, but it may also be
useful for other AF_ALG tests in the future.

Signed-off-by: Eric Biggers <ebiggers@google.com>
---
 include/tst_af_alg.h | 32 ++++++++++++++++++++++
 lib/tst_af_alg.c     | 64 ++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 96 insertions(+)

diff --git a/include/tst_af_alg.h b/include/tst_af_alg.h
index fc4b1989a..fd2ff0647 100644
--- a/include/tst_af_alg.h
+++ b/include/tst_af_alg.h
@@ -133,4 +133,36 @@ int tst_alg_setup(const char *algtype, const char *algname,
 int tst_alg_setup_reqfd(const char *algtype, const char *algname,
 			const uint8_t *key, unsigned int keylen);
 
+/** Specification of control data to send to an AF_ALG request socket */
+struct tst_alg_sendmsg_params {
+
+	/** If true, send ALG_SET_OP with ALG_OP_ENCRYPT */
+	bool encrypt;
+
+	/** If true, send ALG_SET_OP with ALG_OP_DECRYPT */
+	bool decrypt;
+
+	/** If ivlen != 0, send ALG_SET_IV */
+	const uint8_t *iv;
+	unsigned int ivlen;
+
+	/** If assoclen != 0, send ALG_SET_AEAD_ASSOCLEN */
+	unsigned int assoclen;
+
+	/* Value to use as msghdr::msg_flags */
+	uint32_t msg_flags;
+};
+
+/**
+ * Send some data to an AF_ALG request socket, including control data.
+ * @param reqfd An AF_ALG request socket
+ * @param data The data to send
+ * @param datalen The length of data in bytes
+ * @param params Specification of the control data to send
+ *
+ * On failure, tst_brk() is called with TBROK.
+ */
+void tst_alg_sendmsg(int reqfd, const void *data, size_t datalen,
+		     const struct tst_alg_sendmsg_params *params);
+
 #endif /* TST_AF_ALG_H */
diff --git a/lib/tst_af_alg.c b/lib/tst_af_alg.c
index 97be548b4..d3895a83d 100644
--- a/lib/tst_af_alg.c
+++ b/lib/tst_af_alg.c
@@ -146,3 +146,67 @@ int tst_alg_setup_reqfd(const char *algtype, const char *algname,
 	close(algfd);
 	return reqfd;
 }
+
+void tst_alg_sendmsg(int reqfd, const void *data, size_t datalen,
+		     const struct tst_alg_sendmsg_params *params)
+{
+	struct iovec iov = {
+		.iov_base = (void *)data,
+		.iov_len = datalen,
+	};
+	struct msghdr msg = {
+		.msg_iov = &iov,
+		.msg_iovlen = 1,
+		.msg_flags = params->msg_flags,
+	};
+	size_t controllen;
+	uint8_t *control;
+	struct cmsghdr *cmsg;
+	struct af_alg_iv *alg_iv;
+
+	if (params->encrypt && params->decrypt)
+		tst_brk(TBROK, "Both encrypt and decrypt are specified");
+
+	controllen = 0;
+	if (params->encrypt || params->decrypt)
+		controllen += CMSG_SPACE(sizeof(uint32_t));
+	if (params->ivlen)
+		controllen += CMSG_SPACE(sizeof(struct af_alg_iv) +
+					 params->ivlen);
+	if (params->assoclen)
+		controllen += CMSG_SPACE(sizeof(uint32_t));
+
+	control = SAFE_MALLOC(controllen);
+	memset(control, 0, controllen);
+	msg.msg_control = control;
+	msg.msg_controllen = controllen;
+	cmsg = CMSG_FIRSTHDR(&msg);
+
+	if (params->encrypt || params->decrypt) {
+		cmsg->cmsg_level = SOL_ALG;
+		cmsg->cmsg_type = ALG_SET_OP;
+		cmsg->cmsg_len = CMSG_LEN(sizeof(uint32_t));
+		*(uint32_t *)CMSG_DATA(cmsg) =
+			params->encrypt ? ALG_OP_ENCRYPT : ALG_OP_DECRYPT;
+		cmsg = CMSG_NXTHDR(&msg, cmsg);
+	}
+	if (params->ivlen) {
+		cmsg->cmsg_level = SOL_ALG;
+		cmsg->cmsg_type = ALG_SET_IV;
+		cmsg->cmsg_len = CMSG_LEN(sizeof(struct af_alg_iv) +
+					  params->ivlen);
+		alg_iv = (struct af_alg_iv *)CMSG_DATA(cmsg);
+		alg_iv->ivlen = params->ivlen;
+		memcpy(alg_iv->iv, params->iv, params->ivlen);
+		cmsg = CMSG_NXTHDR(&msg, cmsg);
+	}
+	if (params->assoclen) {
+		cmsg->cmsg_level = SOL_ALG;
+		cmsg->cmsg_type = ALG_SET_AEAD_ASSOCLEN;
+		cmsg->cmsg_len = CMSG_LEN(sizeof(uint32_t));
+		*(uint32_t *)CMSG_DATA(cmsg) = params->assoclen;
+		cmsg = CMSG_NXTHDR(&msg, cmsg);
+	}
+
+	SAFE_SENDMSG(datalen, reqfd, &msg, 0);
+}
-- 
2.28.0

[LTP PATCH 2/2] crypto/af_alg02: send message with control data before reading

[Eric Biggers]

From: Eric Biggers <ebiggers@google.com>

It isn't clearly defined what happens if you read from an AF_ALG request
socket without previously sending the control data to begin an
encryption or decryption operation.  On some kernels the read will
return 0, while on others it will block.

Testing this corner case isn't the purpose of af_alg02; it just wants to
try to encrypt a zero-length message.  So, change it to explicitly send
a zero-length message with control data.

This fixes the test failure reported at
https://lkml.kernel.org/r/CA+G9fYtebf78TH-XpqArunHc1L6s9mHdLEbpY1EY9tSyDjp=sg@mail.gmail.com

Fixing the test in this way was also previously suggested at
https://lkml.kernel.org/r/20200702033221.GA19367@gondor.apana.org.au

Note, this patch doesn't change the fact that the read() still blocks on
pre-4.14 kernels (which is a kernel bug), and thus the timeout logic in
the test is still needed.

Signed-off-by: Eric Biggers <ebiggers@google.com>
---
 testcases/kernel/crypto/af_alg02.c | 21 ++++++++++++++++++---
 1 file changed, 18 insertions(+), 3 deletions(-)

diff --git a/testcases/kernel/crypto/af_alg02.c b/testcases/kernel/crypto/af_alg02.c
index fab0010c9..31d30777c 100644
--- a/testcases/kernel/crypto/af_alg02.c
+++ b/testcases/kernel/crypto/af_alg02.c
@@ -21,14 +21,29 @@
 #include <pthread.h>
 #include <errno.h>
 
+#define SALSA20_IV_SIZE       8
+#define SALSA20_MIN_KEY_SIZE  16
+
 static void *verify_encrypt(void *arg)
 {
+	const uint8_t iv[SALSA20_IV_SIZE] = { 0 };
+	const struct tst_alg_sendmsg_params params = {
+		.encrypt = true,
+		.iv = iv,
+		.ivlen = SALSA20_IV_SIZE,
+	};
 	char buf[16];
-	int reqfd = tst_alg_setup_reqfd("skcipher", "salsa20", NULL, 16);
+	int reqfd = tst_alg_setup_reqfd("skcipher", "salsa20", NULL,
+					SALSA20_MIN_KEY_SIZE);
 
-	TST_CHECKPOINT_WAKE(0);
+	/* Send a zero-length message to encrypt */
+	tst_alg_sendmsg(reqfd, NULL, 0, &params);
 
-	/* With the bug the kernel crashed here */
+	/*
+	 * Read the zero-length encrypted data.
+	 * With the bug, the kernel crashed here.
+	 */
+	TST_CHECKPOINT_WAKE(0);
 	if (read(reqfd, buf, 16) == 0)
 		tst_res(TPASS, "Successfully \"encrypted\" an empty message");
 	else
-- 
2.28.0

[LTP] [LTP PATCH 2/2] crypto/af_alg02: send message with control data before reading

[Eric Biggers]

From: Eric Biggers <ebiggers@google.com>

It isn't clearly defined what happens if you read from an AF_ALG request
socket without previously sending the control data to begin an
encryption or decryption operation.  On some kernels the read will
return 0, while on others it will block.

Testing this corner case isn't the purpose of af_alg02; it just wants to
try to encrypt a zero-length message.  So, change it to explicitly send
a zero-length message with control data.

This fixes the test failure reported at
https://lkml.kernel.org/r/CA+G9fYtebf78TH-XpqArunHc1L6s9mHdLEbpY1EY9tSyDjp=sg@mail.gmail.com

Fixing the test in this way was also previously suggested at
https://lkml.kernel.org/r/20200702033221.GA19367@gondor.apana.org.au

Note, this patch doesn't change the fact that the read() still blocks on
pre-4.14 kernels (which is a kernel bug), and thus the timeout logic in
the test is still needed.

Signed-off-by: Eric Biggers <ebiggers@google.com>
---
 testcases/kernel/crypto/af_alg02.c | 21 ++++++++++++++++++---
 1 file changed, 18 insertions(+), 3 deletions(-)

diff --git a/testcases/kernel/crypto/af_alg02.c b/testcases/kernel/crypto/af_alg02.c
index fab0010c9..31d30777c 100644
--- a/testcases/kernel/crypto/af_alg02.c
+++ b/testcases/kernel/crypto/af_alg02.c
@@ -21,14 +21,29 @@
 #include <pthread.h>
 #include <errno.h>
 
+#define SALSA20_IV_SIZE       8
+#define SALSA20_MIN_KEY_SIZE  16
+
 static void *verify_encrypt(void *arg)
 {
+	const uint8_t iv[SALSA20_IV_SIZE] = { 0 };
+	const struct tst_alg_sendmsg_params params = {
+		.encrypt = true,
+		.iv = iv,
+		.ivlen = SALSA20_IV_SIZE,
+	};
 	char buf[16];
-	int reqfd = tst_alg_setup_reqfd("skcipher", "salsa20", NULL, 16);
+	int reqfd = tst_alg_setup_reqfd("skcipher", "salsa20", NULL,
+					SALSA20_MIN_KEY_SIZE);
 
-	TST_CHECKPOINT_WAKE(0);
+	/* Send a zero-length message to encrypt */
+	tst_alg_sendmsg(reqfd, NULL, 0, &params);
 
-	/* With the bug the kernel crashed here */
+	/*
+	 * Read the zero-length encrypted data.
+	 * With the bug, the kernel crashed here.
+	 */
+	TST_CHECKPOINT_WAKE(0);
 	if (read(reqfd, buf, 16) == 0)
 		tst_res(TPASS, "Successfully \"encrypted\" an empty message");
 	else
-- 
2.28.0

Re: [LTP] [LTP PATCH 0/2] ltp: fix af_alg02 to specify control data

[Petr Vorel]

Hi Eric,

> It isn't clearly defined what happens if you read from an AF_ALG request
> socket without previously sending the control data to begin an
> encryption or decryption operation.  On some kernels the read will
> return 0, while on others it will block.

> Testing this corner case isn't the purpose of af_alg02; it just wants to
> try to encrypt a zero-length message.  So, change it to explicitly send
> a zero-length message with control data.

> This fixes the test failure reported at
> https://lkml.kernel.org/r/CA+G9fYtebf78TH-XpqArunHc1L6s9mHdLEbpY1EY9tSyDjp=sg@mail.gmail.com

> Fixing the test in this way was also previously suggested at
> https://lkml.kernel.org/r/20200702033221.GA19367@gondor.apana.org.au

> Note, this patch doesn't change the fact that the read() still blocks on
> pre-4.14 kernels (which is a kernel bug), and thus the timeout logic in
> the test is still needed.

Thanks for the fix, merged!

Kind regards,
Petr

[LTP] [LTP PATCH 0/2] ltp: fix af_alg02 to specify control data

[Petr Vorel]

Hi Eric,

> It isn't clearly defined what happens if you read from an AF_ALG request
> socket without previously sending the control data to begin an
> encryption or decryption operation.  On some kernels the read will
> return 0, while on others it will block.

> Testing this corner case isn't the purpose of af_alg02; it just wants to
> try to encrypt a zero-length message.  So, change it to explicitly send
> a zero-length message with control data.

> This fixes the test failure reported at
> https://lkml.kernel.org/r/CA+G9fYtebf78TH-XpqArunHc1L6s9mHdLEbpY1EY9tSyDjp=sg@mail.gmail.com

> Fixing the test in this way was also previously suggested at
> https://lkml.kernel.org/r/20200702033221.GA19367@gondor.apana.org.au

> Note, this patch doesn't change the fact that the read() still blocks on
> pre-4.14 kernels (which is a kernel bug), and thus the timeout logic in
> the test is still needed.

Thanks for the fix, merged!

Kind regards,
Petr