diff mbox series

[v5,09/11] kselftest: arm64: fake_sigreturn_duplicated_fpsimd

Message ID 20190902112932.36129-10-cristian.marussi@arm.com (mailing list archive)
State New, archived
Headers show
Series Add arm64/signal initial kselftest support | expand

Commit Message

Cristian Marussi Sept. 2, 2019, 11:29 a.m. UTC
Add a simple fake_sigreturn testcase which builds a ucontext_t with
an anomalous additional fpsimd_context and place it onto the stack.
Expects a SIGSEGV on test PASS.

Signed-off-by: Cristian Marussi <cristian.marussi@arm.com>
---
v3 --> v4
- fix commit
- missing include
- using new get_starting_head() helper
- added test description
---
 .../fake_sigreturn_duplicated_fpsimd.c        | 52 +++++++++++++++++++
 1 file changed, 52 insertions(+)
 create mode 100644 tools/testing/selftests/arm64/signal/testcases/fake_sigreturn_duplicated_fpsimd.c

Comments

Dave Martin Sept. 4, 2019, 11:49 a.m. UTC | #1
On Mon, Sep 02, 2019 at 12:29:30pm +0100, Cristian Marussi wrote:
> Add a simple fake_sigreturn testcase which builds a ucontext_t with
> an anomalous additional fpsimd_context and place it onto the stack.
> Expects a SIGSEGV on test PASS.
> 
> Signed-off-by: Cristian Marussi <cristian.marussi@arm.com>
> ---
> v3 --> v4
> - fix commit
> - missing include
> - using new get_starting_head() helper
> - added test description
> ---
>  .../fake_sigreturn_duplicated_fpsimd.c        | 52 +++++++++++++++++++
>  1 file changed, 52 insertions(+)
>  create mode 100644 tools/testing/selftests/arm64/signal/testcases/fake_sigreturn_duplicated_fpsimd.c
> 
> diff --git a/tools/testing/selftests/arm64/signal/testcases/fake_sigreturn_duplicated_fpsimd.c b/tools/testing/selftests/arm64/signal/testcases/fake_sigreturn_duplicated_fpsimd.c
> new file mode 100644
> index 000000000000..c7122c44f53f
> --- /dev/null
> +++ b/tools/testing/selftests/arm64/signal/testcases/fake_sigreturn_duplicated_fpsimd.c
> @@ -0,0 +1,52 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Copyright (C) 2019 ARM Limited
> + *
> + * Place a fake sigframe on the stack including an additional FPSIMD
> + * record: on sigreturn Kernel must spot this attempt and the test
> + * case is expected to be terminated via SEGV.
> + */
> +
> +#include <signal.h>
> +#include <ucontext.h>
> +
> +#include "test_signals_utils.h"
> +#include "testcases.h"
> +
> +struct fake_sigframe sf;
> +
> +static int fake_sigreturn_duplicated_fpsimd_run(struct tdescr *td,
> +						siginfo_t *si, ucontext_t *uc)
> +{
> +	size_t resv_sz, need_sz;
> +	struct _aarch64_ctx *shead = GET_SF_RESV_HEAD(sf), *head;
> +
> +	/* just to fill the ucontext_t with something real */
> +	if (!get_current_context(td, &sf.uc))
> +		return 1;
> +
> +	resv_sz = GET_SF_RESV_SIZE(sf);
> +	need_sz = HDR_SZ + sizeof(struct fpsimd_context);

Nit: Maybe write this sum in the same order as the records we're going 
o append, i.e.:

	need_sz = sizeof(struct fpsimd_context) + HDR_SZ; /* for terminator */

Also, maybe fail this test if there is no fpsimd_context in the initial
frame from get_current_context(): that would be a kernel bug, but we
wouldn't be giving fake_sigreturn() duplicate fpsimd_contexts in that
case -- so this test wouldn't test the thing it's supposed to test.

> +
> +	head = get_starting_head(shead, need_sz, resv_sz, NULL);
> +	if (head) {
> +		/* Add a spurios fpsimd_context */
> +		head->magic = FPSIMD_MAGIC;
> +		head->size = sizeof(struct fpsimd_context);
> +		/* and terminate */
> +		write_terminator_record(GET_RESV_NEXT_HEAD(head));
> +
> +		ASSERT_BAD_CONTEXT(&sf.uc);
> +		fake_sigreturn(&sf, sizeof(sf), 0);
> +	}
> +
> +	return 1;

[...]

Cheers
---Dave
Cristian Marussi Sept. 5, 2019, 12:15 p.m. UTC | #2
Hi

On 04/09/2019 12:49, Dave Martin wrote:
> On Mon, Sep 02, 2019 at 12:29:30pm +0100, Cristian Marussi wrote:
>> Add a simple fake_sigreturn testcase which builds a ucontext_t with
>> an anomalous additional fpsimd_context and place it onto the stack.
>> Expects a SIGSEGV on test PASS.
>>
>> Signed-off-by: Cristian Marussi <cristian.marussi@arm.com>
>> ---
>> v3 --> v4
>> - fix commit
>> - missing include
>> - using new get_starting_head() helper
>> - added test description
>> ---
>>  .../fake_sigreturn_duplicated_fpsimd.c        | 52 +++++++++++++++++++
>>  1 file changed, 52 insertions(+)
>>  create mode 100644 tools/testing/selftests/arm64/signal/testcases/fake_sigreturn_duplicated_fpsimd.c
>>
>> diff --git a/tools/testing/selftests/arm64/signal/testcases/fake_sigreturn_duplicated_fpsimd.c b/tools/testing/selftests/arm64/signal/testcases/fake_sigreturn_duplicated_fpsimd.c
>> new file mode 100644
>> index 000000000000..c7122c44f53f
>> --- /dev/null
>> +++ b/tools/testing/selftests/arm64/signal/testcases/fake_sigreturn_duplicated_fpsimd.c
>> @@ -0,0 +1,52 @@
>> +// SPDX-License-Identifier: GPL-2.0
>> +/*
>> + * Copyright (C) 2019 ARM Limited
>> + *
>> + * Place a fake sigframe on the stack including an additional FPSIMD
>> + * record: on sigreturn Kernel must spot this attempt and the test
>> + * case is expected to be terminated via SEGV.
>> + */
>> +
>> +#include <signal.h>
>> +#include <ucontext.h>
>> +
>> +#include "test_signals_utils.h"
>> +#include "testcases.h"
>> +
>> +struct fake_sigframe sf;
>> +
>> +static int fake_sigreturn_duplicated_fpsimd_run(struct tdescr *td,
>> +						siginfo_t *si, ucontext_t *uc)
>> +{
>> +	size_t resv_sz, need_sz;
>> +	struct _aarch64_ctx *shead = GET_SF_RESV_HEAD(sf), *head;
>> +
>> +	/* just to fill the ucontext_t with something real */
>> +	if (!get_current_context(td, &sf.uc))
>> +		return 1;
>> +
>> +	resv_sz = GET_SF_RESV_SIZE(sf);
>> +	need_sz = HDR_SZ + sizeof(struct fpsimd_context);
> 
> Nit: Maybe write this sum in the same order as the records we're going 
> o append, i.e.:
> 
> 	need_sz = sizeof(struct fpsimd_context) + HDR_SZ; /* for terminator */

Ok

> 
> Also, maybe fail this test if there is no fpsimd_context in the initial
> frame from get_current_context(): that would be a kernel bug, but we
> wouldn't be giving fake_sigreturn() duplicate fpsimd_contexts in that
> case -- so this test wouldn't test the thing it's supposed to test.
> 

Any context grabbed by get_current_context() is verified at first to be sane
when is copied in the handler by ASSERT_GOOD_CONTEXT()

>   } else if (signum == sig_copyctx && current->live_uc) {
>                 memcpy(current->live_uc, uc, current->live_sz);
>                 ASSERT_GOOD_CONTEXT(current->live_uc);
>                 current->live_uc_valid = 1;

A missing fpsimd in the original sigframe would lead to an abort()
straight away while preparing the test, and the test will fail.

Cheers

Cristian

>> +
>> +	head = get_starting_head(shead, need_sz, resv_sz, NULL);
>> +	if (head) {
>> +		/* Add a spurios fpsimd_context */
>> +		head->magic = FPSIMD_MAGIC;
>> +		head->size = sizeof(struct fpsimd_context);
>> +		/* and terminate */
>> +		write_terminator_record(GET_RESV_NEXT_HEAD(head));
>> +
>> +		ASSERT_BAD_CONTEXT(&sf.uc);
>> +		fake_sigreturn(&sf, sizeof(sf), 0);
>> +	}
>> +
>> +	return 1;
> 
> [...]
> 
> Cheers
> ---Dave
>
Dave Martin Sept. 5, 2019, 12:39 p.m. UTC | #3
On Thu, Sep 05, 2019 at 01:15:58PM +0100, Cristian Marussi wrote:
> Hi
> 
> On 04/09/2019 12:49, Dave Martin wrote:
> > On Mon, Sep 02, 2019 at 12:29:30pm +0100, Cristian Marussi wrote:
> >> Add a simple fake_sigreturn testcase which builds a ucontext_t with
> >> an anomalous additional fpsimd_context and place it onto the stack.
> >> Expects a SIGSEGV on test PASS.
> >>
> >> Signed-off-by: Cristian Marussi <cristian.marussi@arm.com>
> >> ---
> >> v3 --> v4
> >> - fix commit
> >> - missing include
> >> - using new get_starting_head() helper
> >> - added test description
> >> ---
> >>  .../fake_sigreturn_duplicated_fpsimd.c        | 52 +++++++++++++++++++
> >>  1 file changed, 52 insertions(+)
> >>  create mode 100644 tools/testing/selftests/arm64/signal/testcases/fake_sigreturn_duplicated_fpsimd.c
> >>
> >> diff --git a/tools/testing/selftests/arm64/signal/testcases/fake_sigreturn_duplicated_fpsimd.c b/tools/testing/selftests/arm64/signal/testcases/fake_sigreturn_duplicated_fpsimd.c
> >> new file mode 100644
> >> index 000000000000..c7122c44f53f
> >> --- /dev/null
> >> +++ b/tools/testing/selftests/arm64/signal/testcases/fake_sigreturn_duplicated_fpsimd.c
> >> @@ -0,0 +1,52 @@
> >> +// SPDX-License-Identifier: GPL-2.0
> >> +/*
> >> + * Copyright (C) 2019 ARM Limited
> >> + *
> >> + * Place a fake sigframe on the stack including an additional FPSIMD
> >> + * record: on sigreturn Kernel must spot this attempt and the test
> >> + * case is expected to be terminated via SEGV.
> >> + */
> >> +
> >> +#include <signal.h>
> >> +#include <ucontext.h>
> >> +
> >> +#include "test_signals_utils.h"
> >> +#include "testcases.h"
> >> +
> >> +struct fake_sigframe sf;
> >> +
> >> +static int fake_sigreturn_duplicated_fpsimd_run(struct tdescr *td,
> >> +						siginfo_t *si, ucontext_t *uc)
> >> +{
> >> +	size_t resv_sz, need_sz;
> >> +	struct _aarch64_ctx *shead = GET_SF_RESV_HEAD(sf), *head;
> >> +
> >> +	/* just to fill the ucontext_t with something real */
> >> +	if (!get_current_context(td, &sf.uc))
> >> +		return 1;
> >> +
> >> +	resv_sz = GET_SF_RESV_SIZE(sf);
> >> +	need_sz = HDR_SZ + sizeof(struct fpsimd_context);
> > 
> > Nit: Maybe write this sum in the same order as the records we're going 
> > o append, i.e.:
> > 
> > 	need_sz = sizeof(struct fpsimd_context) + HDR_SZ; /* for terminator */
> 
> Ok
> 
> > 
> > Also, maybe fail this test if there is no fpsimd_context in the initial
> > frame from get_current_context(): that would be a kernel bug, but we
> > wouldn't be giving fake_sigreturn() duplicate fpsimd_contexts in that
> > case -- so this test wouldn't test the thing it's supposed to test.
> > 
> 
> Any context grabbed by get_current_context() is verified at first to be sane
> when is copied in the handler by ASSERT_GOOD_CONTEXT()
> 
> >   } else if (signum == sig_copyctx && current->live_uc) {
> >                 memcpy(current->live_uc, uc, current->live_sz);
> >                 ASSERT_GOOD_CONTEXT(current->live_uc);
> >                 current->live_uc_valid = 1;
> 
> A missing fpsimd in the original sigframe would lead to an abort()
> straight away while preparing the test, and the test will fail.

OK, but there is no abort() in ASSERT_GOOD_CONTEXT(), only assert(0).
Can you add an abort() after the assert() in there in patch 2?

People probably aren't going to be building the tests with -DNDEBUG, but
we should avoid unnecessary surprises...

Cheers
---Dave
Cristian Marussi Sept. 5, 2019, 1:32 p.m. UTC | #4
On 05/09/2019 13:39, Dave Martin wrote:
> On Thu, Sep 05, 2019 at 01:15:58PM +0100, Cristian Marussi wrote:
>> Hi
>>
>> On 04/09/2019 12:49, Dave Martin wrote:
>>> On Mon, Sep 02, 2019 at 12:29:30pm +0100, Cristian Marussi wrote:
>>>> Add a simple fake_sigreturn testcase which builds a ucontext_t with
>>>> an anomalous additional fpsimd_context and place it onto the stack.
>>>> Expects a SIGSEGV on test PASS.
>>>>
>>>> Signed-off-by: Cristian Marussi <cristian.marussi@arm.com>
>>>> ---
>>>> v3 --> v4
>>>> - fix commit
>>>> - missing include
>>>> - using new get_starting_head() helper
>>>> - added test description
>>>> ---
>>>>  .../fake_sigreturn_duplicated_fpsimd.c        | 52 +++++++++++++++++++
>>>>  1 file changed, 52 insertions(+)
>>>>  create mode 100644 tools/testing/selftests/arm64/signal/testcases/fake_sigreturn_duplicated_fpsimd.c
>>>>
>>>> diff --git a/tools/testing/selftests/arm64/signal/testcases/fake_sigreturn_duplicated_fpsimd.c b/tools/testing/selftests/arm64/signal/testcases/fake_sigreturn_duplicated_fpsimd.c
>>>> new file mode 100644
>>>> index 000000000000..c7122c44f53f
>>>> --- /dev/null
>>>> +++ b/tools/testing/selftests/arm64/signal/testcases/fake_sigreturn_duplicated_fpsimd.c
>>>> @@ -0,0 +1,52 @@
>>>> +// SPDX-License-Identifier: GPL-2.0
>>>> +/*
>>>> + * Copyright (C) 2019 ARM Limited
>>>> + *
>>>> + * Place a fake sigframe on the stack including an additional FPSIMD
>>>> + * record: on sigreturn Kernel must spot this attempt and the test
>>>> + * case is expected to be terminated via SEGV.
>>>> + */
>>>> +
>>>> +#include <signal.h>
>>>> +#include <ucontext.h>
>>>> +
>>>> +#include "test_signals_utils.h"
>>>> +#include "testcases.h"
>>>> +
>>>> +struct fake_sigframe sf;
>>>> +
>>>> +static int fake_sigreturn_duplicated_fpsimd_run(struct tdescr *td,
>>>> +						siginfo_t *si, ucontext_t *uc)
>>>> +{
>>>> +	size_t resv_sz, need_sz;
>>>> +	struct _aarch64_ctx *shead = GET_SF_RESV_HEAD(sf), *head;
>>>> +
>>>> +	/* just to fill the ucontext_t with something real */
>>>> +	if (!get_current_context(td, &sf.uc))
>>>> +		return 1;
>>>> +
>>>> +	resv_sz = GET_SF_RESV_SIZE(sf);
>>>> +	need_sz = HDR_SZ + sizeof(struct fpsimd_context);
>>>
>>> Nit: Maybe write this sum in the same order as the records we're going 
>>> o append, i.e.:
>>>
>>> 	need_sz = sizeof(struct fpsimd_context) + HDR_SZ; /* for terminator */
>>
>> Ok
>>
>>>
>>> Also, maybe fail this test if there is no fpsimd_context in the initial
>>> frame from get_current_context(): that would be a kernel bug, but we
>>> wouldn't be giving fake_sigreturn() duplicate fpsimd_contexts in that
>>> case -- so this test wouldn't test the thing it's supposed to test.
>>>
>>
>> Any context grabbed by get_current_context() is verified at first to be sane
>> when is copied in the handler by ASSERT_GOOD_CONTEXT()
>>
>>>   } else if (signum == sig_copyctx && current->live_uc) {
>>>                 memcpy(current->live_uc, uc, current->live_sz);
>>>                 ASSERT_GOOD_CONTEXT(current->live_uc);
>>>                 current->live_uc_valid = 1;
>>
>> A missing fpsimd in the original sigframe would lead to an abort()
>> straight away while preparing the test, and the test will fail.
> 
> OK, but there is no abort() in ASSERT_GOOD_CONTEXT(), only assert(0).
> Can you add an abort() after the assert() in there in patch 2?
> 
Ok yes, I meant the abort coming from assert(0) in fact....I'll review all
the critical asserts for additional aborts in V6

> People probably aren't going to be building the tests with -DNDEBUG, but
> we should avoid unnecessary surprises...
> 
> Cheers
> ---Dave
>
Dave Martin Sept. 5, 2019, 2:20 p.m. UTC | #5
On Thu, Sep 05, 2019 at 02:32:09PM +0100, Cristian Marussi wrote:
> On 05/09/2019 13:39, Dave Martin wrote:
> > On Thu, Sep 05, 2019 at 01:15:58PM +0100, Cristian Marussi wrote:
> >> Hi
> >>
> >> On 04/09/2019 12:49, Dave Martin wrote:
> >>> On Mon, Sep 02, 2019 at 12:29:30pm +0100, Cristian Marussi wrote:
> >>>> Add a simple fake_sigreturn testcase which builds a ucontext_t with
> >>>> an anomalous additional fpsimd_context and place it onto the stack.
> >>>> Expects a SIGSEGV on test PASS.
> >>>>
> >>>> Signed-off-by: Cristian Marussi <cristian.marussi@arm.com>
> >>>> ---
> >>>> v3 --> v4
> >>>> - fix commit
> >>>> - missing include
> >>>> - using new get_starting_head() helper
> >>>> - added test description
> >>>> ---
> >>>>  .../fake_sigreturn_duplicated_fpsimd.c        | 52 +++++++++++++++++++
> >>>>  1 file changed, 52 insertions(+)
> >>>>  create mode 100644 tools/testing/selftests/arm64/signal/testcases/fake_sigreturn_duplicated_fpsimd.c
> >>>>
> >>>> diff --git a/tools/testing/selftests/arm64/signal/testcases/fake_sigreturn_duplicated_fpsimd.c b/tools/testing/selftests/arm64/signal/testcases/fake_sigreturn_duplicated_fpsimd.c
> >>>> new file mode 100644
> >>>> index 000000000000..c7122c44f53f
> >>>> --- /dev/null
> >>>> +++ b/tools/testing/selftests/arm64/signal/testcases/fake_sigreturn_duplicated_fpsimd.c
> >>>> @@ -0,0 +1,52 @@
> >>>> +// SPDX-License-Identifier: GPL-2.0
> >>>> +/*
> >>>> + * Copyright (C) 2019 ARM Limited
> >>>> + *
> >>>> + * Place a fake sigframe on the stack including an additional FPSIMD
> >>>> + * record: on sigreturn Kernel must spot this attempt and the test
> >>>> + * case is expected to be terminated via SEGV.
> >>>> + */
> >>>> +
> >>>> +#include <signal.h>
> >>>> +#include <ucontext.h>
> >>>> +
> >>>> +#include "test_signals_utils.h"
> >>>> +#include "testcases.h"
> >>>> +
> >>>> +struct fake_sigframe sf;
> >>>> +
> >>>> +static int fake_sigreturn_duplicated_fpsimd_run(struct tdescr *td,
> >>>> +						siginfo_t *si, ucontext_t *uc)
> >>>> +{
> >>>> +	size_t resv_sz, need_sz;
> >>>> +	struct _aarch64_ctx *shead = GET_SF_RESV_HEAD(sf), *head;
> >>>> +
> >>>> +	/* just to fill the ucontext_t with something real */
> >>>> +	if (!get_current_context(td, &sf.uc))
> >>>> +		return 1;
> >>>> +
> >>>> +	resv_sz = GET_SF_RESV_SIZE(sf);
> >>>> +	need_sz = HDR_SZ + sizeof(struct fpsimd_context);
> >>>
> >>> Nit: Maybe write this sum in the same order as the records we're going 
> >>> o append, i.e.:
> >>>
> >>> 	need_sz = sizeof(struct fpsimd_context) + HDR_SZ; /* for terminator */
> >>
> >> Ok
> >>
> >>>
> >>> Also, maybe fail this test if there is no fpsimd_context in the initial
> >>> frame from get_current_context(): that would be a kernel bug, but we
> >>> wouldn't be giving fake_sigreturn() duplicate fpsimd_contexts in that
> >>> case -- so this test wouldn't test the thing it's supposed to test.
> >>>
> >>
> >> Any context grabbed by get_current_context() is verified at first to be sane
> >> when is copied in the handler by ASSERT_GOOD_CONTEXT()
> >>
> >>>   } else if (signum == sig_copyctx && current->live_uc) {
> >>>                 memcpy(current->live_uc, uc, current->live_sz);
> >>>                 ASSERT_GOOD_CONTEXT(current->live_uc);
> >>>                 current->live_uc_valid = 1;
> >>
> >> A missing fpsimd in the original sigframe would lead to an abort()
> >> straight away while preparing the test, and the test will fail.
> > 
> > OK, but there is no abort() in ASSERT_GOOD_CONTEXT(), only assert(0).
> > Can you add an abort() after the assert() in there in patch 2?
> > 
> Ok yes, I meant the abort coming from assert(0) in fact....I'll review all
> the critical asserts for additional aborts in V6

OK -- I guess this only matters for things that should be reported as
test failures.  Things that are purely bugs in the test are less of a
concern.

Cheers
---Dave
Cristian Marussi Sept. 9, 2019, 6:03 p.m. UTC | #6
On 04/09/2019 12:49, Dave Martin wrote:
> On Mon, Sep 02, 2019 at 12:29:30pm +0100, Cristian Marussi wrote:
>> Add a simple fake_sigreturn testcase which builds a ucontext_t with
>> an anomalous additional fpsimd_context and place it onto the stack.
>> Expects a SIGSEGV on test PASS.
>>
>> Signed-off-by: Cristian Marussi <cristian.marussi@arm.com>
>> ---
>> v3 --> v4
>> - fix commit
>> - missing include
>> - using new get_starting_head() helper
>> - added test description
>> ---
>>  .../fake_sigreturn_duplicated_fpsimd.c        | 52 +++++++++++++++++++
>>  1 file changed, 52 insertions(+)
>>  create mode 100644 tools/testing/selftests/arm64/signal/testcases/fake_sigreturn_duplicated_fpsimd.c
>>
>> diff --git a/tools/testing/selftests/arm64/signal/testcases/fake_sigreturn_duplicated_fpsimd.c b/tools/testing/selftests/arm64/signal/testcases/fake_sigreturn_duplicated_fpsimd.c
>> new file mode 100644
>> index 000000000000..c7122c44f53f
>> --- /dev/null
>> +++ b/tools/testing/selftests/arm64/signal/testcases/fake_sigreturn_duplicated_fpsimd.c
>> @@ -0,0 +1,52 @@
>> +// SPDX-License-Identifier: GPL-2.0
>> +/*
>> + * Copyright (C) 2019 ARM Limited
>> + *
>> + * Place a fake sigframe on the stack including an additional FPSIMD
>> + * record: on sigreturn Kernel must spot this attempt and the test
>> + * case is expected to be terminated via SEGV.
>> + */
>> +
>> +#include <signal.h>
>> +#include <ucontext.h>
>> +
>> +#include "test_signals_utils.h"
>> +#include "testcases.h"
>> +
>> +struct fake_sigframe sf;
>> +
>> +static int fake_sigreturn_duplicated_fpsimd_run(struct tdescr *td,
>> +						siginfo_t *si, ucontext_t *uc)
>> +{
>> +	size_t resv_sz, need_sz;
>> +	struct _aarch64_ctx *shead = GET_SF_RESV_HEAD(sf), *head;
>> +
>> +	/* just to fill the ucontext_t with something real */
>> +	if (!get_current_context(td, &sf.uc))
>> +		return 1;
>> +
>> +	resv_sz = GET_SF_RESV_SIZE(sf);
>> +	need_sz = HDR_SZ + sizeof(struct fpsimd_context);
> 
> Nit: Maybe write this sum in the same order as the records we're going 
> o append, i.e.:
> 
> 	need_sz = sizeof(struct fpsimd_context) + HDR_SZ; /* for terminator */
> 

Ok
> Also, maybe fail this test if there is no fpsimd_context in the initial
> frame from get_current_context(): that would be a kernel bug, but we
> wouldn't be giving fake_sigreturn() duplicate fpsimd_contexts in that
> case -- so this test wouldn't test the thing it's supposed to test.
> 

I've reviewed  ASSERT_BAD/GOOD_CONTEXT() macros in this series to use abort()
instead of assert(): this means any frame get with get_current_context() will
be checked to be GOOD before the test starts.

>> +
>> +	head = get_starting_head(shead, need_sz, resv_sz, NULL);
>> +	if (head) {
>> +		/* Add a spurios fpsimd_context */
>> +		head->magic = FPSIMD_MAGIC;
>> +		head->size = sizeof(struct fpsimd_context);
>> +		/* and terminate */
>> +		write_terminator_record(GET_RESV_NEXT_HEAD(head));
>> +
>> +		ASSERT_BAD_CONTEXT(&sf.uc);
>> +		fake_sigreturn(&sf, sizeof(sf), 0);
>> +	}
>> +
>> +	return 1;
> 

Here I'll avoid the timeout on !head and exit straight away (like in other fake_ tests)
> [...]
> 
> Cheers
> ---Dave
> 

Cheers

Cristian
diff mbox series

Patch

diff --git a/tools/testing/selftests/arm64/signal/testcases/fake_sigreturn_duplicated_fpsimd.c b/tools/testing/selftests/arm64/signal/testcases/fake_sigreturn_duplicated_fpsimd.c
new file mode 100644
index 000000000000..c7122c44f53f
--- /dev/null
+++ b/tools/testing/selftests/arm64/signal/testcases/fake_sigreturn_duplicated_fpsimd.c
@@ -0,0 +1,52 @@ 
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Copyright (C) 2019 ARM Limited
+ *
+ * Place a fake sigframe on the stack including an additional FPSIMD
+ * record: on sigreturn Kernel must spot this attempt and the test
+ * case is expected to be terminated via SEGV.
+ */
+
+#include <signal.h>
+#include <ucontext.h>
+
+#include "test_signals_utils.h"
+#include "testcases.h"
+
+struct fake_sigframe sf;
+
+static int fake_sigreturn_duplicated_fpsimd_run(struct tdescr *td,
+						siginfo_t *si, ucontext_t *uc)
+{
+	size_t resv_sz, need_sz;
+	struct _aarch64_ctx *shead = GET_SF_RESV_HEAD(sf), *head;
+
+	/* just to fill the ucontext_t with something real */
+	if (!get_current_context(td, &sf.uc))
+		return 1;
+
+	resv_sz = GET_SF_RESV_SIZE(sf);
+	need_sz = HDR_SZ + sizeof(struct fpsimd_context);
+
+	head = get_starting_head(shead, need_sz, resv_sz, NULL);
+	if (head) {
+		/* Add a spurios fpsimd_context */
+		head->magic = FPSIMD_MAGIC;
+		head->size = sizeof(struct fpsimd_context);
+		/* and terminate */
+		write_terminator_record(GET_RESV_NEXT_HEAD(head));
+
+		ASSERT_BAD_CONTEXT(&sf.uc);
+		fake_sigreturn(&sf, sizeof(sf), 0);
+	}
+
+	return 1;
+}
+
+struct tdescr tde = {
+		.name = "FAKE_SIGRETURN_DUPLICATED_FPSIMD",
+		.descr = "Triggers a sigreturn including two fpsimd_context",
+		.sig_ok = SIGSEGV,
+		.timeout = 3,
+		.run = fake_sigreturn_duplicated_fpsimd_run,
+};