[LTP] [PATCH] syscalls/add_key05: add maxbytes/maxkeys test under unprivileged user

[LTP] [PATCH] syscalls/add_key05: add maxbytes/maxkeys test under unprivileged user

[Yang Xu]

Signed-off-by: Yang Xu <xuyang2018.jy@cn.fujitsu.com>
---
 runtest/syscalls                              |   1 +
 testcases/kernel/syscalls/add_key/.gitignore  |   1 +
 testcases/kernel/syscalls/add_key/add_key05.c | 180 ++++++++++++++++++
 3 files changed, 182 insertions(+)
 create mode 100644 testcases/kernel/syscalls/add_key/add_key05.c

diff --git a/runtest/syscalls b/runtest/syscalls
index 577a4a527..df7bbcbf1 100644
--- a/runtest/syscalls
+++ b/runtest/syscalls
@@ -18,6 +18,7 @@ add_key01 add_key01
 add_key02 add_key02
 add_key03 add_key03
 add_key04 add_key04
+add_key05 add_key05
 
 adjtimex01 adjtimex01
 adjtimex02 adjtimex02
diff --git a/testcases/kernel/syscalls/add_key/.gitignore b/testcases/kernel/syscalls/add_key/.gitignore
index b9a04214d..f57dc2228 100644
--- a/testcases/kernel/syscalls/add_key/.gitignore
+++ b/testcases/kernel/syscalls/add_key/.gitignore
@@ -2,3 +2,4 @@
 /add_key02
 /add_key03
 /add_key04
+/add_key05
diff --git a/testcases/kernel/syscalls/add_key/add_key05.c b/testcases/kernel/syscalls/add_key/add_key05.c
new file mode 100644
index 000000000..0d5e9db28
--- /dev/null
+++ b/testcases/kernel/syscalls/add_key/add_key05.c
@@ -0,0 +1,180 @@
+// SPDX-License-Identifier: GPL-2.0-or-later
+/*
+ * Copyright (c) 2020 FUJITSU LIMITED. All rights reserved.
+ * Author: Yang Xu <xuyang2018.jy@cn.fujitsu.com>
+ *
+ *Description:
+ * Test unprivileged user can support the number of keys and the
+ * number of bytes consumed in payloads of the keys.The defalut value
+ * is 200 and 20000.
+ * This is also a regresstion test for
+ * commit a08bf91ce28e ("KEYS: allow reaching the keys quotas exact")
+ *
+ */
+#include <stdio.h>
+#include <stdlib.h>
+#include <pwd.h>
+#include "tst_test.h"
+#include "lapi/keyctl.h"
+
+static char *user_buf, *user_buf1, *keyring_buf;
+static const char *username = "ltp_add_key05";
+static int user_added;
+struct passwd *ltpuser;
+static unsigned int used_bytes, max_bytes, used_key, max_key, data_len;
+char fmt[1024];
+int flag[2] = {1, 0};
+
+void add_user(void)
+{
+	if (user_added)
+		return;
+
+	const char *const cmd_useradd[] = {"useradd", username, NULL};
+	int rc;
+
+	switch ((rc = tst_run_cmd(cmd_useradd, NULL, NULL, 1))) {
+	case 0:
+		user_added = 1;
+		ltpuser = SAFE_GETPWNAM(username);
+		break;
+	case 1:
+	case 255:
+		break;
+	default:
+		tst_brk(TBROK, "Useradd failed (%d)", rc);
+	}
+	sprintf(fmt, "%5u: %%*5d %%*d/%%*d %%d/%%d %%d/%%d", ltpuser->pw_uid);
+}
+
+void clean_user(void)
+{
+	if (!user_added)
+		return;
+
+	const char *const cmd_userdel[] = {"userdel", "-r", username, NULL};
+
+	if (tst_run_cmd(cmd_userdel, NULL, NULL, 1))
+		tst_res(TWARN | TERRNO, "'userdel -r %s' failed", username);
+	else
+		user_added = 0;
+}
+
+void verify_max_btyes(void)
+{
+	char *buf, *invalid_buf;
+	int plen, invalid_plen;
+
+	tst_res(TINFO, "test max bytes under unprivileged user");
+	invalid_plen = max_bytes - used_bytes - data_len - 8;
+	plen = invalid_plen - 1;
+	buf = tst_alloc(plen);
+	invalid_buf = tst_alloc(invalid_plen);
+
+	TEST(add_key("user", "test_inv", invalid_buf, invalid_plen, KEY_SPEC_THREAD_KEYRING));
+	if (TST_RET != -1)
+		tst_res(TFAIL, "add_key(test_inv) succeeded unexpectedltly");
+	else {
+		if (TST_ERR == EDQUOT)
+			tst_res(TPASS | TTERRNO, "add_key(test_inv) failed as expected");
+		else
+			tst_res(TFAIL | TTERRNO, "add_key(test_inv) failed expected EDQUOT got");
+	}
+
+	TEST(add_key("user", "test_max", buf, plen, KEY_SPEC_THREAD_KEYRING));
+	if (TST_RET != -1) {
+		tst_res(TPASS, "add_key(test_max) succeeded as expected");
+		SAFE_FILE_LINES_SCANF("/proc/key-users", fmt, &used_key, &max_key, &used_bytes, &max_bytes);
+		if (used_bytes == max_bytes)
+			tst_res(TPASS, "allow reaching the max bytes exactly");
+		else
+			tst_res(TFAIL, "max used bytes %u, key allow max bytes %u", used_bytes, max_bytes);
+	} else
+		tst_res(TFAIL | TTERRNO, "add_key(test_max) failed unexpectedly");
+}
+
+void verify_max_keys(void)
+{
+	unsigned int i;
+	char desc[10];
+
+	tst_res(TINFO, "test max keys under unprivileged user");
+	for (i = used_key + 1; i < max_key; i++) {
+		sprintf(desc, "abc%d", i);
+		TEST(add_key("keyring", desc, keyring_buf, 0, KEY_SPEC_THREAD_KEYRING));
+		if (TST_RET == -1)
+			tst_res(TFAIL | TTERRNO, "add keyring key(%s) failed", desc);
+	}
+
+	TEST(add_key("keyring", "abc200", keyring_buf, 0, KEY_SPEC_THREAD_KEYRING));
+	if (TST_RET == -1) {
+		tst_res(TFAIL | TTERRNO, "add keyring key(abc200) failed");
+		goto count;
+	} else
+		tst_res(TPASS, "add keyring key(abc200) succedd");
+
+	TEST(add_key("keyring", "abc201", keyring_buf, 0, KEY_SPEC_THREAD_KEYRING));
+	if (TST_RET != -1) {
+		tst_res(TFAIL, "add keyring key(abc201) succeeded unexpectedly");
+		goto count;
+	} else {
+		if (TST_ERR == EDQUOT)
+			tst_res(TPASS | TTERRNO, "add keyring key(abc201) failed as expected over max key");
+		else
+			tst_res(TFAIL | TTERRNO, "add_keyring failed expected EDQUOT got");
+	}
+count:
+	SAFE_FILE_LINES_SCANF("/proc/key-users", fmt, &used_key, &max_key, &used_bytes, &max_bytes);
+	if (used_key == max_key)
+		tst_res(TPASS, "allow reaching the max key exactly");
+	else
+		tst_res(TFAIL, "max used key %u, key allow max key %u", used_key, max_key);
+}
+
+static void do_test(unsigned int n)
+{
+	add_user();
+	int f_used_bytes = 0;
+
+	if (!SAFE_FORK()) {
+		SAFE_SETUID(ltpuser->pw_uid);
+
+		TEST(add_key("user", "test1", user_buf, 64, KEY_SPEC_THREAD_KEYRING));
+		if (TST_RET == -1)
+			tst_brk(TFAIL | TTERRNO, "add key test1 failed");
+		SAFE_FILE_LINES_SCANF("/proc/key-users", fmt, &used_key, &max_key, &used_bytes, &max_bytes);
+		f_used_bytes = used_bytes;
+
+		TEST(add_key("user", "test2", user_buf1, 64, KEY_SPEC_THREAD_KEYRING));
+		if (TST_RET == -1)
+			tst_brk(TFAIL | TTERRNO, "add key test2 failed");
+		SAFE_FILE_LINES_SCANF("/proc/key-users", fmt, &used_key, &max_key, &used_bytes, &max_bytes);
+
+		data_len = used_bytes - f_used_bytes - strlen("test1") - 1 - 64;
+		if (flag[n])
+			verify_max_btyes();
+		else
+			verify_max_keys();
+		exit(0);
+	}
+	tst_reap_children();
+	clean_user();
+}
+
+static struct tst_test test = {
+	.test = do_test,
+	.tcnt = 2,
+	.needs_root = 1,
+	.forks_child = 1,
+	.cleanup = clean_user,
+	.bufs = (struct tst_buffers []) {
+		{&keyring_buf, .size = 1},
+		{&user_buf, .size = 64},
+		{&user_buf1, .size = 64},
+		{}
+	},
+	.tags = (const struct tst_tag[]) {
+		{"linux-git", "a08bf91ce28"},
+		{}
+	}
+};
-- 
2.18.0

[LTP] [PATCH] syscalls/add_key05: add maxbytes/maxkeys test under unprivileged user

[Cyril Hrubis]

Hi!
> diff --git a/runtest/syscalls b/runtest/syscalls
> index 577a4a527..df7bbcbf1 100644
> --- a/runtest/syscalls
> +++ b/runtest/syscalls
> @@ -18,6 +18,7 @@ add_key01 add_key01
>  add_key02 add_key02
>  add_key03 add_key03
>  add_key04 add_key04
> +add_key05 add_key05
>  
>  adjtimex01 adjtimex01
>  adjtimex02 adjtimex02
> diff --git a/testcases/kernel/syscalls/add_key/.gitignore b/testcases/kernel/syscalls/add_key/.gitignore
> index b9a04214d..f57dc2228 100644
> --- a/testcases/kernel/syscalls/add_key/.gitignore
> +++ b/testcases/kernel/syscalls/add_key/.gitignore
> @@ -2,3 +2,4 @@
>  /add_key02
>  /add_key03
>  /add_key04
> +/add_key05
> diff --git a/testcases/kernel/syscalls/add_key/add_key05.c b/testcases/kernel/syscalls/add_key/add_key05.c
> new file mode 100644
> index 000000000..0d5e9db28
> --- /dev/null
> +++ b/testcases/kernel/syscalls/add_key/add_key05.c
> @@ -0,0 +1,180 @@
> +// SPDX-License-Identifier: GPL-2.0-or-later
> +/*
> + * Copyright (c) 2020 FUJITSU LIMITED. All rights reserved.
> + * Author: Yang Xu <xuyang2018.jy@cn.fujitsu.com>
> + *
> + *Description:
> + * Test unprivileged user can support the number of keys and the
> + * number of bytes consumed in payloads of the keys.The defalut value
> + * is 200 and 20000.
> + * This is also a regresstion test for
> + * commit a08bf91ce28e ("KEYS: allow reaching the keys quotas exact")
> + *
> + */
> +#include <stdio.h>
> +#include <stdlib.h>
> +#include <pwd.h>
> +#include "tst_test.h"
> +#include "lapi/keyctl.h"
> +
> +static char *user_buf, *user_buf1, *keyring_buf;
> +static const char *username = "ltp_add_key05";
> +static int user_added;
> +struct passwd *ltpuser;
> +static unsigned int used_bytes, max_bytes, used_key, max_key, data_len;
> +char fmt[1024];
> +int flag[2] = {1, 0};
> +
> +void add_user(void)
> +{
> +	if (user_added)
> +		return;
> +
> +	const char *const cmd_useradd[] = {"useradd", username, NULL};
> +	int rc;
> +
> +	switch ((rc = tst_run_cmd(cmd_useradd, NULL, NULL, 1))) {
> +	case 0:
> +		user_added = 1;
> +		ltpuser = SAFE_GETPWNAM(username);
> +		break;
> +	case 1:
> +	case 255:
> +		break;
> +	default:
> +		tst_brk(TBROK, "Useradd failed (%d)", rc);
> +	}
> +	sprintf(fmt, "%5u: %%*5d %%*d/%%*d %%d/%%d %%d/%%d", ltpuser->pw_uid);
> +}
> +
> +void clean_user(void)
> +{
> +	if (!user_added)
> +		return;
> +
> +	const char *const cmd_userdel[] = {"userdel", "-r", username, NULL};
> +
> +	if (tst_run_cmd(cmd_userdel, NULL, NULL, 1))
> +		tst_res(TWARN | TERRNO, "'userdel -r %s' failed", username);
> +	else
> +		user_added = 0;
> +}
> +
> +void verify_max_btyes(void)
> +{
> +	char *buf, *invalid_buf;
> +	int plen, invalid_plen;
> +
> +	tst_res(TINFO, "test max bytes under unprivileged user");
> +	invalid_plen = max_bytes - used_bytes - data_len - 8;

What is the -8 for, strlen("test_inv")?

I guess that it will be more readable if we used the strlen() here as
well.

> +	plen = invalid_plen - 1;
> +	buf = tst_alloc(plen);
> +	invalid_buf = tst_alloc(invalid_plen);
> +
> +	TEST(add_key("user", "test_inv", invalid_buf, invalid_plen, KEY_SPEC_THREAD_KEYRING));
> +	if (TST_RET != -1)
> +		tst_res(TFAIL, "add_key(test_inv) succeeded unexpectedltly");
> +	else {
> +		if (TST_ERR == EDQUOT)
> +			tst_res(TPASS | TTERRNO, "add_key(test_inv) failed as expected");
> +		else
> +			tst_res(TFAIL | TTERRNO, "add_key(test_inv) failed expected EDQUOT got");
> +	}
> +
> +	TEST(add_key("user", "test_max", buf, plen, KEY_SPEC_THREAD_KEYRING));
> +	if (TST_RET != -1) {
> +		tst_res(TPASS, "add_key(test_max) succeeded as expected");
> +		SAFE_FILE_LINES_SCANF("/proc/key-users", fmt, &used_key, &max_key, &used_bytes, &max_bytes);
> +		if (used_bytes == max_bytes)
> +			tst_res(TPASS, "allow reaching the max bytes exactly");
> +		else
> +			tst_res(TFAIL, "max used bytes %u, key allow max bytes %u", used_bytes, max_bytes);
> +	} else
> +		tst_res(TFAIL | TTERRNO, "add_key(test_max) failed unexpectedly");
> +}
> +
> +void verify_max_keys(void)
> +{
> +	unsigned int i;
> +	char desc[10];
> +
> +	tst_res(TINFO, "test max keys under unprivileged user");
> +	for (i = used_key + 1; i < max_key; i++) {
> +		sprintf(desc, "abc%d", i);
> +		TEST(add_key("keyring", desc, keyring_buf, 0, KEY_SPEC_THREAD_KEYRING));
> +		if (TST_RET == -1)
> +			tst_res(TFAIL | TTERRNO, "add keyring key(%s) failed", desc);

Why we are using the "keyring" ring here instead? I doubt that it
matters for the quota, but it just seems strange.

Also we should stop the loop on a first failure, I guess that goto
count: would suffice.

> +	}
> +
> +	TEST(add_key("keyring", "abc200", keyring_buf, 0, KEY_SPEC_THREAD_KEYRING));
> +
> +	if (TST_RET == -1) {
> +		tst_res(TFAIL | TTERRNO, "add keyring key(abc200) failed");
> +		goto count;
> +	} else
> +		tst_res(TPASS, "add keyring key(abc200) succedd");

Why don't we just start the loop above at used_key? There is no point in
adding the last key here outside of the loop.

> +	TEST(add_key("keyring", "abc201", keyring_buf, 0, KEY_SPEC_THREAD_KEYRING));
> +	if (TST_RET != -1) {
> +		tst_res(TFAIL, "add keyring key(abc201) succeeded unexpectedly");
> +		goto count;

We should add a key with a different name than the previous "abc%d"
pattern here, if the upper limit was over 200 we would just replace a
key here instread which will not fail at all.

> +	} else {
> +		if (TST_ERR == EDQUOT)
> +			tst_res(TPASS | TTERRNO, "add keyring key(abc201) failed as expected over max key");
> +		else
> +			tst_res(TFAIL | TTERRNO, "add_keyring failed expected EDQUOT got");
> +	}
> +count:
> +	SAFE_FILE_LINES_SCANF("/proc/key-users", fmt, &used_key, &max_key, &used_bytes, &max_bytes);
> +	if (used_key == max_key)
> +		tst_res(TPASS, "allow reaching the max key exactly");
> +	else
> +		tst_res(TFAIL, "max used key %u, key allow max key %u", used_key, max_key);
> +}
> +
> +static void do_test(unsigned int n)
> +{
> +	add_user();
> +	int f_used_bytes = 0;
> +
> +	if (!SAFE_FORK()) {
> +		SAFE_SETUID(ltpuser->pw_uid);
> +
> +		TEST(add_key("user", "test1", user_buf, 64, KEY_SPEC_THREAD_KEYRING));
> +		if (TST_RET == -1)
> +			tst_brk(TFAIL | TTERRNO, "add key test1 failed");
> +		SAFE_FILE_LINES_SCANF("/proc/key-users", fmt, &used_key, &max_key, &used_bytes, &max_bytes);
> +		f_used_bytes = used_bytes;
> +
> +		TEST(add_key("user", "test2", user_buf1, 64, KEY_SPEC_THREAD_KEYRING));
> +		if (TST_RET == -1)
> +			tst_brk(TFAIL | TTERRNO, "add key test2 failed");

Do we really need to pass a different users_buf to each call?

> +		SAFE_FILE_LINES_SCANF("/proc/key-users", fmt, &used_key, &max_key, &used_bytes, &max_bytes);
> +
> +		data_len = used_bytes - f_used_bytes - strlen("test1") - 1 - 64;

So this code here is used to determine how much data will consume the
key structure in kernel itself? I guess that this is useless to run in
the case of testing for maximal number of keys, right? Can we put this
code into a function something as get_kernel_key_data_size() and call it
from the verify_max_bytes() only?

I guess that the SAFE_FILE_LINES_SCANF() intializes the max_key for the
max_keys test. Having a global variables that are initalized in random
places makes the code really confusing, can we avoid that if poissible
please?

I guess that we can at least create:

parse_proc_key_users(int *used_keys, int *max_key, int *used_bytes, *int max_bytes)

function that would read the values into a local variables and copy the
results only if non-NULL pointers were passed.

That way the verify_max_keys() would call:
parse_proc_key_users(NULL, &max_keys, NULL, NULL);
and use the result for the loop that adds the keys.

> +		if (flag[n])

Huh, why not just if (n)?

> +			verify_max_btyes();
> +		else
> +			verify_max_keys();
> +		exit(0);
> +	}
> +	tst_reap_children();
> +	clean_user();

What is the reason to add and remove a user for each iteration?

Aare you cleaning the keys that way?

> +}
> +
> +static struct tst_test test = {
> +	.test = do_test,
> +	.tcnt = 2,
> +	.needs_root = 1,
> +	.forks_child = 1,
> +	.cleanup = clean_user,
> +	.bufs = (struct tst_buffers []) {
> +		{&keyring_buf, .size = 1},
> +		{&user_buf, .size = 64},
> +		{&user_buf1, .size = 64},
> +		{}
> +	},
> +	.tags = (const struct tst_tag[]) {
> +		{"linux-git", "a08bf91ce28"},
> +		{}
> +	}
> +};
> -- 
> 2.18.0
> 
> 
> 
> 
> -- 
> Mailing list info: https://lists.linux.it/listinfo/ltp

-- 
Cyril Hrubis
chrubis@suse.cz

[LTP] [PATCH] syscalls/add_key05: add maxbytes/maxkeys test under unprivileged user

[Yang Xu]

Hi!

> Hi!
>> diff --git a/runtest/syscalls b/runtest/syscalls
>> index 577a4a527..df7bbcbf1 100644
>> --
>> +void verify_max_btyes(void)
>> +{
>> +	char *buf, *invalid_buf;
>> +	int plen, invalid_plen;
>> +
>> +	tst_res(TINFO, "test max bytes under unprivileged user");
>> +	invalid_plen = max_bytes - used_bytes - data_len - 8;
> 
> What is the -8 for, strlen("test_inv")?
Yes.
> 
> I guess that it will be more readable if we used the strlen() here as
> well.
OK. I will use strlen.
> 
>> +	plen = invalid_plen - 1;
>> +	buf = tst_alloc(plen);
>> +	invalid_buf = tst_alloc(invalid_plen);
>> +
>> +	TEST(add_key("user", "test_inv", invalid_buf, invalid_plen, KEY_SPEC_THREAD_KEYRING));
>> +	if (TST_RET != -1)
>> +		tst_res(TFAIL, "add_key(test_inv) succeeded unexpectedltly");
>> +	else {
>> +		if (TST_ERR == EDQUOT)
>> +			tst_res(TPASS | TTERRNO, "add_key(test_inv) failed as expected");
>> +		else
>> +			tst_res(TFAIL | TTERRNO, "add_key(test_inv) failed expected EDQUOT got");
>> +	}
>> +
>> +	TEST(add_key("user", "test_max", buf, plen, KEY_SPEC_THREAD_KEYRING));
>> +	if (TST_RET != -1) {
>> +		tst_res(TPASS, "add_key(test_max) succeeded as expected");
>> +		SAFE_FILE_LINES_SCANF("/proc/key-users", fmt, &used_key, &max_key, &used_bytes, &max_bytes);
>> +		if (used_bytes == max_bytes)
>> +			tst_res(TPASS, "allow reaching the max bytes exactly");
>> +		else
>> +			tst_res(TFAIL, "max used bytes %u, key allow max bytes %u", used_bytes, max_bytes);
>> +	} else
>> +		tst_res(TFAIL | TTERRNO, "add_key(test_max) failed unexpectedly");
>> +}
>> +
>> +void verify_max_keys(void)
>> +{
>> +	unsigned int i;
>> +	char desc[10];
>> +
>> +	tst_res(TINFO, "test max keys under unprivileged user");
>> +	for (i = used_key + 1; i < max_key; i++) {
>> +		sprintf(desc, "abc%d", i);
>> +		TEST(add_key("keyring", desc, keyring_buf, 0, KEY_SPEC_THREAD_KEYRING));
>> +		if (TST_RET == -1)
>> +			tst_res(TFAIL | TTERRNO, "add keyring key(%s) failed", desc);
> 
> Why we are using the "keyring" ring here instead? I doubt that it
> matters for the quota, but it just seems strange.
It doesn't matter quota, only because "keyrings" plen is 0. I will use 
"user" type for this.
  >
> Also we should stop the loop on a first failure, I guess that goto
> count: would suffice.
I ignored this before, thanks.
> 
>> +	}
>> +
>> +	TEST(add_key("keyring", "abc200", keyring_buf, 0, KEY_SPEC_THREAD_KEYRING));
>> +
>> +	if (TST_RET == -1) {
>> +		tst_res(TFAIL | TTERRNO, "add keyring key(abc200) failed");
>> +		goto count;
>> +	} else
>> +		tst_res(TPASS, "add keyring key(abc200) succedd");
> 
> Why don't we just start the loop above at used_key? There is no point in
> adding the last key here outside of the loop.
> 
Using start + 1 more clean?
abc100 >> the 100th key,
We can change code (including last key)as below:
for (i = used_key + 1; i <= max_key; i ++)

>> +	TEST(add_key("keyring", "abc201", keyring_buf, 0, KEY_SPEC_THREAD_KEYRING));
>> +	if (TST_RET != -1) {
>> +		tst_res(TFAIL, "add keyring key(abc201) succeeded unexpectedly");
>> +		goto count;
> 
> We should add a key with a different name than the previous "abc%d"
> pattern here, if the upper limit was over 200 we would just replace a
> key here instread which will not fail at all.
Do you mean to use  "invalid_num_key" to avoid upper limit over 200?

> 
>> +	} else {
>> +		if (TST_ERR == EDQUOT)
>> +			tst_res(TPASS | TTERRNO, "add keyring key(abc201) failed as expected over max key");
>> +		else
>> +			tst_res(TFAIL | TTERRNO, "add_keyring failed expected EDQUOT got");
>> +	}
>> +count:
>> +	SAFE_FILE_LINES_SCANF("/proc/key-users", fmt, &used_key, &max_key, &used_bytes, &max_bytes);
>> +	if (used_key == max_key)
>> +		tst_res(TPASS, "allow reaching the max key exactly");
>> +	else
>> +		tst_res(TFAIL, "max used key %u, key allow max key %u", used_key, max_key);
>> +}
>> +
>> +static void do_test(unsigned int n)
>> +{
>> +	add_user();
>> +	int f_used_bytes = 0;
>> +
>> +	if (!SAFE_FORK()) {
>> +		SAFE_SETUID(ltpuser->pw_uid);
>> +
>> +		TEST(add_key("user", "test1", user_buf, 64, KEY_SPEC_THREAD_KEYRING));
>> +		if (TST_RET == -1)
>> +			tst_brk(TFAIL | TTERRNO, "add key test1 failed");
>> +		SAFE_FILE_LINES_SCANF("/proc/key-users", fmt, &used_key, &max_key, &used_bytes, &max_bytes);
>> +		f_used_bytes = used_bytes;
>> +
>> +		TEST(add_key("user", "test2", user_buf1, 64, KEY_SPEC_THREAD_KEYRING));
>> +		if (TST_RET == -1)
>> +			tst_brk(TFAIL | TTERRNO, "add key test2 failed");
> 
> Do we really need to pass a different users_buf to each call?
I have seen kernel code, payload is used to instantiate or update the 
key, I think it is no problme to use the same buf because we don't get 
it again from kernel space.
> 
>> +		SAFE_FILE_LINES_SCANF("/proc/key-users", fmt, &used_key, &max_key, &used_bytes, &max_bytes);
>> +
>> +		data_len = used_bytes - f_used_bytes - strlen("test1") - 1 - 64;
> 
> So this code here is used to determine how much data will consume the
> key structure in kernel itself? 
I debug this code after adding test1 key, show in /proc/keys
add_key05.c:154: INFO: 1535d151 I--Q---    24 perm 3f030000     0     0 
keyring   _ses: 1

add_key05.c:154: INFO: 15d7df09 I--Q---     6 perm 1f3f0000     0 65534 
keyring   _uid.0: empty

add_key05.c:154: INFO: 1a300c72 I--Q---     1 perm 3f010000  1003     0 
user      test1: 64

add_key05.c:154: INFO: 2e57738c I--Q---     1 perm 3f010000  1003     0 
keyring   _tid: 1

the key num is 2 in /proc/key-users, I use this code to figure keyring 
  _tid: 1(I don't know why has it) consumed data bytes.
I guess that this is useless to run in
> the case of testing for maximal number of keys, right? Can we put this
> code into a function something as get_kernel_key_data_size() and call it
> from the verify_max_bytes() only?
Yes, it was only used in max_bytes.
> 
> I guess that the SAFE_FILE_LINES_SCANF() intializes the max_key for the
> max_keys test. Having a global variables that are initalized in random
> places makes the code really confusing, can we avoid that if poissible
> please?
Ok. I will avoid this and make code clean.
> 
> I guess that we can at least create:
> 
> parse_proc_key_users(int *used_keys, int *max_key, int *used_bytes, *int max_bytes)
> 
> function that would read the values into a local variables and copy the
> results only if non-NULL pointers were passed.
> 
> That way the verify_max_keys() would call:
> parse_proc_key_users(NULL, &max_keys, NULL, NULL);
> and use the result for the loop that adds the keys.
Sound good, I will use it.
> 
>> +		if (flag[n])
> 
> Huh, why not just if (n)?
> 
OK.
>> +			verify_max_btyes();
>> +		else
>> +			verify_max_keys();
>> +		exit(0);
>> +	}
>> +	tst_reap_children();
>> +	clean_user();
> 
> What is the reason to add and remove a user for each iteration?
> 
> Aare you cleaning the keys that way?
Yes, I planed to look for a wise (try KEYCTL_CLEAR
  or KEYCTL_INVALIDATE)way but failed. Can you give me some
suggestion?
> 
>> +}
>> +
>> +static struct tst_test test = {
>> +	.test = do_test,
>> +	.tcnt = 2,
>> +	.needs_root = 1,
>> +	.forks_child = 1,
>> +	.cleanup = clean_user,
>> +	.bufs = (struct tst_buffers []) {
>> +		{&keyring_buf, .size = 1},
>> +		{&user_buf, .size = 64},
>> +		{&user_buf1, .size = 64},
>> +		{}
>> +	},
>> +	.tags = (const struct tst_tag[]) {
>> +		{"linux-git", "a08bf91ce28"},
>> +		{}
>> +	}
>> +};
>> -- 
>> 2.18.0
>>
>>
>>
>>
>> -- 
>> Mailing list info: https://lists.linux.it/listinfo/ltp
> 

[LTP] [PATCH] syscalls/add_key05: add maxbytes/maxkeys test under unprivileged user

[Yang Xu]

Hi!

> Hi!
> 
>> Hi!
>>> diff --git a/runtest/syscalls b/runtest/syscalls
>>> index 577a4a527..df7bbcbf1 100644
>>> -- 
>>> +void verify_max_btyes(void)
>>> +{
>>> +??? char *buf, *invalid_buf;
>>> +??? int plen, invalid_plen;
>>> +
>>> +??? tst_res(TINFO, "test max bytes under unprivileged user");
>>> +??? invalid_plen = max_bytes - used_bytes - data_len - 8;
>>
>> What is the -8 for, strlen("test_inv")?
> Yes.
>>
>> I guess that it will be more readable if we used the strlen() here as
>> well.
> OK. I will use strlen.
>>
>>> +??? plen = invalid_plen - 1;
>>> +??? buf = tst_alloc(plen);
>>> +??? invalid_buf = tst_alloc(invalid_plen);
>>> +
>>> +??? TEST(add_key("user", "test_inv", invalid_buf, invalid_plen, 
>>> KEY_SPEC_THREAD_KEYRING));
>>> +??? if (TST_RET != -1)
>>> +??????? tst_res(TFAIL, "add_key(test_inv) succeeded unexpectedltly");
>>> +??? else {
>>> +??????? if (TST_ERR == EDQUOT)
>>> +??????????? tst_res(TPASS | TTERRNO, "add_key(test_inv) failed as 
>>> expected");
>>> +??????? else
>>> +??????????? tst_res(TFAIL | TTERRNO, "add_key(test_inv) failed 
>>> expected EDQUOT got");
>>> +??? }
>>> +
>>> +??? TEST(add_key("user", "test_max", buf, plen, 
>>> KEY_SPEC_THREAD_KEYRING));
>>> +??? if (TST_RET != -1) {
>>> +??????? tst_res(TPASS, "add_key(test_max) succeeded as expected");
>>> +??????? SAFE_FILE_LINES_SCANF("/proc/key-users", fmt, &used_key, 
>>> &max_key, &used_bytes, &max_bytes);
>>> +??????? if (used_bytes == max_bytes)
>>> +??????????? tst_res(TPASS, "allow reaching the max bytes exactly");
>>> +??????? else
>>> +??????????? tst_res(TFAIL, "max used bytes %u, key allow max bytes 
>>> %u", used_bytes, max_bytes);
>>> +??? } else
>>> +??????? tst_res(TFAIL | TTERRNO, "add_key(test_max) failed 
>>> unexpectedly");
>>> +}
>>> +
>>> +void verify_max_keys(void)
>>> +{
>>> +??? unsigned int i;
>>> +??? char desc[10];
>>> +
>>> +??? tst_res(TINFO, "test max keys under unprivileged user");
>>> +??? for (i = used_key + 1; i < max_key; i++) {
>>> +??????? sprintf(desc, "abc%d", i);
>>> +??????? TEST(add_key("keyring", desc, keyring_buf, 0, 
>>> KEY_SPEC_THREAD_KEYRING));
>>> +??????? if (TST_RET == -1)
>>> +??????????? tst_res(TFAIL | TTERRNO, "add keyring key(%s) failed", 
>>> desc);
>>
>> Why we are using the "keyring" ring here instead? I doubt that it
>> matters for the quota, but it just seems strange.
> It doesn't matter quota, only because "keyrings" plen is 0. I will use 
> "user" type for this.
>  ?>
>> Also we should stop the loop on a first failure, I guess that goto
>> count: would suffice.
> I ignored this before, thanks.
>>
>>> +??? }
>>> +
>>> +??? TEST(add_key("keyring", "abc200", keyring_buf, 0, 
>>> KEY_SPEC_THREAD_KEYRING));
>>> +
>>> +??? if (TST_RET == -1) {
>>> +??????? tst_res(TFAIL | TTERRNO, "add keyring key(abc200) failed");
>>> +??????? goto count;
>>> +??? } else
>>> +??????? tst_res(TPASS, "add keyring key(abc200) succedd");
>>
>> Why don't we just start the loop above at used_key? There is no point in
>> adding the last key here outside of the loop.
>>
> Using start + 1 more clean?
> abc100 >> the 100th key,
> We can change code (including last key)as below:
> for (i = used_key + 1; i <= max_key; i ++)
> 
>>> +??? TEST(add_key("keyring", "abc201", keyring_buf, 0, 
>>> KEY_SPEC_THREAD_KEYRING));
>>> +??? if (TST_RET != -1) {
>>> +??????? tst_res(TFAIL, "add keyring key(abc201) succeeded 
>>> unexpectedly");
>>> +??????? goto count;
>>
>> We should add a key with a different name than the previous "abc%d"
>> pattern here, if the upper limit was over 200 we would just replace a
>> key here instread which will not fail at all.
> Do you mean to use? "invalid_num_key" to avoid upper limit over 200?
> 
>>
>>> +??? } else {
>>> +??????? if (TST_ERR == EDQUOT)
>>> +??????????? tst_res(TPASS | TTERRNO, "add keyring key(abc201) failed 
>>> as expected over max key");
>>> +??????? else
>>> +??????????? tst_res(TFAIL | TTERRNO, "add_keyring failed expected 
>>> EDQUOT got");
>>> +??? }
>>> +count:
>>> +??? SAFE_FILE_LINES_SCANF("/proc/key-users", fmt, &used_key, 
>>> &max_key, &used_bytes, &max_bytes);
>>> +??? if (used_key == max_key)
>>> +??????? tst_res(TPASS, "allow reaching the max key exactly");
>>> +??? else
>>> +??????? tst_res(TFAIL, "max used key %u, key allow max key %u", 
>>> used_key, max_key);
>>> +}
>>> +
>>> +static void do_test(unsigned int n)
>>> +{
>>> +??? add_user();
>>> +??? int f_used_bytes = 0;
>>> +
>>> +??? if (!SAFE_FORK()) {
>>> +??????? SAFE_SETUID(ltpuser->pw_uid);
>>> +
>>> +??????? TEST(add_key("user", "test1", user_buf, 64, 
>>> KEY_SPEC_THREAD_KEYRING));
>>> +??????? if (TST_RET == -1)
>>> +??????????? tst_brk(TFAIL | TTERRNO, "add key test1 failed");
>>> +??????? SAFE_FILE_LINES_SCANF("/proc/key-users", fmt, &used_key, 
>>> &max_key, &used_bytes, &max_bytes);
>>> +??????? f_used_bytes = used_bytes;
>>> +
>>> +??????? TEST(add_key("user", "test2", user_buf1, 64, 
>>> KEY_SPEC_THREAD_KEYRING));
>>> +??????? if (TST_RET == -1)
>>> +??????????? tst_brk(TFAIL | TTERRNO, "add key test2 failed");
>>
>> Do we really need to pass a different users_buf to each call?
> I have seen kernel code, payload is used to instantiate or update the 
> key, I think it is no problme to use the same buf because we don't get 
> it again from kernel space.
>>
>>> +??????? SAFE_FILE_LINES_SCANF("/proc/key-users", fmt, &used_key, 
>>> &max_key, &used_bytes, &max_bytes);
>>> +
>>> +??????? data_len = used_bytes - f_used_bytes - strlen("test1") - 1 - 
>>> 64;
>>
>> So this code here is used to determine how much data will consume the
>> key structure in kernel itself? 

Sorry for the wrong info. This code is designed to compute delta, I have 
seen kernel code yestorday:
add_key calltrace as below:
add_key()
   key_create_or_update()
     key_alloc()
     __key_instantiate_and_link
       generic_key_instantiate
         key_payload_reserve
           ......

In key_alloc, we use type->def_datalen to alloc space, but when 
instantiating key, we using delta(quota_len -type->def_datalen) to 
adjust reserver space. So I compute this variable(delta, add_key05.c 
uses data_len) to avoid  that we can reach max bytes in key_alloc but 
got EDQOUT in key_payload_reserve.

more info:
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/Documentation/security/keys/core.rst#n1353

Also, using my v2 patch(send later), this case also fails on lastest 
kernel because of incomplete kernel commit.

more info:
https://patchwork.kernel.org/patch/11411507/

Best Regards
Yang Xu
> I debug this code after adding test1 key, show in /proc/keys
> add_key05.c:154: INFO: 1535d151 I--Q---??? 24 perm 3f030000???? 0???? 0 
> keyring?? _ses: 1
> 
> add_key05.c:154: INFO: 15d7df09 I--Q---???? 6 perm 1f3f0000???? 0 65534 
> keyring?? _uid.0: empty
> 
> add_key05.c:154: INFO: 1a300c72 I--Q---???? 1 perm 3f010000? 1003???? 0 
> user????? test1: 64
> 
> add_key05.c:154: INFO: 2e57738c I--Q---???? 1 perm 3f010000? 1003???? 0 
> keyring?? _tid: 1
> 
> the key num is 2 in /proc/key-users, I use this code to figure keyring 
>  ?_tid: 1(I don't know why has it) consumed data bytes.
> I guess that this is useless to run in
>> the case of testing for maximal number of keys, right? Can we put this
>> code into a function something as get_kernel_key_data_size() and call it
>> from the verify_max_bytes() only?
> Yes, it was only used in max_bytes.
>>
>> I guess that the SAFE_FILE_LINES_SCANF() intializes the max_key for the
>> max_keys test. Having a global variables that are initalized in random
>> places makes the code really confusing, can we avoid that if poissible
>> please?
> Ok. I will avoid this and make code clean.
>>
>> I guess that we can at least create:
>>
>> parse_proc_key_users(int *used_keys, int *max_key, int *used_bytes, 
>> *int max_bytes)
>>
>> function that would read the values into a local variables and copy the
>> results only if non-NULL pointers were passed.
>>
>> That way the verify_max_keys() would call:
>> parse_proc_key_users(NULL, &max_keys, NULL, NULL);
>> and use the result for the loop that adds the keys.
> Sound good, I will use it.
>>
>>> +??????? if (flag[n])
>>
>> Huh, why not just if (n)?
>>
> OK.
>>> +??????????? verify_max_btyes();
>>> +??????? else
>>> +??????????? verify_max_keys();
>>> +??????? exit(0);
>>> +??? }
>>> +??? tst_reap_children();
>>> +??? clean_user();
>>
>> What is the reason to add and remove a user for each iteration?
>>
>> Aare you cleaning the keys that way?
> Yes, I planed to look for a wise (try KEYCTL_CLEAR
>  ?or KEYCTL_INVALIDATE)way but failed. Can you give me some
> suggestion?
>>
>>> +}
>>> +
>>> +static struct tst_test test = {
>>> +??? .test = do_test,
>>> +??? .tcnt = 2,
>>> +??? .needs_root = 1,
>>> +??? .forks_child = 1,
>>> +??? .cleanup = clean_user,
>>> +??? .bufs = (struct tst_buffers []) {
>>> +??????? {&keyring_buf, .size = 1},
>>> +??????? {&user_buf, .size = 64},
>>> +??????? {&user_buf1, .size = 64},
>>> +??????? {}
>>> +??? },
>>> +??? .tags = (const struct tst_tag[]) {
>>> +??????? {"linux-git", "a08bf91ce28"},
>>> +??????? {}
>>> +??? }
>>> +};
>>> -- 
>>> 2.18.0
>>>
>>>
>>>
>>>
>>> -- 
>>> Mailing list info: https://lists.linux.it/listinfo/ltp
>>

[LTP] [PATCH v2] syscalls/add_key05: add maxbytes/maxkeys test under unprivileged user

[Yang Xu]

This case is designed to test whether we can reach maxbytes/maxkeys
quota exactly under unprivileged users. It is a regression test for
commit a08bf91ce28e ("KEYS: allow reaching the keys quotas exactly").
But this kenrel commit is a incomplete fix, it still fails on 5.6.3-rc1.
Fix patch[1] is still under review.

[1]https://patchwork.kernel.org/patch/11411507/

Signed-off-by: Yang Xu <xuyang2018.jy@cn.fujitsu.com>
---
v1->v2:
1. using parse_proc_key_user
2. add some messages about delta(v1 uses data_len)
3. fix something pointed by Cyril
 runtest/syscalls                              |   1 +
 testcases/kernel/syscalls/add_key/.gitignore  |   1 +
 testcases/kernel/syscalls/add_key/add_key05.c | 211 ++++++++++++++++++
 3 files changed, 213 insertions(+)
 create mode 100644 testcases/kernel/syscalls/add_key/add_key05.c

diff --git a/runtest/syscalls b/runtest/syscalls
index a9faf1e25..0462a08ef 100644
--- a/runtest/syscalls
+++ b/runtest/syscalls
@@ -18,6 +18,7 @@ add_key01 add_key01
 add_key02 add_key02
 add_key03 add_key03
 add_key04 add_key04
+add_key05 add_key05
 
 adjtimex01 adjtimex01
 adjtimex02 adjtimex02
diff --git a/testcases/kernel/syscalls/add_key/.gitignore b/testcases/kernel/syscalls/add_key/.gitignore
index b9a04214d..f57dc2228 100644
--- a/testcases/kernel/syscalls/add_key/.gitignore
+++ b/testcases/kernel/syscalls/add_key/.gitignore
@@ -2,3 +2,4 @@
 /add_key02
 /add_key03
 /add_key04
+/add_key05
diff --git a/testcases/kernel/syscalls/add_key/add_key05.c b/testcases/kernel/syscalls/add_key/add_key05.c
new file mode 100644
index 000000000..66b303544
--- /dev/null
+++ b/testcases/kernel/syscalls/add_key/add_key05.c
@@ -0,0 +1,211 @@
+// SPDX-License-Identifier: GPL-2.0-or-later
+/*
+ * Copyright (c) 2020 FUJITSU LIMITED. All rights reserved.
+ * Author: Yang Xu <xuyang2018.jy@cn.fujitsu.com>
+ *
+ *Description:
+ * Test unprivileged user can support the number of keys and the
+ * number of bytes consumed in payloads of the keys.The defalut value
+ * is 200 and 20000.
+ * This is also a regresstion test for
+ * commit a08bf91ce28e ("KEYS: allow reaching the keys quotas exactly")
+ *
+ */
+#include <stdio.h>
+#include <stdlib.h>
+#include <pwd.h>
+#include "tst_test.h"
+#include "lapi/keyctl.h"
+
+static char *user_buf;
+static const char *username = "ltp_add_key05";
+static int user_added;
+struct passwd *ltpuser;
+static char fmt[1024];
+
+static void add_user(void)
+{
+	if (user_added)
+		return;
+
+	const char *const cmd_useradd[] = {"useradd", username, NULL};
+	int rc;
+
+	switch ((rc = tst_run_cmd(cmd_useradd, NULL, NULL, 1))) {
+	case 0:
+		user_added = 1;
+		ltpuser = SAFE_GETPWNAM(username);
+		break;
+	case 1:
+	case 255:
+		break;
+	default:
+		tst_brk(TBROK, "Useradd failed (%d)", rc);
+	}
+	sprintf(fmt, "%5u: %%*5d %%*d/%%*d %%d/%%d %%d/%%d", ltpuser->pw_uid);
+}
+
+static void clean_user(void)
+{
+	if (!user_added)
+		return;
+
+	const char *const cmd_userdel[] = {"userdel", "-r", username, NULL};
+
+	if (tst_run_cmd(cmd_userdel, NULL, NULL, 1))
+		tst_res(TWARN | TERRNO, "'userdel -r %s' failed", username);
+	else
+		user_added = 0;
+}
+
+static inline void parse_proc_key_users(int *used_key, int *max_key, int *used_bytes, int *max_bytes)
+{
+	unsigned int val[4];
+	SAFE_FILE_LINES_SCANF("/proc/key-users", fmt, &val[0], &val[1], &val[2], &val[3]);
+
+	if (used_key)
+		*used_key = val[0];
+	if (max_key)
+		*max_key = val[1];
+	if (used_bytes)
+		*used_bytes = val[2];
+	if (max_bytes)
+		*max_bytes = val[3];
+}
+
+static void verify_max_btyes(void)
+{
+	char *buf;
+	int plen, invalid_plen, delta;
+	int used_bytes, max_bytes, tmp_used_bytes;
+
+	tst_res(TINFO, "test max bytes under unprivileged user");
+
+	parse_proc_key_users(NULL, NULL, &tmp_used_bytes, NULL);
+	TEST(add_key("user", "test2", user_buf, 64, KEY_SPEC_THREAD_KEYRING));
+	if (TST_RET == -1) {
+		tst_res(TFAIL | TTERRNO, "add key test2 failed");
+		return;
+	}
+	parse_proc_key_users(NULL, NULL, &used_bytes, &max_bytes);
+
+	/*
+	 * Compute delta between default datalen(in key_alloc) and actual
+	 * datlen(in key_payload_reserve).
+	 * more info see kernel code: security/keys/key.c
+	 */
+	delta = used_bytes - tmp_used_bytes - strlen("test2") - 1 - 64;
+	invalid_plen = max_bytes - used_bytes - delta - strlen("test_xxx");
+	buf = tst_alloc(invalid_plen);
+
+	TEST(add_key("user", "test_inv", buf, invalid_plen, KEY_SPEC_THREAD_KEYRING));
+	if (TST_RET != -1) {
+		tst_res(TFAIL, "add_key(test_inv) succeeded unexpectedltly");
+		return;
+	}
+	if (TST_ERR == EDQUOT)
+		tst_res(TPASS | TTERRNO, "add_key(test_inv) failed as expected");
+	else
+		tst_res(TFAIL | TTERRNO, "add_key(test_inv) failed expected EDQUOT got");
+
+	/*Reset delta*/
+	TEST(add_key("user", "test3", user_buf, 64, KEY_SPEC_THREAD_KEYRING));
+	if (TST_RET == -1) {
+		tst_res(TFAIL | TTERRNO, "add key test3 failed");
+		return;
+	}
+	TEST(add_key("user", "test4", user_buf, 64, KEY_SPEC_THREAD_KEYRING));
+	if (TST_RET == -1) {
+		tst_res(TFAIL | TTERRNO, "add key test4 failed");
+		return;
+	}
+	parse_proc_key_users(NULL, NULL, &used_bytes, &max_bytes);
+	plen = max_bytes - used_bytes - delta - strlen("test_xxx") - 1;
+	TEST(add_key("user", "test_max", buf, plen, KEY_SPEC_THREAD_KEYRING));
+	if (TST_RET == -1) {
+		 tst_res(TFAIL | TTERRNO, "add_key(test_max) failed unexpectedly");
+		 return;
+	}
+
+	tst_res(TPASS, "add_key(test_max) succeeded as expected");
+	parse_proc_key_users(NULL, NULL, &tmp_used_bytes, &max_bytes);
+	if (tmp_used_bytes == max_bytes)
+		tst_res(TPASS, "allow reaching the max bytes exactly");
+	else
+		tst_res(TFAIL, "max used bytes %u, key allow max bytes %u", tmp_used_bytes, max_bytes);
+}
+
+static void verify_max_keys(void)
+{
+	int i, used_key, max_key, used_bytes, max_bytes;
+	char desc[10];
+	int plen;
+
+	tst_res(TINFO, "test max keys under unprivileged user");
+	parse_proc_key_users(&used_key, &max_key, &used_bytes, &max_bytes);
+	plen = (max_bytes - used_bytes) / (max_key - used_key);
+	if (plen > 64)
+		plen = 64;
+
+	for (i = used_key + 1; i <= max_key; i++) {
+		sprintf(desc, "abc%d", i);
+		TEST(add_key("user", desc, user_buf, 64, KEY_SPEC_THREAD_KEYRING));
+		if (TST_RET == -1) {
+			tst_res(TFAIL | TTERRNO, "add keyring key(%s) failed", desc);
+			goto count;
+		}
+	}
+
+	TEST(add_key("user", "test_invalid_key", user_buf, 64, KEY_SPEC_THREAD_KEYRING));
+	if (TST_RET != -1) {
+		tst_res(TFAIL, "add keyring key(test_invalid_key) succeeded unexpectedly");
+		goto count;
+	}
+	if (TST_ERR == EDQUOT)
+		tst_res(TPASS | TTERRNO, "add_key(test_invalid_key) failed as expected");
+	else
+		tst_res(TFAIL | TTERRNO, "add_key(test_invalid_key) failed expected EDQUOT got");
+
+count:
+	parse_proc_key_users(&used_key, &max_key, NULL, NULL);
+	if (used_key == max_key)
+		tst_res(TPASS, "allow reaching the max key(%u) exactly", max_key);
+	else
+		tst_res(TFAIL, "max used key %u, key allow max key %u", used_key, max_key);
+}
+
+static void do_test(unsigned int n)
+{
+	add_user();
+	if (!SAFE_FORK()) {
+		SAFE_SETUID(ltpuser->pw_uid);
+		TEST(add_key("user", "test1", user_buf, 64, KEY_SPEC_THREAD_KEYRING));
+		if (TST_RET == -1) {
+			tst_res(TFAIL | TTERRNO, "add key test1 failed");
+			return;
+		}
+		if (n)
+			verify_max_keys();
+		else
+			verify_max_btyes();
+		exit(0);
+	}
+	tst_reap_children();
+	clean_user();
+}
+
+static struct tst_test test = {
+	.test = do_test,
+	.tcnt = 2,
+	.needs_root = 1,
+	.forks_child = 1,
+	.cleanup = clean_user,
+	.bufs = (struct tst_buffers []) {
+		{&user_buf, .size = 64},
+		{}
+	},
+	.tags = (const struct tst_tag[]) {
+		{"linux-git", "a08bf91ce28"},
+		{}
+	}
+};
-- 
2.18.0

[LTP] [PATCH v2] syscalls/add_key05: add maxbytes/maxkeys test under unprivileged user

[Petr Vorel]

Hi Xu,

thanks for your patch.

> This case is designed to test whether we can reach maxbytes/maxkeys
> quota exactly under unprivileged users. It is a regression test for
> commit a08bf91ce28e ("KEYS: allow reaching the keys quotas exactly").
> But this kenrel commit is a incomplete fix, it still fails on 5.6.3-rc1.
> Fix patch[1] is still under review.

> [1]https://patchwork.kernel.org/patch/11411507/
It's already in next tree, congratulations :).
https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git/commit/?h=next-20200317&id=2e356101e72ab1361821b3af024d64877d9a798d

> Signed-off-by: Yang Xu <xuyang2018.jy@cn.fujitsu.com>
> ---
> v1->v2:
> 1. using parse_proc_key_user
> 2. add some messages about delta(v1 uses data_len)
> 3. fix something pointed by Cyril
...

> +++ b/testcases/kernel/syscalls/add_key/add_key05.c
> @@ -0,0 +1,211 @@
> +// SPDX-License-Identifier: GPL-2.0-or-later
> +/*
> + * Copyright (c) 2020 FUJITSU LIMITED. All rights reserved.
> + * Author: Yang Xu <xuyang2018.jy@cn.fujitsu.com>
> + *
> + *Description:
Few formatting like comments (sorry for pointing out unimportant nits, but with
little care it can be avoided, of course this can be fixed by person who merges):
nit: missing space after *. I also tend to avoid text like "Description"
(it's obvious it's a description).

> + * Test unprivileged user can support the number of keys and the
> + * number of bytes consumed in payloads of the keys.The defalut value
nit: missing space after dot.

> + * is 200 and 20000.
> + * This is also a regresstion test for
> + * commit a08bf91ce28e ("KEYS: allow reaching the keys quotas exactly")
It's not bad to mention this commit, but maybe not needed since there is
linux-git tag.
> + *
Why extra line with * ?
> + */
> +#include <stdio.h>
> +#include <stdlib.h>
> +#include <pwd.h>
> +#include "tst_test.h"
> +#include "lapi/keyctl.h"
> +
> +static char *user_buf;
> +static const char *username = "ltp_add_key05";
> +static int user_added;
> +struct passwd *ltpuser;
> +static char fmt[1024];
> +
> +static void add_user(void)
> +{
> +	if (user_added)
> +		return;
> +
> +	const char *const cmd_useradd[] = {"useradd", username, NULL};
> +	int rc;
> +
> +	switch ((rc = tst_run_cmd(cmd_useradd, NULL, NULL, 1))) {
> +	case 0:
> +		user_added = 1;
> +		ltpuser = SAFE_GETPWNAM(username);
> +		break;
> +	case 1:
> +	case 255:
> +		break;
> +	default:
> +		tst_brk(TBROK, "Useradd failed (%d)", rc);
> +	}
> +	sprintf(fmt, "%5u: %%*5d %%*d/%%*d %%d/%%d %%d/%%d", ltpuser->pw_uid);
> +}
I've been planning to add functionality like this to the library for some time.
(there is identical code in setpriority01.c and other tests would benefit from that).
But no need to stop this patch just for this, I've created issue for it:
https://github.com/linux-test-project/ltp/issues/468

> +
> +static void clean_user(void)
> +{
> +	if (!user_added)
> +		return;
> +
> +	const char *const cmd_userdel[] = {"userdel", "-r", username, NULL};
> +
> +	if (tst_run_cmd(cmd_userdel, NULL, NULL, 1))
> +		tst_res(TWARN | TERRNO, "'userdel -r %s' failed", username);
> +	else
> +		user_added = 0;
> +}
> +
> +static inline void parse_proc_key_users(int *used_key, int *max_key, int *used_bytes, int *max_bytes)
> +{
> +	unsigned int val[4];
> +	SAFE_FILE_LINES_SCANF("/proc/key-users", fmt, &val[0], &val[1], &val[2], &val[3]);
> +
> +	if (used_key)
> +		*used_key = val[0];
> +	if (max_key)
> +		*max_key = val[1];
> +	if (used_bytes)
> +		*used_bytes = val[2];
> +	if (max_bytes)
> +		*max_bytes = val[3];
> +}
> +
> +static void verify_max_btyes(void)
> +{
> +	char *buf;
> +	int plen, invalid_plen, delta;
> +	int used_bytes, max_bytes, tmp_used_bytes;
> +
> +	tst_res(TINFO, "test max bytes under unprivileged user");
> +
> +	parse_proc_key_users(NULL, NULL, &tmp_used_bytes, NULL);
> +	TEST(add_key("user", "test2", user_buf, 64, KEY_SPEC_THREAD_KEYRING));
> +	if (TST_RET == -1) {
> +		tst_res(TFAIL | TTERRNO, "add key test2 failed");
> +		return;
> +	}
> +	parse_proc_key_users(NULL, NULL, &used_bytes, &max_bytes);
> +
> +	/*
> +	 * Compute delta between default datalen(in key_alloc) and actual
> +	 * datlen(in key_payload_reserve).
> +	 * more info see kernel code: security/keys/key.c
> +	 */
> +	delta = used_bytes - tmp_used_bytes - strlen("test2") - 1 - 64;
> +	invalid_plen = max_bytes - used_bytes - delta - strlen("test_xxx");
> +	buf = tst_alloc(invalid_plen);
> +
> +	TEST(add_key("user", "test_inv", buf, invalid_plen, KEY_SPEC_THREAD_KEYRING));
> +	if (TST_RET != -1) {
> +		tst_res(TFAIL, "add_key(test_inv) succeeded unexpectedltly");
> +		return;
> +	}
> +	if (TST_ERR == EDQUOT)
> +		tst_res(TPASS | TTERRNO, "add_key(test_inv) failed as expected");
> +	else
> +		tst_res(TFAIL | TTERRNO, "add_key(test_inv) failed expected EDQUOT got");
> +
> +	/*Reset delta*/
> +	TEST(add_key("user", "test3", user_buf, 64, KEY_SPEC_THREAD_KEYRING));
> +	if (TST_RET == -1) {
> +		tst_res(TFAIL | TTERRNO, "add key test3 failed");
> +		return;
> +	}
> +	TEST(add_key("user", "test4", user_buf, 64, KEY_SPEC_THREAD_KEYRING));
> +	if (TST_RET == -1) {
> +		tst_res(TFAIL | TTERRNO, "add key test4 failed");
> +		return;
> +	}
> +	parse_proc_key_users(NULL, NULL, &used_bytes, &max_bytes);
> +	plen = max_bytes - used_bytes - delta - strlen("test_xxx") - 1;
> +	TEST(add_key("user", "test_max", buf, plen, KEY_SPEC_THREAD_KEYRING));
> +	if (TST_RET == -1) {
> +		 tst_res(TFAIL | TTERRNO, "add_key(test_max) failed unexpectedly");
> +		 return;
Hm, I get this failure, I guess both are because missing your fix
2e356101e72ab1361821b3af024d64877d9a798d, right?

add_key05.c:106: PASS: add_key(test_inv) failed as expected: EDQUOT (122)
add_key05.c:125: FAIL: add_key(test_max) failed unexpectedly: EDQUOT (122)
no crontab for ltp_add_key05
userdel: ltp_add_key05 home directory (/home/ltp_add_key05) not found

Tested on tmpfs and ext4 on openSUSE (5.5.7-1-default):

Mar 17 12:54:52 dell5510 useradd[28122]: new user: name=ltp_add_key05, UID=1001, GID=100, home=/home/ltp_add_key05, shell=/bin/bash, from=/dev/pts/3
Mar 17 12:54:52 dell5510 nscd[1781]: 1781 monitored file `/etc/passwd` was written to
Mar 17 12:54:52 dell5510 nscd[1781]: 1781 monitored file `/etc/passwd` was moved into place, adding watch
Mar 17 12:54:52 dell5510 nscd[1781]: 1781 monitored file `/etc/group` was written to
Mar 17 12:54:52 dell5510 nscd[1781]: 1781 monitoring file `/etc/passwd` (35)
Mar 17 12:54:52 dell5510 nscd[1781]: 1781 monitoring directory `/etc` (2)
Mar 17 12:54:52 dell5510 nscd[1781]: 1781 monitoring file `/etc/group` (3)
Mar 17 12:54:52 dell5510 nscd[1781]: 1781 monitoring directory `/etc` (2)
Mar 17 12:54:52 dell5510 nscd[1781]: 1781 monitoring file `/etc/passwd` (35)
Mar 17 12:54:52 dell5510 nscd[1781]: 1781 monitoring directory `/etc` (2)
Mar 17 12:54:52 dell5510 nscd[1781]: 1781 monitoring file `/etc/group` (3)
Mar 17 12:54:52 dell5510 nscd[1781]: 1781 monitoring directory `/etc` (2)
Mar 17 12:54:52 dell5510 crontab[28135]: (root) DELETE (ltp_add_key05)
Mar 17 12:54:52 dell5510 userdel[28133]: delete user 'ltp_add_key05'
Mar 17 12:54:52 dell5510 nscd[1781]: 1781 monitored file `/etc/passwd` was written to
Mar 17 12:54:52 dell5510 nscd[1781]: 1781 monitored file `/etc/passwd` was moved into place, adding watch
Mar 17 12:54:52 dell5510 nscd[1781]: 1781 monitored file `/etc/group` was written to
Mar 17 12:54:52 dell5510 nscd[1781]: 1781 monitoring file `/etc/passwd` (36)
Mar 17 12:54:52 dell5510 nscd[1781]: 1781 monitoring directory `/etc` (2)
Mar 17 12:54:52 dell5510 nscd[1781]: 1781 monitoring file `/etc/group` (3)
Mar 17 12:54:52 dell5510 nscd[1781]: 1781 monitoring directory `/etc` (2)
Mar 17 12:54:52 dell5510 nscd[1781]: 1781 monitoring file `/etc/passwd` (36)
Mar 17 12:54:52 dell5510 nscd[1781]: 1781 monitoring directory `/etc` (2)
Mar 17 12:54:52 dell5510 nscd[1781]: 1781 monitoring file `/etc/group` (3)
Mar 17 12:54:52 dell5510 nscd[1781]: 1781 monitoring directory `/etc` (2)
Mar 17 12:54:52 dell5510 useradd[28145]: new user: name=ltp_add_key05, UID=1001, GID=100, home=/home/ltp_add_key05, shell=/bin/bash, from=/dev/pts/3
Mar 17 12:54:52 dell5510 nscd[1781]: 1781 monitored file `/etc/passwd` was written to
Mar 17 12:54:52 dell5510 nscd[1781]: 1781 monitored file `/etc/passwd` was moved into place, adding watch
Mar 17 12:54:52 dell5510 nscd[1781]: 1781 monitored file `/etc/group` was written to
Mar 17 12:54:52 dell5510 nscd[1781]: 1781 monitoring file `/etc/passwd` (37)
Mar 17 12:54:52 dell5510 nscd[1781]: 1781 monitoring directory `/etc` (2)
Mar 17 12:54:52 dell5510 nscd[1781]: 1781 monitoring file `/etc/group` (3)
Mar 17 12:54:52 dell5510 nscd[1781]: 1781 monitoring directory `/etc` (2)
Mar 17 12:54:53 dell5510 nscd[1781]: 1781 monitoring file `/etc/passwd` (37)
Mar 17 12:54:53 dell5510 nscd[1781]: 1781 monitoring directory `/etc` (2)
Mar 17 12:54:53 dell5510 nscd[1781]: 1781 monitoring file `/etc/group` (3)
Mar 17 12:54:53 dell5510 nscd[1781]: 1781 monitoring directory `/etc` (2)
Mar 17 12:54:53 dell5510 crontab[28158]: (root) DELETE (ltp_add_key05)
Mar 17 12:54:53 dell5510 userdel[28156]: delete user 'ltp_add_key05'
Mar 17 12:54:53 dell5510 nscd[1781]: 1781 monitored file `/etc/passwd` was written to
Mar 17 12:54:53 dell5510 nscd[1781]: 1781 monitored file `/etc/passwd` was moved into place, adding watch
Mar 17 12:54:53 dell5510 nscd[1781]: 1781 monitored file `/etc/group` was written to
Mar 17 12:54:53 dell5510 nscd[1781]: 1781 monitoring file `/etc/passwd` (38)
Mar 17 12:54:53 dell5510 nscd[1781]: 1781 monitoring directory `/etc` (2)
Mar 17 12:54:53 dell5510 nscd[1781]: 1781 monitoring file `/etc/group` (3)
Mar 17 12:54:53 dell5510 nscd[1781]: 1781 monitoring directory `/etc` (2)
Mar 17 12:54:53 dell5510 nscd[1781]: 1781 monitoring file `/etc/passwd` (38)
Mar 17 12:54:53 dell5510 nscd[1781]: 1781 monitoring directory `/etc` (2)
Mar 17 12:54:53 dell5510 nscd[1781]: 1781 monitoring file `/etc/group` (3)
Mar 17 12:54:53 dell5510 nscd[1781]: 1781 monitoring directory `/etc` (2)

Also failed on Debian (5.3.0-rc3+)
add_key05.c:82: INFO: test max bytes under unprivileged user
add_key05.c:107: PASS: add_key(test_inv) failed as expected: EDQUOT (122)
add_key05.c:126: FAIL: add_key(test_max) failed unexpectedly: EDQUOT (122)
userdel: ltp_add_key05 mail spool (/var/mail/ltp_add_key05) not found
userdel: ltp_add_key05 home directory (/home/ltp_add_key05) not found

Mar 17 11:58:32 debian-testing useradd[969]: new group: name=ltp_add_key05, GID=1002
Mar 17 11:58:32 debian-testing useradd[969]: new user: name=ltp_add_key05, UID=1002, GID=1002, home=/home/ltp_add_key05, shell=
Mar 17 11:58:32 debian-testing userdel[975]: delete user 'ltp_add_key05'
Mar 17 11:58:32 debian-testing userdel[975]: removed group 'ltp_add_key05' owned by 'ltp_add_key05'
Mar 17 11:58:32 debian-testing userdel[975]: removed shadow group 'ltp_add_key05' owned by 'ltp_add_key05'
Mar 17 11:58:32 debian-testing useradd[980]: new group: name=ltp_add_key05, GID=1002
Mar 17 11:58:32 debian-testing useradd[980]: new user: name=ltp_add_key05, UID=1002, GID=1002, home=/home/ltp_add_key05, shell=
Mar 17 11:58:32 debian-testing userdel[986]: delete user 'ltp_add_key05'
Mar 17 11:58:32 debian-testing userdel[986]: removed group 'ltp_add_key05' owned by 'ltp_add_key05'
Mar 17 11:58:32 debian-testing userdel[986]: removed shadow group 'ltp_add_key05' owned by 'ltp_add_key05'

The rest looks ok to me.

Kind regards,
Petr

[LTP] [PATCH v2] syscalls/add_key05: add maxbytes/maxkeys test under unprivileged user

[Yang Xu]

Hi Petr


> Hi Xu,
> 
> thanks for your patch.
> 
>> This case is designed to test whether we can reach maxbytes/maxkeys
>> quota exactly under unprivileged users. It is a regression test for
>> commit a08bf91ce28e ("KEYS: allow reaching the keys quotas exactly").
>> But this kenrel commit is a incomplete fix, it still fails on 5.6.3-rc1.
>> Fix patch[1] is still under review.
> 
>> [1]https://patchwork.kernel.org/patch/11411507/
> It's already in next tree, congratulations :).
> https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git/commit/?h=next-20200317&id=2e356101e72ab1361821b3af024d64877d9a798d
Thanks.
> 
>> Signed-off-by: Yang Xu <xuyang2018.jy@cn.fujitsu.com>
>> ---
>> v1->v2:
>> 1. using parse_proc_key_user
>> 2. add some messages about delta(v1 uses data_len)
>> 3. fix something pointed by Cyril
> ...
> 
>> +++ b/testcases/kernel/syscalls/add_key/add_key05.c
>> @@ -0,0 +1,211 @@
>> +// SPDX-License-Identifier: GPL-2.0-or-later
>> +/*
>> + * Copyright (c) 2020 FUJITSU LIMITED. All rights reserved.
>> + * Author: Yang Xu <xuyang2018.jy@cn.fujitsu.com>
>> + *
>> + *Description:
> Few formatting like comments (sorry for pointing out unimportant nits, but with
> little care it can be avoided, of course this can be fixed by person who merges):
> nit: missing space after *. I also tend to avoid text like "Description"
> (it's obvious it's a description).

Right, I will remove "Description:".
>> + * Test unprivileged user can support the number of keys and the
>> + * number of bytes consumed in payloads of the keys.The defalut value
> nit: missing space after dot.
> 
>> + * is 200 and 20000.
>> + * This is also a regresstion test for
>> + * commit a08bf91ce28e ("KEYS: allow reaching the keys quotas exactly")
> It's not bad to mention this commit, but maybe not needed since there is
> linux-git tag.
I prefer to keep it, it is more clear for user.
>> + *
> Why extra line with * ?
I will remove it.
>> + */
>> +#include <stdio.h>
>> +#include <stdlib.h>
>> +#include <pwd.h>
>> +#include "tst_test.h"
>> +#include "lapi/keyctl.h"
>> +
>> +static char *user_buf;
>> +static const char *username = "ltp_add_key05";
>> +static int user_added;
>> +struct passwd *ltpuser;
>> +static char fmt[1024];
>> +
>> +static void add_user(void)
>> +{
>> +	if (user_added)
>> +		return;
>> +
>> +	const char *const cmd_useradd[] = {"useradd", username, NULL};
>> +	int rc;
>> +
>> +	switch ((rc = tst_run_cmd(cmd_useradd, NULL, NULL, 1))) {
>> +	case 0:
>> +		user_added = 1;
>> +		ltpuser = SAFE_GETPWNAM(username);
>> +		break;
>> +	case 1:
>> +	case 255:
>> +		break;
>> +	default:
>> +		tst_brk(TBROK, "Useradd failed (%d)", rc);
>> +	}
>> +	sprintf(fmt, "%5u: %%*5d %%*d/%%*d %%d/%%d %%d/%%d", ltpuser->pw_uid);
>> +}
> I've been planning to add functionality like this to the library for some time.
> (there is identical code in setpriority01.c and other tests would benefit from that).
> But no need to stop this patch just for this, I've created issue for it:
> https://github.com/linux-test-project/ltp/issues/468
> 
That't great.
>> +
>> +static void clean_user(void)
>> +{
>> +	if (!user_added)
>> +		return;
>> +
>> +	const char *const cmd_userdel[] = {"userdel", "-r", username, NULL};
>> +
>> +	if (tst_run_cmd(cmd_userdel, NULL, NULL, 1))
>> +		tst_res(TWARN | TERRNO, "'userdel -r %s' failed", username);
>> +	else
>> +		user_added = 0;
>> +}
>> +
>> +static inline void parse_proc_key_users(int *used_key, int *max_key, int *used_bytes, int *max_bytes)
>> +{
>> +	unsigned int val[4];
>> +	SAFE_FILE_LINES_SCANF("/proc/key-users", fmt, &val[0], &val[1], &val[2], &val[3]);
>> +
>> +	if (used_key)
>> +		*used_key = val[0];
>> +	if (max_key)
>> +		*max_key = val[1];
>> +	if (used_bytes)
>> +		*used_bytes = val[2];
>> +	if (max_bytes)
>> +		*max_bytes = val[3];
>> +}
>> +
>> +static void verify_max_btyes(void)
>> +{
>> +	char *buf;
>> +	int plen, invalid_plen, delta;
>> +	int used_bytes, max_bytes, tmp_used_bytes;
>> +
>> +	tst_res(TINFO, "test max bytes under unprivileged user");
>> +
>> +	parse_proc_key_users(NULL, NULL, &tmp_used_bytes, NULL);
>> +	TEST(add_key("user", "test2", user_buf, 64, KEY_SPEC_THREAD_KEYRING));
>> +	if (TST_RET == -1) {
>> +		tst_res(TFAIL | TTERRNO, "add key test2 failed");
>> +		return;
>> +	}
>> +	parse_proc_key_users(NULL, NULL, &used_bytes, &max_bytes);
>> +
>> +	/*
>> +	 * Compute delta between default datalen(in key_alloc) and actual
>> +	 * datlen(in key_payload_reserve).
>> +	 * more info see kernel code: security/keys/key.c
>> +	 */
>> +	delta = used_bytes - tmp_used_bytes - strlen("test2") - 1 - 64;
>> +	invalid_plen = max_bytes - used_bytes - delta - strlen("test_xxx");
>> +	buf = tst_alloc(invalid_plen);
>> +
>> +	TEST(add_key("user", "test_inv", buf, invalid_plen, KEY_SPEC_THREAD_KEYRING));
>> +	if (TST_RET != -1) {
>> +		tst_res(TFAIL, "add_key(test_inv) succeeded unexpectedltly");
>> +		return;
>> +	}
>> +	if (TST_ERR == EDQUOT)
>> +		tst_res(TPASS | TTERRNO, "add_key(test_inv) failed as expected");
>> +	else
>> +		tst_res(TFAIL | TTERRNO, "add_key(test_inv) failed expected EDQUOT got");
>> +
>> +	/*Reset delta*/
>> +	TEST(add_key("user", "test3", user_buf, 64, KEY_SPEC_THREAD_KEYRING));
>> +	if (TST_RET == -1) {
>> +		tst_res(TFAIL | TTERRNO, "add key test3 failed");
>> +		return;
>> +	}
>> +	TEST(add_key("user", "test4", user_buf, 64, KEY_SPEC_THREAD_KEYRING));
>> +	if (TST_RET == -1) {
>> +		tst_res(TFAIL | TTERRNO, "add key test4 failed");
>> +		return;
>> +	}
>> +	parse_proc_key_users(NULL, NULL, &used_bytes, &max_bytes);
>> +	plen = max_bytes - used_bytes - delta - strlen("test_xxx") - 1;
>> +	TEST(add_key("user", "test_max", buf, plen, KEY_SPEC_THREAD_KEYRING));
>> +	if (TST_RET == -1) {
>> +		 tst_res(TFAIL | TTERRNO, "add_key(test_max) failed unexpectedly");
>> +		 return;
> Hm, I get this failure, I guess both are because missing your fix
> 2e356101e72ab1361821b3af024d64877d9a798d, right?
Yes, if your kernel is only with commit ("KEYS: allow reaching the keys 
quotas exact"), add_key(test_max) will fail because we don't remove this 
limit when initializing the key.
> 
> add_key05.c:106: PASS: add_key(test_inv) failed as expected: EDQUOT (122)
> add_key05.c:125: FAIL: add_key(test_max) failed unexpectedly: EDQUOT (122)
> no crontab for ltp_add_key05
> userdel: ltp_add_key05 home directory (/home/ltp_add_key05) not found
> 
> Tested on tmpfs and ext4 on openSUSE (5.5.7-1-default):
> 
> Mar 17 12:54:52 dell5510 useradd[28122]: new user: name=ltp_add_key05, UID=1001, GID=100, home=/home/ltp_add_key05, shell=/bin/bash, from=/dev/pts/3
> Mar 17 12:54:52 dell5510 nscd[1781]: 1781 monitored file `/etc/passwd` was written to
> Mar 17 12:54:52 dell5510 nscd[1781]: 1781 monitored file `/etc/passwd` was moved into place, adding watch
> Mar 17 12:54:52 dell5510 nscd[1781]: 1781 monitored file `/etc/group` was written to
> Mar 17 12:54:52 dell5510 nscd[1781]: 1781 monitoring file `/etc/passwd` (35)
> Mar 17 12:54:52 dell5510 nscd[1781]: 1781 monitoring directory `/etc` (2)
> Mar 17 12:54:52 dell5510 nscd[1781]: 1781 monitoring file `/etc/group` (3)
> Mar 17 12:54:52 dell5510 nscd[1781]: 1781 monitoring directory `/etc` (2)
> Mar 17 12:54:52 dell5510 nscd[1781]: 1781 monitoring file `/etc/passwd` (35)
> Mar 17 12:54:52 dell5510 nscd[1781]: 1781 monitoring directory `/etc` (2)
> Mar 17 12:54:52 dell5510 nscd[1781]: 1781 monitoring file `/etc/group` (3)
> Mar 17 12:54:52 dell5510 nscd[1781]: 1781 monitoring directory `/etc` (2)
> Mar 17 12:54:52 dell5510 crontab[28135]: (root) DELETE (ltp_add_key05)
> Mar 17 12:54:52 dell5510 userdel[28133]: delete user 'ltp_add_key05'
> Mar 17 12:54:52 dell5510 nscd[1781]: 1781 monitored file `/etc/passwd` was written to
> Mar 17 12:54:52 dell5510 nscd[1781]: 1781 monitored file `/etc/passwd` was moved into place, adding watch
> Mar 17 12:54:52 dell5510 nscd[1781]: 1781 monitored file `/etc/group` was written to
> Mar 17 12:54:52 dell5510 nscd[1781]: 1781 monitoring file `/etc/passwd` (36)
> Mar 17 12:54:52 dell5510 nscd[1781]: 1781 monitoring directory `/etc` (2)
> Mar 17 12:54:52 dell5510 nscd[1781]: 1781 monitoring file `/etc/group` (3)
> Mar 17 12:54:52 dell5510 nscd[1781]: 1781 monitoring directory `/etc` (2)
> Mar 17 12:54:52 dell5510 nscd[1781]: 1781 monitoring file `/etc/passwd` (36)
> Mar 17 12:54:52 dell5510 nscd[1781]: 1781 monitoring directory `/etc` (2)
> Mar 17 12:54:52 dell5510 nscd[1781]: 1781 monitoring file `/etc/group` (3)
> Mar 17 12:54:52 dell5510 nscd[1781]: 1781 monitoring directory `/etc` (2)
> Mar 17 12:54:52 dell5510 useradd[28145]: new user: name=ltp_add_key05, UID=1001, GID=100, home=/home/ltp_add_key05, shell=/bin/bash, from=/dev/pts/3
> Mar 17 12:54:52 dell5510 nscd[1781]: 1781 monitored file `/etc/passwd` was written to
> Mar 17 12:54:52 dell5510 nscd[1781]: 1781 monitored file `/etc/passwd` was moved into place, adding watch
> Mar 17 12:54:52 dell5510 nscd[1781]: 1781 monitored file `/etc/group` was written to
> Mar 17 12:54:52 dell5510 nscd[1781]: 1781 monitoring file `/etc/passwd` (37)
> Mar 17 12:54:52 dell5510 nscd[1781]: 1781 monitoring directory `/etc` (2)
> Mar 17 12:54:52 dell5510 nscd[1781]: 1781 monitoring file `/etc/group` (3)
> Mar 17 12:54:52 dell5510 nscd[1781]: 1781 monitoring directory `/etc` (2)
> Mar 17 12:54:53 dell5510 nscd[1781]: 1781 monitoring file `/etc/passwd` (37)
> Mar 17 12:54:53 dell5510 nscd[1781]: 1781 monitoring directory `/etc` (2)
> Mar 17 12:54:53 dell5510 nscd[1781]: 1781 monitoring file `/etc/group` (3)
> Mar 17 12:54:53 dell5510 nscd[1781]: 1781 monitoring directory `/etc` (2)
> Mar 17 12:54:53 dell5510 crontab[28158]: (root) DELETE (ltp_add_key05)
> Mar 17 12:54:53 dell5510 userdel[28156]: delete user 'ltp_add_key05'
> Mar 17 12:54:53 dell5510 nscd[1781]: 1781 monitored file `/etc/passwd` was written to
> Mar 17 12:54:53 dell5510 nscd[1781]: 1781 monitored file `/etc/passwd` was moved into place, adding watch
> Mar 17 12:54:53 dell5510 nscd[1781]: 1781 monitored file `/etc/group` was written to
> Mar 17 12:54:53 dell5510 nscd[1781]: 1781 monitoring file `/etc/passwd` (38)
> Mar 17 12:54:53 dell5510 nscd[1781]: 1781 monitoring directory `/etc` (2)
> Mar 17 12:54:53 dell5510 nscd[1781]: 1781 monitoring file `/etc/group` (3)
> Mar 17 12:54:53 dell5510 nscd[1781]: 1781 monitoring directory `/etc` (2)
> Mar 17 12:54:53 dell5510 nscd[1781]: 1781 monitoring file `/etc/passwd` (38)
> Mar 17 12:54:53 dell5510 nscd[1781]: 1781 monitoring directory `/etc` (2)
> Mar 17 12:54:53 dell5510 nscd[1781]: 1781 monitoring file `/etc/group` (3)
> Mar 17 12:54:53 dell5510 nscd[1781]: 1781 monitoring directory `/etc` (2)
> 
> Also failed on Debian (5.3.0-rc3+)
> add_key05.c:82: INFO: test max bytes under unprivileged user
> add_key05.c:107: PASS: add_key(test_inv) failed as expected: EDQUOT (122)
> add_key05.c:126: FAIL: add_key(test_max) failed unexpectedly: EDQUOT (122)
> userdel: ltp_add_key05 mail spool (/var/mail/ltp_add_key05) not found
> userdel: ltp_add_key05 home directory (/home/ltp_add_key05) not found
> 
> Mar 17 11:58:32 debian-testing useradd[969]: new group: name=ltp_add_key05, GID=1002
> Mar 17 11:58:32 debian-testing useradd[969]: new user: name=ltp_add_key05, UID=1002, GID=1002, home=/home/ltp_add_key05, shell=
> Mar 17 11:58:32 debian-testing userdel[975]: delete user 'ltp_add_key05'
> Mar 17 11:58:32 debian-testing userdel[975]: removed group 'ltp_add_key05' owned by 'ltp_add_key05'
> Mar 17 11:58:32 debian-testing userdel[975]: removed shadow group 'ltp_add_key05' owned by 'ltp_add_key05'
> Mar 17 11:58:32 debian-testing useradd[980]: new group: name=ltp_add_key05, GID=1002
> Mar 17 11:58:32 debian-testing useradd[980]: new user: name=ltp_add_key05, UID=1002, GID=1002, home=/home/ltp_add_key05, shell=
> Mar 17 11:58:32 debian-testing userdel[986]: delete user 'ltp_add_key05'
> Mar 17 11:58:32 debian-testing userdel[986]: removed group 'ltp_add_key05' owned by 'ltp_add_key05'
> Mar 17 11:58:32 debian-testing userdel[986]: removed shadow group 'ltp_add_key05' owned by 'ltp_add_key05'
> 
> The rest looks ok to me.
Thanks for your reivew. I will send v3 (also add save_store for keys/btyes )

Best Regards
Yang Xu
> 
> Kind regards,
> Petr
> 
> 

[LTP] [PATCH v3] syscalls/add_key05: add maxbytes/maxkeys test under unprivileged user

[Yang Xu]

This case is designed to test whether we can reach maxbytes/maxkeys
quota exactly under unprivileged users. It is a regression test for
commit a08bf91ce28e ("KEYS: allow reaching the keys quotas exactly").
But this kenrel commit is a incomplete fix, it still fails on lastest
upstream kernel. Fix patch[1] is on linux-next branch.

[1]https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git/commit/?id=2e356101e72a

Signed-off-by: Yang Xu <xuyang2018.jy@cn.fujitsu.com>
---
v2->v3:
1.add save restore for bytes/keys
2.add linux tag even it is only on next branch
3.fix description
4.remove useless plen in test_max_keys
 runtest/syscalls                              |   1 +
 testcases/kernel/syscalls/add_key/.gitignore  |   1 +
 testcases/kernel/syscalls/add_key/add_key05.c | 222 ++++++++++++++++++
 3 files changed, 224 insertions(+)
 create mode 100644 testcases/kernel/syscalls/add_key/add_key05.c

diff --git a/runtest/syscalls b/runtest/syscalls
index 224de8c9b..6ba4b36e2 100644
--- a/runtest/syscalls
+++ b/runtest/syscalls
@@ -18,6 +18,7 @@ add_key01 add_key01
 add_key02 add_key02
 add_key03 add_key03
 add_key04 add_key04
+add_key05 add_key05
 
 adjtimex01 adjtimex01
 adjtimex02 adjtimex02
diff --git a/testcases/kernel/syscalls/add_key/.gitignore b/testcases/kernel/syscalls/add_key/.gitignore
index b9a04214d..f57dc2228 100644
--- a/testcases/kernel/syscalls/add_key/.gitignore
+++ b/testcases/kernel/syscalls/add_key/.gitignore
@@ -2,3 +2,4 @@
 /add_key02
 /add_key03
 /add_key04
+/add_key05
diff --git a/testcases/kernel/syscalls/add_key/add_key05.c b/testcases/kernel/syscalls/add_key/add_key05.c
new file mode 100644
index 000000000..131195094
--- /dev/null
+++ b/testcases/kernel/syscalls/add_key/add_key05.c
@@ -0,0 +1,222 @@
+// SPDX-License-Identifier: GPL-2.0-or-later
+/*
+ * Copyright (c) 2020 FUJITSU LIMITED. All rights reserved.
+ * Author: Yang Xu <xuyang2018.jy@cn.fujitsu.com>
+ *
+ * Test unprivileged user can support the number of keys and the
+ * number of bytes consumed in payloads of the keys. The defalut
+ * value is 200 and 20000.
+ * This is also a regresstion test for
+ * commit a08bf91ce28e ("KEYS: allow reaching the keys quotas exactly")
+ * commit 2e356101e72a ("KEYS: reaching the keys quotas correctly")
+ */
+
+#include <stdio.h>
+#include <stdlib.h>
+#include <pwd.h>
+#include "tst_test.h"
+#include "lapi/keyctl.h"
+
+static char *user_buf;
+static const char *username = "ltp_add_key05";
+static int user_added;
+struct passwd *ltpuser;
+static char fmt[1024];
+
+static const char * const save_restore[] = {
+	"?/proc/sys/kernel/keys/maxkeys",
+	"?/proc/sys/kernel/keys/maxbytes",
+	NULL,
+};
+
+static void add_user(void)
+{
+	if (user_added)
+		return;
+
+	const char *const cmd_useradd[] = {"useradd", username, NULL};
+	int rc;
+
+	switch ((rc = tst_run_cmd(cmd_useradd, NULL, NULL, 1))) {
+	case 0:
+		user_added = 1;
+		ltpuser = SAFE_GETPWNAM(username);
+		break;
+	case 1:
+	case 255:
+		break;
+	default:
+		tst_brk(TBROK, "Useradd failed (%d)", rc);
+	}
+	sprintf(fmt, "%5u: %%*5d %%*d/%%*d %%d/%%d %%d/%%d", ltpuser->pw_uid);
+}
+
+static void clean_user(void)
+{
+	if (!user_added)
+		return;
+
+	const char *const cmd_userdel[] = {"userdel", "-r", username, NULL};
+
+	if (tst_run_cmd(cmd_userdel, NULL, NULL, 1))
+		tst_res(TWARN | TERRNO, "'userdel -r %s' failed", username);
+	else
+		user_added = 0;
+}
+
+static inline void parse_proc_key_users(int *used_key, int *max_key, int *used_bytes, int *max_bytes)
+{
+	unsigned int val[4];
+	SAFE_FILE_LINES_SCANF("/proc/key-users", fmt, &val[0], &val[1], &val[2], &val[3]);
+
+	if (used_key)
+		*used_key = val[0];
+	if (max_key)
+		*max_key = val[1];
+	if (used_bytes)
+		*used_bytes = val[2];
+	if (max_bytes)
+		*max_bytes = val[3];
+}
+
+static void verify_max_btyes(void)
+{
+	char *buf;
+	int plen, invalid_plen, delta;
+	int used_bytes, max_bytes, tmp_used_bytes;
+
+	tst_res(TINFO, "test max bytes under unprivileged user");
+
+	parse_proc_key_users(NULL, NULL, &tmp_used_bytes, NULL);
+	TEST(add_key("user", "test2", user_buf, 64, KEY_SPEC_THREAD_KEYRING));
+	if (TST_RET == -1) {
+		tst_res(TFAIL | TTERRNO, "add key test2 failed");
+		return;
+	}
+	parse_proc_key_users(NULL, NULL, &used_bytes, &max_bytes);
+
+	/*
+	 * Compute delta between default datalen(in key_alloc) and actual
+	 * datlen(in key_payload_reserve).
+	 * more info see kernel code: security/keys/key.c
+	 */
+	delta = used_bytes - tmp_used_bytes - strlen("test2") - 1 - 64;
+	invalid_plen = max_bytes - used_bytes - delta - strlen("test_xxx");
+	buf = tst_alloc(invalid_plen);
+
+	TEST(add_key("user", "test_inv", buf, invalid_plen, KEY_SPEC_THREAD_KEYRING));
+	if (TST_RET != -1) {
+		tst_res(TFAIL, "add_key(test_inv) succeeded unexpectedltly");
+		return;
+	}
+	if (TST_ERR == EDQUOT)
+		tst_res(TPASS | TTERRNO, "add_key(test_inv) failed as expected");
+	else
+		tst_res(TFAIL | TTERRNO, "add_key(test_inv) failed expected EDQUOT got");
+
+	/*Reset delta*/
+	TEST(add_key("user", "test3", user_buf, 64, KEY_SPEC_THREAD_KEYRING));
+	if (TST_RET == -1) {
+		tst_res(TFAIL | TTERRNO, "add key test3 failed");
+		return;
+	}
+	TEST(add_key("user", "test4", user_buf, 64, KEY_SPEC_THREAD_KEYRING));
+	if (TST_RET == -1) {
+		tst_res(TFAIL | TTERRNO, "add key test4 failed");
+		return;
+	}
+	parse_proc_key_users(NULL, NULL, &used_bytes, &max_bytes);
+	plen = max_bytes - used_bytes - delta - strlen("test_xxx") - 1;
+	TEST(add_key("user", "test_max", buf, plen, KEY_SPEC_THREAD_KEYRING));
+	if (TST_RET == -1) {
+		 tst_res(TFAIL | TTERRNO, "add_key(test_max) failed unexpectedly");
+		 return;
+	}
+
+	tst_res(TPASS, "add_key(test_max) succeeded as expected");
+	parse_proc_key_users(NULL, NULL, &tmp_used_bytes, &max_bytes);
+	if (tmp_used_bytes == max_bytes)
+		tst_res(TPASS, "allow reaching the max bytes exactly");
+	else
+		tst_res(TFAIL, "max used bytes %u, key allow max bytes %u", tmp_used_bytes, max_bytes);
+}
+
+static void verify_max_keys(void)
+{
+	int i, used_key, max_key;
+	char desc[10];
+
+	tst_res(TINFO, "test max keys under unprivileged user");
+	parse_proc_key_users(&used_key, &max_key, NULL, NULL);
+
+	for (i = used_key + 1; i <= max_key; i++) {
+		sprintf(desc, "abc%d", i);
+		TEST(add_key("user", desc, user_buf, 64, KEY_SPEC_THREAD_KEYRING));
+		if (TST_RET == -1) {
+			tst_res(TFAIL | TTERRNO, "add keyring key(%s) failed", desc);
+			goto count;
+		}
+	}
+
+	TEST(add_key("user", "test_invalid_key", user_buf, 64, KEY_SPEC_THREAD_KEYRING));
+	if (TST_RET != -1) {
+		tst_res(TFAIL, "add keyring key(test_invalid_key) succeeded unexpectedly");
+		goto count;
+	}
+	if (TST_ERR == EDQUOT)
+		tst_res(TPASS | TTERRNO, "add_key(test_invalid_key) failed as expected");
+	else
+		tst_res(TFAIL | TTERRNO, "add_key(test_invalid_key) failed expected EDQUOT got");
+
+count:
+	parse_proc_key_users(&used_key, &max_key, NULL, NULL);
+	if (used_key == max_key)
+		tst_res(TPASS, "allow reaching the max key(%u) exactly", max_key);
+	else
+		tst_res(TFAIL, "max used key %u, key allow max key %u", used_key, max_key);
+}
+
+static void do_test(unsigned int n)
+{
+	add_user();
+	if (!SAFE_FORK()) {
+		SAFE_SETUID(ltpuser->pw_uid);
+		TEST(add_key("user", "test1", user_buf, 64, KEY_SPEC_THREAD_KEYRING));
+		if (TST_RET == -1) {
+			tst_res(TFAIL | TTERRNO, "add key test1 failed");
+			return;
+		}
+		if (n)
+			verify_max_keys();
+		else
+			verify_max_btyes();
+		exit(0);
+	}
+	tst_reap_children();
+	clean_user();
+}
+
+static void setup(void)
+{
+	SAFE_FILE_PRINTF("/proc/sys/kernel/keys/maxkeys", "200");
+	SAFE_FILE_PRINTF("/proc/sys/kernel/keys/maxbytes", "20000");
+}
+
+static struct tst_test test = {
+	.test = do_test,
+	.tcnt = 2,
+	.needs_root = 1,
+	.forks_child = 1,
+	.setup = setup,
+	.cleanup = clean_user,
+	.save_restore = save_restore,
+	.bufs = (struct tst_buffers []) {
+		{&user_buf, .size = 64},
+		{}
+	},
+	.tags = (const struct tst_tag[]) {
+		{"linux-git", "a08bf91ce28"},
+		{"linux-git", "2e356101e72"},
+		{}
+	}
+};
-- 
2.18.1

[LTP] [PATCH v2] syscalls/add_key05: add maxbytes/maxkeys test under unprivileged user

[Petr Vorel]

Hi Xu,

> > Hm, I get this failure, I guess both are because missing your fix
> > 2e356101e72ab1361821b3af024d64877d9a798d, right?
> Yes, if your kernel is only with commit ("KEYS: allow reaching the keys
> quotas exact"), add_key(test_max) will fail because we don't remove this
> limit when initializing the key.

Great, thanks for a confirmation, that was my main concern.

> > add_key05.c:106: PASS: add_key(test_inv) failed as expected: EDQUOT (122)
> > add_key05.c:125: FAIL: add_key(test_max) failed unexpectedly: EDQUOT (122)
> > no crontab for ltp_add_key05
> > userdel: ltp_add_key05 home directory (/home/ltp_add_key05) not found

> > The rest looks ok to me.
> Thanks for your reivew. I will send v3 (also add save_store for keys/btyes )
OK, I'll wait for v3 (but I could also merge just this one and fix that tiny
formatting issues, up to you).

Kind regards,
Petr

[LTP] [PATCH v2] syscalls/add_key05: add maxbytes/maxkeys test under unprivileged user

[Petr Vorel]

Hi Xu,

> > > The rest looks ok to me.
> > Thanks for your reivew. I will send v3 (also add save_store for keys/btyes )
> OK, I'll wait for v3 (but I could also merge just this one and fix that tiny
> formatting issues, up to you).

OK, v3 already sent. I'll have look today. Thanks!

Kind regards,
Petr

[LTP] [PATCH v3] syscalls/add_key05: add maxbytes/maxkeys test under unprivileged user

[Petr Vorel]

Hi Xu,

> This case is designed to test whether we can reach maxbytes/maxkeys
> quota exactly under unprivileged users. It is a regression test for
> commit a08bf91ce28e ("KEYS: allow reaching the keys quotas exactly").
> But this kenrel commit is a incomplete fix, it still fails on lastest
nit: typos: kenrel, lastest (will be fixed during merge).
> upstream kernel. Fix patch[1] is on linux-next branch.

> [1]https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git/commit/?id=2e356101e72a
I guess this commit get's preserved when merged in mainline, so we don't have to
wait for it.

Reviewed-by: Petr Vorel <pvorel@suse.cz>
LGTM except one concern about ltpuser setup.

> +++ b/testcases/kernel/syscalls/add_key/add_key05.c
...
> + * Test unprivileged user can support the number of keys and the
> + * number of bytes consumed in payloads of the keys. The defalut
> + * value is 200 and 20000.
> + * This is also a regresstion test for
nit: typos: defalut, regresstion (will be fixed during merge).

> + * commit a08bf91ce28e ("KEYS: allow reaching the keys quotas exactly")
> + * commit 2e356101e72a ("KEYS: reaching the keys quotas correctly")
> + */

> +struct passwd *ltpuser;
...
> +static void add_user(void)
> +{
> +	if (user_added)
> +		return;
> +
> +	const char *const cmd_useradd[] = {"useradd", username, NULL};
> +	int rc;
> +
> +	switch ((rc = tst_run_cmd(cmd_useradd, NULL, NULL, 1))) {
> +	case 0:
> +		user_added = 1;
> +		ltpuser = SAFE_GETPWNAM(username);
> +		break;
> +	case 1:
> +	case 255:
> +		break;
> +	default:
> +		tst_brk(TBROK, "Useradd failed (%d)", rc);
> +	}
> +	sprintf(fmt, "%5u: %%*5d %%*d/%%*d %%d/%%d %%d/%%d", ltpuser->pw_uid);
> +}
ltpuser is undefined for 1 and 255, therefore it gets SIGSEGV. It should either
require tst_run_cmd to exit 0 or use 0 as UID (root UID).

Kind regards,
Petr

[LTP] [PATCH v3] syscalls/add_key05: add maxbytes/maxkeys test under unprivileged user

[Yang Xu]

Hi Petr

> Hi Xu,
> 
>> This case is designed to test whether we can reach maxbytes/maxkeys
>> quota exactly under unprivileged users. It is a regression test for
>> commit a08bf91ce28e ("KEYS: allow reaching the keys quotas exactly").
>> But this kenrel commit is a incomplete fix, it still fails on lastest
> nit: typos: kenrel, lastest (will be fixed during merge).
>> upstream kernel. Fix patch[1] is on linux-next branch.
> 
>> [1]https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git/commit/?id=2e356101e72a
> I guess this commit get's preserved when merged in mainline, so we don't have to
> wait for it.
This morning, I got email about this patch[1](change subject"keys: Fix 
the keys quotas limit check"). I'm not sure whether they will recycle 
and modify this.

[1]https://patchwork.kernel.org/patch/11411507/
> 
> Reviewed-by: Petr Vorel <pvorel@suse.cz>
> LGTM except one concern about ltpuser setup.
> 
>> +++ b/testcases/kernel/syscalls/add_key/add_key05.c
> ...
>> + * Test unprivileged user can support the number of keys and the
>> + * number of bytes consumed in payloads of the keys. The defalut
>> + * value is 200 and 20000.
>> + * This is also a regresstion test for
> nit: typos: defalut, regresstion (will be fixed during merge).
Sorry for these typos.
> 
>> + * commit a08bf91ce28e ("KEYS: allow reaching the keys quotas exactly")
>> + * commit 2e356101e72a ("KEYS: reaching the keys quotas correctly")
>> + */
> 
>> +struct passwd *ltpuser;
> ...
>> +static void add_user(void)
>> +{
>> +	if (user_added)
>> +		return;
>> +
>> +	const char *const cmd_useradd[] = {"useradd", username, NULL};
>> +	int rc;
>> +
>> +	switch ((rc = tst_run_cmd(cmd_useradd, NULL, NULL, 1))) {
>> +	case 0:
>> +		user_added = 1;
>> +		ltpuser = SAFE_GETPWNAM(username);
>> +		break;
>> +	case 1:
>> +	case 255:
>> +		break;
>> +	default:
>> +		tst_brk(TBROK, "Useradd failed (%d)", rc);
>> +	}
>> +	sprintf(fmt, "%5u: %%*5d %%*d/%%*d %%d/%%d %%d/%%d", ltpuser->pw_uid);
>> +}
> ltpuser is undefined for 1 and 255, therefore it gets SIGSEGV. It should either
> require tst_run_cmd to exit 0 or use 0 as UID (root UID).
Oh, sorry for this obvious mistake, thanks for pointing out it.
Yes, I prefer to exit when running into 0 or 255.

Best Regards
Yang Xu
> 
> Kind regards,
> Petr
> 
> 

[LTP] [PATCH v3] syscalls/add_key05: add maxbytes/maxkeys test under unprivileged user

[Petr Vorel]

Hi Xu,

> > > This case is designed to test whether we can reach maxbytes/maxkeys
> > > quota exactly under unprivileged users. It is a regression test for
> > > commit a08bf91ce28e ("KEYS: allow reaching the keys quotas exactly").
> > > But this kenrel commit is a incomplete fix, it still fails on lastest
> > nit: typos: kenrel, lastest (will be fixed during merge).
> > > upstream kernel. Fix patch[1] is on linux-next branch.

> > > [1]https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git/commit/?id=2e356101e72a
> > I guess this commit get's preserved when merged in mainline, so we don't have to
> > wait for it.
> This morning, I got email about this patch[1](change subject"keys: Fix the
> keys quotas limit check"). I'm not sure whether they will recycle and modify
> this.

> [1]https://patchwork.kernel.org/patch/11411507/

Although the maintainers were discussing commit message change, in the end
Jarkko has sent it:
https://lkml.org/lkml/2020/3/15/314
Well, if the commit hash changes, we'll just fix it.

> > > +struct passwd *ltpuser;
> > ...
> > > +static void add_user(void)
> > > +{
> > > +	if (user_added)
> > > +		return;
> > > +
> > > +	const char *const cmd_useradd[] = {"useradd", username, NULL};
> > > +	int rc;
> > > +
> > > +	switch ((rc = tst_run_cmd(cmd_useradd, NULL, NULL, 1))) {
> > > +	case 0:
> > > +		user_added = 1;
> > > +		ltpuser = SAFE_GETPWNAM(username);
> > > +		break;
> > > +	case 1:
> > > +	case 255:
> > > +		break;
> > > +	default:
> > > +		tst_brk(TBROK, "Useradd failed (%d)", rc);
> > > +	}
> > > +	sprintf(fmt, "%5u: %%*5d %%*d/%%*d %%d/%%d %%d/%%d", ltpuser->pw_uid);
> > > +}
> > ltpuser is undefined for 1 and 255, therefore it gets SIGSEGV. It should either
> > require tst_run_cmd to exit 0 or use 0 as UID (root UID).
> Oh, sorry for this obvious mistake, thanks for pointing out it.
> Yes, I prefer to exit when running into 0 or 255.

Fine :). I'm going to merge with handling specially only 255:

Kind regards,
Petr

index a0c8c00cc..a39bfa0b7 100644
--- testcases/kernel/syscalls/add_key/add_key05.c
+++ testcases/kernel/syscalls/add_key/add_key05.c
@@ -43,11 +43,11 @@ static void add_user(void)
                user_added = 1;
                ltpuser = SAFE_GETPWNAM(username);
                break;
-       case 1:
        case 255:
+               tst_brk(TCONF, "useradd not found");
                break;
        default:
-               tst_brk(TBROK, "Useradd failed (%d)", rc);
+               tst_brk(TBROK, "useradd failed (%d)", rc);
        }
        sprintf(fmt, "%5u: %%*5d %%*d/%%*d %%d/%%d %%d/%%d", ltpuser->pw_uid);
 }

[LTP] [PATCH v3] syscalls/add_key05: add maxbytes/maxkeys test under unprivileged user

[Petr Vorel]

Hi Xu,

merged. Thanks for your patch!

Kind regards,
Petr