* [LTP] [PATCH v2] crypto/af_alg02: fixed read() being stuck
@ 2019-05-07 11:38 Christian Amann
2019-05-07 11:41 ` Christian Amann
0 siblings, 1 reply; 3+ messages in thread
From: Christian Amann @ 2019-05-07 11:38 UTC (permalink / raw)
To: ltp
On kernels < 4.14 (missing commit 2d97591ef43d) reading from
the request socket does not return and the testcase does not
finish.
This fix moves the logic to a child thread in order for the
parent to handle the timeout and report a message to the user.
Signed-off-by: Christian Amann <camann@suse.com>
---
testcases/kernel/crypto/Makefile | 2 ++
testcases/kernel/crypto/af_alg02.c | 29 ++++++++++++++++++++++++++++-
2 files changed, 30 insertions(+), 1 deletion(-)
diff --git a/testcases/kernel/crypto/Makefile b/testcases/kernel/crypto/Makefile
index 76f9308c2..6547e1cb6 100644
--- a/testcases/kernel/crypto/Makefile
+++ b/testcases/kernel/crypto/Makefile
@@ -20,3 +20,5 @@ include $(top_srcdir)/include/mk/testcases.mk
CFLAGS += -D_GNU_SOURCE
include $(top_srcdir)/include/mk/generic_leaf_target.mk
+
+af_alg02: CFLAGS += -pthread
diff --git a/testcases/kernel/crypto/af_alg02.c b/testcases/kernel/crypto/af_alg02.c
index a9e820423..7f8c81b66 100644
--- a/testcases/kernel/crypto/af_alg02.c
+++ b/testcases/kernel/crypto/af_alg02.c
@@ -7,12 +7,20 @@
* Regression test for commit ecaaab564978 ("crypto: salsa20 - fix
* blkcipher_walk API usage"), or CVE-2017-17805. This test verifies that an
* empty message can be encrypted with Salsa20 without crashing the kernel.
+ *
+ * read() fix:
+ * Calls read() in child thread in order to manually kill it after a timeout.
+ * With kernels missing commit 2d97591ef43d ("crypto: af_alg - consolidation
+ * of duplicate code") read() does not return.
*/
#include "tst_test.h"
#include "tst_af_alg.h"
+#include "tst_safe_pthread.h"
+#include <pthread.h>
+#include <errno.h>
-static void run(void)
+void *verify_encrypt(void *arg)
{
char buf[16];
int reqfd = tst_alg_setup_reqfd("skcipher", "salsa20", NULL, 16);
@@ -22,8 +30,27 @@ static void run(void)
tst_res(TPASS, "Successfully \"encrypted\" an empty message");
else
tst_res(TBROK, "read() didn't return 0");
+ return arg;
+}
+
+static void run(void)
+{
+ pthread_t thr;
+
+ pthread_setcancelstate(PTHREAD_CANCEL_ENABLE, NULL);
+ SAFE_PTHREAD_CREATE(&thr, NULL, verify_encrypt, NULL);
+
+ while (pthread_kill(thr, 0) != ESRCH) {
+ if (tst_timeout_remaining() <= 10) {
+ pthread_cancel(thr);
+ tst_brk(TBROK,
+ "Timed out while reading from request socket.");
+ }
+ usleep(1000);
+ }
}
static struct tst_test test = {
.test_all = run,
+ .timeout = 20,
};
--
2.16.4
^ permalink raw reply related [flat|nested] 3+ messages in thread
* [LTP] [PATCH v2] crypto/af_alg02: fixed read() being stuck
2019-05-07 11:38 [LTP] [PATCH v2] crypto/af_alg02: fixed read() being stuck Christian Amann
@ 2019-05-07 11:41 ` Christian Amann
2019-05-07 12:57 ` Li Wang
0 siblings, 1 reply; 3+ messages in thread
From: Christian Amann @ 2019-05-07 11:41 UTC (permalink / raw)
To: ltp
@ Li Wang
Okay, I think I understand it now. It looks much cleaner when done like
you suggested. Thanks!
Regards,
Christian
On 07/05/2019 13:38, Christian Amann wrote:
> On kernels < 4.14 (missing commit 2d97591ef43d) reading from
> the request socket does not return and the testcase does not
> finish.
>
> This fix moves the logic to a child thread in order for the
> parent to handle the timeout and report a message to the user.
>
> Signed-off-by: Christian Amann <camann@suse.com>
> ---
> testcases/kernel/crypto/Makefile | 2 ++
> testcases/kernel/crypto/af_alg02.c | 29 ++++++++++++++++++++++++++++-
> 2 files changed, 30 insertions(+), 1 deletion(-)
>
> diff --git a/testcases/kernel/crypto/Makefile b/testcases/kernel/crypto/Makefile
> index 76f9308c2..6547e1cb6 100644
> --- a/testcases/kernel/crypto/Makefile
> +++ b/testcases/kernel/crypto/Makefile
> @@ -20,3 +20,5 @@ include $(top_srcdir)/include/mk/testcases.mk
> CFLAGS += -D_GNU_SOURCE
>
> include $(top_srcdir)/include/mk/generic_leaf_target.mk
> +
> +af_alg02: CFLAGS += -pthread
> diff --git a/testcases/kernel/crypto/af_alg02.c b/testcases/kernel/crypto/af_alg02.c
> index a9e820423..7f8c81b66 100644
> --- a/testcases/kernel/crypto/af_alg02.c
> +++ b/testcases/kernel/crypto/af_alg02.c
> @@ -7,12 +7,20 @@
> * Regression test for commit ecaaab564978 ("crypto: salsa20 - fix
> * blkcipher_walk API usage"), or CVE-2017-17805. This test verifies that an
> * empty message can be encrypted with Salsa20 without crashing the kernel.
> + *
> + * read() fix:
> + * Calls read() in child thread in order to manually kill it after a timeout.
> + * With kernels missing commit 2d97591ef43d ("crypto: af_alg - consolidation
> + * of duplicate code") read() does not return.
> */
>
> #include "tst_test.h"
> #include "tst_af_alg.h"
> +#include "tst_safe_pthread.h"
> +#include <pthread.h>
> +#include <errno.h>
>
> -static void run(void)
> +void *verify_encrypt(void *arg)
> {
> char buf[16];
> int reqfd = tst_alg_setup_reqfd("skcipher", "salsa20", NULL, 16);
> @@ -22,8 +30,27 @@ static void run(void)
> tst_res(TPASS, "Successfully \"encrypted\" an empty message");
> else
> tst_res(TBROK, "read() didn't return 0");
> + return arg;
> +}
> +
> +static void run(void)
> +{
> + pthread_t thr;
> +
> + pthread_setcancelstate(PTHREAD_CANCEL_ENABLE, NULL);
> + SAFE_PTHREAD_CREATE(&thr, NULL, verify_encrypt, NULL);
> +
> + while (pthread_kill(thr, 0) != ESRCH) {
> + if (tst_timeout_remaining() <= 10) {
> + pthread_cancel(thr);
> + tst_brk(TBROK,
> + "Timed out while reading from request socket.");
> + }
> + usleep(1000);
> + }
> }
>
> static struct tst_test test = {
> .test_all = run,
> + .timeout = 20,
> };
^ permalink raw reply [flat|nested] 3+ messages in thread
* [LTP] [PATCH v2] crypto/af_alg02: fixed read() being stuck
2019-05-07 11:41 ` Christian Amann
@ 2019-05-07 12:57 ` Li Wang
0 siblings, 0 replies; 3+ messages in thread
From: Li Wang @ 2019-05-07 12:57 UTC (permalink / raw)
To: ltp
Hi Christian,
On Tue, May 7, 2019 at 7:41 PM Christian Amann <camann@suse.com> wrote:
> @ Li Wang
>
> Okay, I think I understand it now. It looks much cleaner when done like
> you suggested. Thanks!
>
Thanks for your update, you could try add these additional info in patch
note via `git note add` next time.
# man git-notes
> Regards,
>
> Christian
>
>
> On 07/05/2019 13:38, Christian Amann wrote:
> > On kernels < 4.14 (missing commit 2d97591ef43d) reading from
> > the request socket does not return and the testcase does not
> > finish.
> >
> > This fix moves the logic to a child thread in order for the
> > parent to handle the timeout and report a message to the user.
> >
> > Signed-off-by: Christian Amann <camann@suse.com>
> > ---
> > testcases/kernel/crypto/Makefile | 2 ++
> > testcases/kernel/crypto/af_alg02.c | 29 ++++++++++++++++++++++++++++-
> > 2 files changed, 30 insertions(+), 1 deletion(-)
> >
> > diff --git a/testcases/kernel/crypto/Makefile
> b/testcases/kernel/crypto/Makefile
> > index 76f9308c2..6547e1cb6 100644
> > --- a/testcases/kernel/crypto/Makefile
> > +++ b/testcases/kernel/crypto/Makefile
> > @@ -20,3 +20,5 @@ include $(top_srcdir)/include/mk/testcases.mk
> > CFLAGS += -D_GNU_SOURCE
> >
> > include $(top_srcdir)/include/mk/generic_leaf_target.mk
> > +
> > +af_alg02: CFLAGS += -pthread
> > diff --git a/testcases/kernel/crypto/af_alg02.c
> b/testcases/kernel/crypto/af_alg02.c
> > index a9e820423..7f8c81b66 100644
> > --- a/testcases/kernel/crypto/af_alg02.c
> > +++ b/testcases/kernel/crypto/af_alg02.c
> > @@ -7,12 +7,20 @@
> > * Regression test for commit ecaaab564978 ("crypto: salsa20 - fix
> > * blkcipher_walk API usage"), or CVE-2017-17805. This test verifies
> that an
> > * empty message can be encrypted with Salsa20 without crashing the
> kernel.
> > + *
> > + * read() fix:
> > + * Calls read() in child thread in order to manually kill it after a
> timeout.
>
The comments should be updated too.
> > + * With kernels missing commit 2d97591ef43d ("crypto: af_alg -
> consolidation
> > + * of duplicate code") read() does not return.
> > */
> >
> > #include "tst_test.h"
> > #include "tst_af_alg.h"
> > +#include "tst_safe_pthread.h"
> > +#include <pthread.h>
> > +#include <errno.h>
> >
> > -static void run(void)
> > +void *verify_encrypt(void *arg)
>
We could set LTP_ATTRIBUTE_UNUSED at the behind of unused parameter to get
rid of compiling warning:
void *verify_encrypt(void *arg LTP_ATTRIBUTE_UNUSED)
> > {
> > char buf[16];
>
TST_CHECKPOINT_WAKE(0);
> > int reqfd = tst_alg_setup_reqfd("skcipher", "salsa20", NULL, 16);
> > @@ -22,8 +30,27 @@ static void run(void)
> > tst_res(TPASS, "Successfully \"encrypted\" an empty
> message");
> > else
> > tst_res(TBROK, "read() didn't return 0");
>
Should use 'TFAIL' but 'TBROK' here.
See test-writing-guidelines.txt:
406
-------------------------------------------------------------------------------
407 void tst_res(int ttype, char *arg_fmt, ...);
408
-------------------------------------------------------------------------------
409
410 Printf-like function to report test result, it's mostly used with
ttype:
411
412 |==============================
413 | 'TPASS' | Test has passed.
414 | 'TFAIL' | Test has failed.
415 | 'TINFO' | General message.
416 |==============================
...
422
-------------------------------------------------------------------------------
423 void tst_brk(int ttype, char *arg_fmt, ...);
424
-------------------------------------------------------------------------------
425
426 Printf-like function to report error and exit the test, it can be
used with ttype:
427
428 |============================================================
429 | 'TBROK' | Something has failed in test preparation phase.
430 | 'TCONF' | Test is not appropriate for current configuration
431 (syscall not implemented, unsupported arch, ...)
432 |============================================================
> + return arg;
> > +}
> > +
> > +static void run(void)
> > +{
> > + pthread_t thr;
> > +
> > + pthread_setcancelstate(PTHREAD_CANCEL_ENABLE, NULL);
> > + SAFE_PTHREAD_CREATE(&thr, NULL, verify_encrypt, NULL);
> > +
>
A checkpoint is neccessary here, because we'd better confirm the thread has
already running before check it by pthread_kill().
TST_CHECKPOINT_WAIT(0);
> > + while (pthread_kill(thr, 0) != ESRCH) {
> > + if (tst_timeout_remaining() <= 10) {
> > + pthread_cancel(thr);
> > + tst_brk(TBROK,
> > + "Timed out while reading from request
> socket.");
> > + }
> > + usleep(1000);
> > + }
> > }
> >
> > static struct tst_test test = {
> > .test_all = run,
> > + .timeout = 20,
> > };
>
> --
> Mailing list info: https://lists.linux.it/listinfo/ltp
>
--
Regards,
Li Wang
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.linux.it/pipermail/ltp/attachments/20190507/0db6e2b4/attachment-0001.html>
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2019-05-07 12:57 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-05-07 11:38 [LTP] [PATCH v2] crypto/af_alg02: fixed read() being stuck Christian Amann
2019-05-07 11:41 ` Christian Amann
2019-05-07 12:57 ` Li Wang
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.