* [LTP] [PATCH] syscalls/keyctl06: update to test for follow-on fix
@ 2017-11-02 17:15 Eric Biggers
2017-11-03 1:44 ` Xiao Yang
` (2 more replies)
0 siblings, 3 replies; 4+ messages in thread
From: Eric Biggers @ 2017-11-02 17:15 UTC (permalink / raw)
To: ltp
From: Eric Biggers <ebiggers@google.com>
The original fix for the buffer overflow bug changed keyring_read() to
start returning a short value if the specified buffer is too small, but
the documented behavior is actually to return the size needed. Update
the test to expect the documented behavior, as is provided by a
follow-on fix.
Signed-off-by: Eric Biggers <ebiggers@google.com>
---
testcases/kernel/syscalls/keyctl/keyctl06.c | 41 ++++++++++++++++++-----------
1 file changed, 25 insertions(+), 16 deletions(-)
diff --git a/testcases/kernel/syscalls/keyctl/keyctl06.c b/testcases/kernel/syscalls/keyctl/keyctl06.c
index 88734313d..c62010e0b 100644
--- a/testcases/kernel/syscalls/keyctl/keyctl06.c
+++ b/testcases/kernel/syscalls/keyctl/keyctl06.c
@@ -16,8 +16,16 @@
*/
/*
- * Regression test for commit e645016abc80 ("KEYS: fix writing past end of
- * user-supplied buffer in keyring_read()").
+ * Regression test for:
+ *
+ * commit e645016abc80 ("KEYS: fix writing past end of user-supplied buffer
+ * in keyring_read()").
+ *
+ * as well as its follow-on fix:
+ *
+ * commit 3239b6f29bdf ("KEYS: return full count in keyring_read() if
+ * buffer is too small")
+ *
*/
#include <errno.h>
@@ -25,20 +33,20 @@
#include "tst_test.h"
#include "lapi/keyctl.h"
-static key_serial_t add_test_key(const char *description)
+static void add_test_key(const char *description)
{
TEST(add_key("user", description, "payload", 7,
KEY_SPEC_PROCESS_KEYRING));
if (TEST_RETURN < 0)
tst_brk(TBROK | TTERRNO, "Failed to add test key");
- return TEST_RETURN;
}
static void do_test(void)
{
key_serial_t key_ids[2];
- key_serial_t key_id_1 = add_test_key("key1");
- key_serial_t key_id_2 = add_test_key("key2");
+
+ add_test_key("key1");
+ add_test_key("key2");
memset(key_ids, 0, sizeof(key_ids));
TEST(keyctl(KEYCTL_READ, KEY_SPEC_PROCESS_KEYRING,
@@ -46,21 +54,22 @@ static void do_test(void)
if (TEST_RETURN < 0)
tst_brk(TBROK | TTERRNO, "KEYCTL_READ failed");
+ /*
+ * Do not check key_ids[0], as the contents of the buffer are
+ * unspecified if it was too small. However, key_ids[1] must not have
+ * been written to, as it was outside the buffer.
+ */
+
if (key_ids[1] != 0)
tst_brk(TFAIL, "KEYCTL_READ overran the buffer");
- if (key_ids[0] == 0)
- tst_brk(TBROK, "KEYCTL_READ didn't read anything");
-
- if (key_ids[0] != key_id_1 && key_ids[0] != key_id_2)
- tst_brk(TBROK, "KEYCTL_READ didn't return correct key ID");
-
- if (TEST_RETURN != sizeof(key_serial_t)) {
- tst_brk(TBROK, "KEYCTL_READ returned %ld but expected %zu",
- TEST_RETURN, sizeof(key_serial_t));
+ if (TEST_RETURN != sizeof(key_ids)) {
+ tst_brk(TFAIL, "KEYCTL_READ returned %ld but expected %zu",
+ TEST_RETURN, sizeof(key_ids));
}
- tst_res(TPASS, "KEYCTL_READ didn't overrun the buffer");
+ tst_res(TPASS,
+ "KEYCTL_READ returned full count but didn't overrun the buffer");
}
static struct tst_test test = {
--
2.15.0.403.gc27cc4dac6-goog
^ permalink raw reply related [flat|nested] 4+ messages in thread* [LTP] [PATCH] syscalls/keyctl06: update to test for follow-on fix
2017-11-02 17:15 [LTP] [PATCH] syscalls/keyctl06: update to test for follow-on fix Eric Biggers
@ 2017-11-03 1:44 ` Xiao Yang
2017-11-06 6:26 ` Li Wang
2017-11-07 11:10 ` Cyril Hrubis
2 siblings, 0 replies; 4+ messages in thread
From: Xiao Yang @ 2017-11-03 1:44 UTC (permalink / raw)
To: ltp
Hi Eric,
It looks better to me. Thanks for updating this test. :-)
Thanks,
Xiao Yang
On 2017/11/03 1:15, Eric Biggers wrote:
> From: Eric Biggers <ebiggers@google.com>
>
> The original fix for the buffer overflow bug changed keyring_read() to
> start returning a short value if the specified buffer is too small, but
> the documented behavior is actually to return the size needed. Update
> the test to expect the documented behavior, as is provided by a
> follow-on fix.
>
> Signed-off-by: Eric Biggers <ebiggers@google.com>
> ---
> testcases/kernel/syscalls/keyctl/keyctl06.c | 41 ++++++++++++++++++-----------
> 1 file changed, 25 insertions(+), 16 deletions(-)
>
> diff --git a/testcases/kernel/syscalls/keyctl/keyctl06.c b/testcases/kernel/syscalls/keyctl/keyctl06.c
> index 88734313d..c62010e0b 100644
> --- a/testcases/kernel/syscalls/keyctl/keyctl06.c
> +++ b/testcases/kernel/syscalls/keyctl/keyctl06.c
> @@ -16,8 +16,16 @@
> */
>
> /*
> - * Regression test for commit e645016abc80 ("KEYS: fix writing past end of
> - * user-supplied buffer in keyring_read()").
> + * Regression test for:
> + *
> + * commit e645016abc80 ("KEYS: fix writing past end of user-supplied buffer
> + * in keyring_read()").
> + *
> + * as well as its follow-on fix:
> + *
> + * commit 3239b6f29bdf ("KEYS: return full count in keyring_read() if
> + * buffer is too small")
> + *
> */
>
> #include <errno.h>
> @@ -25,20 +33,20 @@
> #include "tst_test.h"
> #include "lapi/keyctl.h"
>
> -static key_serial_t add_test_key(const char *description)
> +static void add_test_key(const char *description)
> {
> TEST(add_key("user", description, "payload", 7,
> KEY_SPEC_PROCESS_KEYRING));
> if (TEST_RETURN < 0)
> tst_brk(TBROK | TTERRNO, "Failed to add test key");
> - return TEST_RETURN;
> }
>
> static void do_test(void)
> {
> key_serial_t key_ids[2];
> - key_serial_t key_id_1 = add_test_key("key1");
> - key_serial_t key_id_2 = add_test_key("key2");
> +
> + add_test_key("key1");
> + add_test_key("key2");
>
> memset(key_ids, 0, sizeof(key_ids));
> TEST(keyctl(KEYCTL_READ, KEY_SPEC_PROCESS_KEYRING,
> @@ -46,21 +54,22 @@ static void do_test(void)
> if (TEST_RETURN < 0)
> tst_brk(TBROK | TTERRNO, "KEYCTL_READ failed");
>
> + /*
> + * Do not check key_ids[0], as the contents of the buffer are
> + * unspecified if it was too small. However, key_ids[1] must not have
> + * been written to, as it was outside the buffer.
> + */
> +
> if (key_ids[1] != 0)
> tst_brk(TFAIL, "KEYCTL_READ overran the buffer");
>
> - if (key_ids[0] == 0)
> - tst_brk(TBROK, "KEYCTL_READ didn't read anything");
> -
> - if (key_ids[0] != key_id_1 && key_ids[0] != key_id_2)
> - tst_brk(TBROK, "KEYCTL_READ didn't return correct key ID");
> -
> - if (TEST_RETURN != sizeof(key_serial_t)) {
> - tst_brk(TBROK, "KEYCTL_READ returned %ld but expected %zu",
> - TEST_RETURN, sizeof(key_serial_t));
> + if (TEST_RETURN != sizeof(key_ids)) {
> + tst_brk(TFAIL, "KEYCTL_READ returned %ld but expected %zu",
> + TEST_RETURN, sizeof(key_ids));
> }
>
> - tst_res(TPASS, "KEYCTL_READ didn't overrun the buffer");
> + tst_res(TPASS,
> + "KEYCTL_READ returned full count but didn't overrun the buffer");
> }
>
> static struct tst_test test = {
^ permalink raw reply [flat|nested] 4+ messages in thread* [LTP] [PATCH] syscalls/keyctl06: update to test for follow-on fix
2017-11-02 17:15 [LTP] [PATCH] syscalls/keyctl06: update to test for follow-on fix Eric Biggers
2017-11-03 1:44 ` Xiao Yang
@ 2017-11-06 6:26 ` Li Wang
2017-11-07 11:10 ` Cyril Hrubis
2 siblings, 0 replies; 4+ messages in thread
From: Li Wang @ 2017-11-06 6:26 UTC (permalink / raw)
To: ltp
This patch works fine to me.
On Fri, Nov 3, 2017 at 1:15 AM, Eric Biggers <ebiggers3@gmail.com> wrote:
> From: Eric Biggers <ebiggers@google.com>
>
> The original fix for the buffer overflow bug changed keyring_read() to
> start returning a short value if the specified buffer is too small, but
> the documented behavior is actually to return the size needed. Update
> the test to expect the documented behavior, as is provided by a
> follow-on fix.
>
> Signed-off-by: Eric Biggers <ebiggers@google.com>
> ---
> testcases/kernel/syscalls/keyctl/keyctl06.c | 41 ++++++++++++++++++-----------
> 1 file changed, 25 insertions(+), 16 deletions(-)
>
> diff --git a/testcases/kernel/syscalls/keyctl/keyctl06.c b/testcases/kernel/syscalls/keyctl/keyctl06.c
> index 88734313d..c62010e0b 100644
> --- a/testcases/kernel/syscalls/keyctl/keyctl06.c
> +++ b/testcases/kernel/syscalls/keyctl/keyctl06.c
> @@ -16,8 +16,16 @@
> */
>
> /*
> - * Regression test for commit e645016abc80 ("KEYS: fix writing past end of
> - * user-supplied buffer in keyring_read()").
> + * Regression test for:
> + *
> + * commit e645016abc80 ("KEYS: fix writing past end of user-supplied buffer
> + * in keyring_read()").
> + *
> + * as well as its follow-on fix:
> + *
> + * commit 3239b6f29bdf ("KEYS: return full count in keyring_read() if
> + * buffer is too small")
> + *
> */
>
> #include <errno.h>
> @@ -25,20 +33,20 @@
> #include "tst_test.h"
> #include "lapi/keyctl.h"
>
> -static key_serial_t add_test_key(const char *description)
> +static void add_test_key(const char *description)
> {
> TEST(add_key("user", description, "payload", 7,
> KEY_SPEC_PROCESS_KEYRING));
> if (TEST_RETURN < 0)
> tst_brk(TBROK | TTERRNO, "Failed to add test key");
> - return TEST_RETURN;
> }
>
> static void do_test(void)
> {
> key_serial_t key_ids[2];
> - key_serial_t key_id_1 = add_test_key("key1");
> - key_serial_t key_id_2 = add_test_key("key2");
> +
> + add_test_key("key1");
> + add_test_key("key2");
>
> memset(key_ids, 0, sizeof(key_ids));
> TEST(keyctl(KEYCTL_READ, KEY_SPEC_PROCESS_KEYRING,
> @@ -46,21 +54,22 @@ static void do_test(void)
> if (TEST_RETURN < 0)
> tst_brk(TBROK | TTERRNO, "KEYCTL_READ failed");
>
> + /*
> + * Do not check key_ids[0], as the contents of the buffer are
> + * unspecified if it was too small. However, key_ids[1] must not have
> + * been written to, as it was outside the buffer.
> + */
> +
> if (key_ids[1] != 0)
> tst_brk(TFAIL, "KEYCTL_READ overran the buffer");
>
> - if (key_ids[0] == 0)
> - tst_brk(TBROK, "KEYCTL_READ didn't read anything");
> -
> - if (key_ids[0] != key_id_1 && key_ids[0] != key_id_2)
> - tst_brk(TBROK, "KEYCTL_READ didn't return correct key ID");
> -
> - if (TEST_RETURN != sizeof(key_serial_t)) {
> - tst_brk(TBROK, "KEYCTL_READ returned %ld but expected %zu",
> - TEST_RETURN, sizeof(key_serial_t));
> + if (TEST_RETURN != sizeof(key_ids)) {
> + tst_brk(TFAIL, "KEYCTL_READ returned %ld but expected %zu",
> + TEST_RETURN, sizeof(key_ids));
> }
>
> - tst_res(TPASS, "KEYCTL_READ didn't overrun the buffer");
> + tst_res(TPASS,
> + "KEYCTL_READ returned full count but didn't overrun the buffer");
> }
>
> static struct tst_test test = {
> --
> 2.15.0.403.gc27cc4dac6-goog
>
>
> --
> Mailing list info: https://lists.linux.it/listinfo/ltp
--
Li Wang
liwang@redhat.com
^ permalink raw reply [flat|nested] 4+ messages in thread* [LTP] [PATCH] syscalls/keyctl06: update to test for follow-on fix
2017-11-02 17:15 [LTP] [PATCH] syscalls/keyctl06: update to test for follow-on fix Eric Biggers
2017-11-03 1:44 ` Xiao Yang
2017-11-06 6:26 ` Li Wang
@ 2017-11-07 11:10 ` Cyril Hrubis
2 siblings, 0 replies; 4+ messages in thread
From: Cyril Hrubis @ 2017-11-07 11:10 UTC (permalink / raw)
To: ltp
Hi!
Pushed, thanks.
--
Cyril Hrubis
chrubis@suse.cz
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2017-11-07 11:10 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2017-11-02 17:15 [LTP] [PATCH] syscalls/keyctl06: update to test for follow-on fix Eric Biggers
2017-11-03 1:44 ` Xiao Yang
2017-11-06 6:26 ` Li Wang
2017-11-07 11:10 ` Cyril Hrubis
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox