[v8,7/7] selftest: test system mappings are sealed.

Commit Message

Jeff Xu March 3, 2025, 5:09 a.m. UTC

From: Jeff Xu <[e-mail]>

Add sysmap_is_sealed.c to test system mappings are sealed.

Note: CONFIG_MSEAL_SYSTEM_MAPPINGS must be set, as indicated in
config file.

Signed-off-by: Jeff Xu <[e-mail]>
---
 .../mseal_system_mappings/.gitignore          |   2 +
 .../selftests/mseal_system_mappings/Makefile  |   6 +
 .../selftests/mseal_system_mappings/config    |   1 +
 .../mseal_system_mappings/sysmap_is_sealed.c  | 113 ++++++++++++++++++
 4 files changed, 122 insertions(+)
 create mode 100644 tools/testing/selftests/mseal_system_mappings/.gitignore
 create mode 100644 tools/testing/selftests/mseal_system_mappings/Makefile
 create mode 100644 tools/testing/selftests/mseal_system_mappings/config
 create mode 100644 tools/testing/selftests/mseal_system_mappings/sysmap_is_sealed.c

Comments

Lorenzo Stoakes March 3, 2025, 12:08 p.m. UTC | #1

On Mon, Mar 03, 2025 at 05:09:21AM +0000, [e-mail] wrote:
> From: Jeff Xu <[e-mail]>
>
> Add sysmap_is_sealed.c to test system mappings are sealed.
>
> Note: CONFIG_MSEAL_SYSTEM_MAPPINGS must be set, as indicated in
> config file.
>
> Signed-off-by: Jeff Xu <[e-mail]>

We do need to add this to the general selftests Makefile, but this code is
fine, so have a:

Reviewed-by: Lorenzo Stoakes <[e-mail]>

Congratulations! :) and thanks for addressing the issues that were raised,
appreciate your efforts on this.

Maybe you could send a fix patch? As it's such a small fix.

Cheers, Lorenzo


> ---
>  .../mseal_system_mappings/.gitignore          |   2 +
>  .../selftests/mseal_system_mappings/Makefile  |   6 +
>  .../selftests/mseal_system_mappings/config    |   1 +
>  .../mseal_system_mappings/sysmap_is_sealed.c  | 113 ++++++++++++++++++

Can you add this to tools/testing/selftests/Makefile? I _think_ adding:

TARGETS += mm

Should do it. Thanks!

>  4 files changed, 122 insertions(+)
>  create mode 100644 tools/testing/selftests/mseal_system_mappings/.gitignore
>  create mode 100644 tools/testing/selftests/mseal_system_mappings/Makefile
>  create mode 100644 tools/testing/selftests/mseal_system_mappings/config
>  create mode 100644 tools/testing/selftests/mseal_system_mappings/sysmap_is_sealed.c
>
> diff --git a/tools/testing/selftests/mseal_system_mappings/.gitignore b/tools/testing/selftests/mseal_system_mappings/.gitignore
> new file mode 100644
> index 000000000000..319c497a595e
> --- /dev/null
> +++ b/tools/testing/selftests/mseal_system_mappings/.gitignore
> @@ -0,0 +1,2 @@
> +# SPDX-License-Identifier: GPL-2.0-only
> +sysmap_is_sealed
> diff --git a/tools/testing/selftests/mseal_system_mappings/Makefile b/tools/testing/selftests/mseal_system_mappings/Makefile
> new file mode 100644
> index 000000000000..2b4504e2f52f
> --- /dev/null
> +++ b/tools/testing/selftests/mseal_system_mappings/Makefile
> @@ -0,0 +1,6 @@
> +# SPDX-License-Identifier: GPL-2.0-only
> +CFLAGS += -std=c99 -pthread -Wall $(KHDR_INCLUDES)
> +
> +TEST_GEN_PROGS := sysmap_is_sealed
> +
> +include ../lib.mk
> diff --git a/tools/testing/selftests/mseal_system_mappings/config b/tools/testing/selftests/mseal_system_mappings/config
> new file mode 100644
> index 000000000000..675cb9f37b86
> --- /dev/null
> +++ b/tools/testing/selftests/mseal_system_mappings/config
> @@ -0,0 +1 @@
> +CONFIG_MSEAL_SYSTEM_MAPPINGS=y
> diff --git a/tools/testing/selftests/mseal_system_mappings/sysmap_is_sealed.c b/tools/testing/selftests/mseal_system_mappings/sysmap_is_sealed.c
> new file mode 100644
> index 000000000000..c1e93794a58b
> --- /dev/null
> +++ b/tools/testing/selftests/mseal_system_mappings/sysmap_is_sealed.c
> @@ -0,0 +1,113 @@
> +// SPDX-License-Identifier: GPL-2.0-only
> +/*
> + * test system mappings are sealed when
> + * KCONFIG_MSEAL_SYSTEM_MAPPINGS=y
> + */
> +
> +#define _GNU_SOURCE
> +#include <stdio.h>
> +#include <errno.h>
> +#include <unistd.h>
> +#include <string.h>
> +#include <stdbool.h>
> +
> +#include "../kselftest.h"
> +#include "../kselftest_harness.h"
> +
> +#define VDSO_NAME "[vdso]"
> +#define VVAR_NAME "[vvar]"
> +#define VVAR_VCLOCK_NAME "[vvar_vclock]"
> +#define UPROBES_NAME "[uprobes]"
> +#define SIGPAGE_NAME "[sigpage]"
> +#define VECTORS_NAME "[vectors]"
> +
> +#define VMFLAGS "VmFlags:"
> +#define MSEAL_FLAGS "sl"
> +#define MAX_LINE_LEN 512
> +
> +bool has_mapping(char *name, FILE *maps)
> +{
> +	char line[MAX_LINE_LEN];
> +
> +	while (fgets(line, sizeof(line), maps)) {
> +		if (strstr(line, name))
> +			return true;
> +	}
> +
> +	return false;
> +}
> +
> +bool mapping_is_sealed(char *name, FILE *maps)
> +{
> +	char line[MAX_LINE_LEN];
> +
> +	while (fgets(line, sizeof(line), maps)) {
> +		if (!strncmp(line, VMFLAGS, strlen(VMFLAGS))) {
> +			if (strstr(line, MSEAL_FLAGS))
> +				return true;
> +
> +			return false;
> +		}
> +	}
> +
> +	return false;
> +}
> +
> +FIXTURE(basic) {
> +	FILE *maps;
> +};
> +
> +FIXTURE_SETUP(basic)
> +{
> +	self->maps = fopen("/proc/self/smaps", "r");
> +	if (!self->maps)
> +		SKIP(return, "Could not open /proc/self/smap, errno=%d",
> +			errno);
> +};
> +
> +FIXTURE_TEARDOWN(basic)
> +{
> +	if (self->maps)
> +		fclose(self->maps);
> +};
> +
> +FIXTURE_VARIANT(basic)
> +{
> +	char *name;
> +};
> +
> +FIXTURE_VARIANT_ADD(basic, vdso) {
> +	.name = VDSO_NAME,
> +};
> +
> +FIXTURE_VARIANT_ADD(basic, vvar) {
> +	.name = VVAR_NAME,
> +};
> +
> +FIXTURE_VARIANT_ADD(basic, vvar_vclock) {
> +	.name = VVAR_VCLOCK_NAME,
> +};
> +
> +FIXTURE_VARIANT_ADD(basic, sigpage) {
> +	.name = SIGPAGE_NAME,
> +};
> +
> +FIXTURE_VARIANT_ADD(basic, vectors) {
> +	.name = VECTORS_NAME,
> +};
> +
> +FIXTURE_VARIANT_ADD(basic, uprobes) {
> +	.name = UPROBES_NAME,
> +};
> +
> +TEST_F(basic, is_sealed)
> +{
> +	if (!has_mapping(variant->name, self->maps)) {
> +		SKIP(return, "could not found the mapping, %s",
> +			variant->name);
> +	}
> +
> +	EXPECT_TRUE(mapping_is_sealed(variant->name, self->maps));
> +};
> +
> +TEST_HARNESS_MAIN
> --
> 2.48.1.711.g2feabab25a-goog
>

Lorenzo Stoakes March 3, 2025, 4:43 p.m. UTC | #2

On Mon, Mar 03, 2025 at 12:08:49PM +0000, Lorenzo Stoakes wrote:
>
> On Mon, Mar 03, 2025 at 05:09:21AM +0000, [e-mail] wrote:
> > From: Jeff Xu <[e-mail]>
> >
> > Add sysmap_is_sealed.c to test system mappings are sealed.
> >
> > Note: CONFIG_MSEAL_SYSTEM_MAPPINGS must be set, as indicated in
> > config file.
> >
> > Signed-off-by: Jeff Xu <[e-mail]>
>
> We do need to add this to the general selftests Makefile, but this code is
> fine, so have a:
>
> Reviewed-by: Lorenzo Stoakes <[e-mail]>
>
> Congratulations! :) and thanks for addressing the issues that were raised,
> appreciate your efforts on this.
>
> Maybe you could send a fix patch? As it's such a small fix.
>
> Cheers, Lorenzo
>
>
> > ---
> >  .../mseal_system_mappings/.gitignore          |   2 +
> >  .../selftests/mseal_system_mappings/Makefile  |   6 +
> >  .../selftests/mseal_system_mappings/config    |   1 +
> >  .../mseal_system_mappings/sysmap_is_sealed.c  | 113 ++++++++++++++++++
>
> Can you add this to tools/testing/selftests/Makefile? I _think_ adding:
>
> TARGETS += mm
>
> Should do it. Thanks!

Obviously I meant to say:

	TARGETS += mseal_system_mappings

Doh! :)

>
> >  4 files changed, 122 insertions(+)
> >  create mode 100644 tools/testing/selftests/mseal_system_mappings/.gitignore
> >  create mode 100644 tools/testing/selftests/mseal_system_mappings/Makefile
> >  create mode 100644 tools/testing/selftests/mseal_system_mappings/config
> >  create mode 100644 tools/testing/selftests/mseal_system_mappings/sysmap_is_sealed.c
> >
> > diff --git a/tools/testing/selftests/mseal_system_mappings/.gitignore b/tools/testing/selftests/mseal_system_mappings/.gitignore
> > new file mode 100644
> > index 000000000000..319c497a595e
> > --- /dev/null
> > +++ b/tools/testing/selftests/mseal_system_mappings/.gitignore
> > @@ -0,0 +1,2 @@
> > +# SPDX-License-Identifier: GPL-2.0-only
> > +sysmap_is_sealed
> > diff --git a/tools/testing/selftests/mseal_system_mappings/Makefile b/tools/testing/selftests/mseal_system_mappings/Makefile
> > new file mode 100644
> > index 000000000000..2b4504e2f52f
> > --- /dev/null
> > +++ b/tools/testing/selftests/mseal_system_mappings/Makefile
> > @@ -0,0 +1,6 @@
> > +# SPDX-License-Identifier: GPL-2.0-only
> > +CFLAGS += -std=c99 -pthread -Wall $(KHDR_INCLUDES)
> > +
> > +TEST_GEN_PROGS := sysmap_is_sealed
> > +
> > +include ../lib.mk
> > diff --git a/tools/testing/selftests/mseal_system_mappings/config b/tools/testing/selftests/mseal_system_mappings/config
> > new file mode 100644
> > index 000000000000..675cb9f37b86
> > --- /dev/null
> > +++ b/tools/testing/selftests/mseal_system_mappings/config
> > @@ -0,0 +1 @@
> > +CONFIG_MSEAL_SYSTEM_MAPPINGS=y
> > diff --git a/tools/testing/selftests/mseal_system_mappings/sysmap_is_sealed.c b/tools/testing/selftests/mseal_system_mappings/sysmap_is_sealed.c
> > new file mode 100644
> > index 000000000000..c1e93794a58b
> > --- /dev/null
> > +++ b/tools/testing/selftests/mseal_system_mappings/sysmap_is_sealed.c
> > @@ -0,0 +1,113 @@
> > +// SPDX-License-Identifier: GPL-2.0-only
> > +/*
> > + * test system mappings are sealed when
> > + * KCONFIG_MSEAL_SYSTEM_MAPPINGS=y
> > + */
> > +
> > +#define _GNU_SOURCE
> > +#include <stdio.h>
> > +#include <errno.h>
> > +#include <unistd.h>
> > +#include <string.h>
> > +#include <stdbool.h>
> > +
> > +#include "../kselftest.h"
> > +#include "../kselftest_harness.h"
> > +
> > +#define VDSO_NAME "[vdso]"
> > +#define VVAR_NAME "[vvar]"
> > +#define VVAR_VCLOCK_NAME "[vvar_vclock]"
> > +#define UPROBES_NAME "[uprobes]"
> > +#define SIGPAGE_NAME "[sigpage]"
> > +#define VECTORS_NAME "[vectors]"
> > +
> > +#define VMFLAGS "VmFlags:"
> > +#define MSEAL_FLAGS "sl"
> > +#define MAX_LINE_LEN 512
> > +
> > +bool has_mapping(char *name, FILE *maps)
> > +{
> > +	char line[MAX_LINE_LEN];
> > +
> > +	while (fgets(line, sizeof(line), maps)) {
> > +		if (strstr(line, name))
> > +			return true;
> > +	}
> > +
> > +	return false;
> > +}
> > +
> > +bool mapping_is_sealed(char *name, FILE *maps)
> > +{
> > +	char line[MAX_LINE_LEN];
> > +
> > +	while (fgets(line, sizeof(line), maps)) {
> > +		if (!strncmp(line, VMFLAGS, strlen(VMFLAGS))) {
> > +			if (strstr(line, MSEAL_FLAGS))
> > +				return true;
> > +
> > +			return false;
> > +		}
> > +	}
> > +
> > +	return false;
> > +}
> > +
> > +FIXTURE(basic) {
> > +	FILE *maps;
> > +};
> > +
> > +FIXTURE_SETUP(basic)
> > +{
> > +	self->maps = fopen("/proc/self/smaps", "r");
> > +	if (!self->maps)
> > +		SKIP(return, "Could not open /proc/self/smap, errno=%d",
> > +			errno);
> > +};
> > +
> > +FIXTURE_TEARDOWN(basic)
> > +{
> > +	if (self->maps)
> > +		fclose(self->maps);
> > +};
> > +
> > +FIXTURE_VARIANT(basic)
> > +{
> > +	char *name;
> > +};
> > +
> > +FIXTURE_VARIANT_ADD(basic, vdso) {
> > +	.name = VDSO_NAME,
> > +};
> > +
> > +FIXTURE_VARIANT_ADD(basic, vvar) {
> > +	.name = VVAR_NAME,
> > +};
> > +
> > +FIXTURE_VARIANT_ADD(basic, vvar_vclock) {
> > +	.name = VVAR_VCLOCK_NAME,
> > +};
> > +
> > +FIXTURE_VARIANT_ADD(basic, sigpage) {
> > +	.name = SIGPAGE_NAME,
> > +};
> > +
> > +FIXTURE_VARIANT_ADD(basic, vectors) {
> > +	.name = VECTORS_NAME,
> > +};
> > +
> > +FIXTURE_VARIANT_ADD(basic, uprobes) {
> > +	.name = UPROBES_NAME,
> > +};
> > +
> > +TEST_F(basic, is_sealed)
> > +{
> > +	if (!has_mapping(variant->name, self->maps)) {
> > +		SKIP(return, "could not found the mapping, %s",
> > +			variant->name);
> > +	}
> > +
> > +	EXPECT_TRUE(mapping_is_sealed(variant->name, self->maps));
> > +};
> > +
> > +TEST_HARNESS_MAIN
> > --
> > 2.48.1.711.g2feabab25a-goog
> >

Kees Cook March 3, 2025, 4:47 p.m. UTC | #3

On Mon, Mar 03, 2025 at 12:08:49PM +0000, Lorenzo Stoakes wrote:
> 
> On Mon, Mar 03, 2025 at 05:09:21AM +0000, [e-mail] wrote:
> > From: Jeff Xu <[e-mail]>
> >
> > Add sysmap_is_sealed.c to test system mappings are sealed.
> >
> > Note: CONFIG_MSEAL_SYSTEM_MAPPINGS must be set, as indicated in
> > config file.
> >
> > Signed-off-by: Jeff Xu <[e-mail]>
> 
> We do need to add this to the general selftests Makefile, but this code is
> fine, so have a:
> 
> Reviewed-by: Lorenzo Stoakes <[e-mail]>
> 
> Congratulations! :) and thanks for addressing the issues that were raised,
> appreciate your efforts on this.
> 
> Maybe you could send a fix patch? As it's such a small fix.
> 
> Cheers, Lorenzo
> 
> 
> > ---
> >  .../mseal_system_mappings/.gitignore          |   2 +
> >  .../selftests/mseal_system_mappings/Makefile  |   6 +
> >  .../selftests/mseal_system_mappings/config    |   1 +
> >  .../mseal_system_mappings/sysmap_is_sealed.c  | 113 ++++++++++++++++++
> 
> Can you add this to tools/testing/selftests/Makefile? I _think_ adding:
> 
> TARGETS += mm

You mean this, I think?

TARGETS += mseal_system_mappings

(But while we're here, why is "mm" where it is in that Makefile?
Everything else is alphabetical?)

-Kees

Kees Cook March 3, 2025, 4:48 p.m. UTC | #4

On Mon, Mar 03, 2025 at 04:43:47PM +0000, Lorenzo Stoakes wrote:
> On Mon, Mar 03, 2025 at 12:08:49PM +0000, Lorenzo Stoakes wrote:
> >
> > On Mon, Mar 03, 2025 at 05:09:21AM +0000, [e-mail] wrote:
> > > From: Jeff Xu <[e-mail]>
> > >
> > > Add sysmap_is_sealed.c to test system mappings are sealed.
> > >
> > > Note: CONFIG_MSEAL_SYSTEM_MAPPINGS must be set, as indicated in
> > > config file.
> > >
> > > Signed-off-by: Jeff Xu <[e-mail]>
> >
> > We do need to add this to the general selftests Makefile, but this code is
> > fine, so have a:
> >
> > Reviewed-by: Lorenzo Stoakes <[e-mail]>
> >
> > Congratulations! :) and thanks for addressing the issues that were raised,
> > appreciate your efforts on this.
> >
> > Maybe you could send a fix patch? As it's such a small fix.
> >
> > Cheers, Lorenzo
> >
> >
> > > ---
> > >  .../mseal_system_mappings/.gitignore          |   2 +
> > >  .../selftests/mseal_system_mappings/Makefile  |   6 +
> > >  .../selftests/mseal_system_mappings/config    |   1 +
> > >  .../mseal_system_mappings/sysmap_is_sealed.c  | 113 ++++++++++++++++++
> >
> > Can you add this to tools/testing/selftests/Makefile? I _think_ adding:
> >
> > TARGETS += mm
> >
> > Should do it. Thanks!
> 
> Obviously I meant to say:
> 
> 	TARGETS += mseal_system_mappings
> 
> Doh! :)

Heh. Simultaneous emails! ;)

Lorenzo Stoakes March 3, 2025, 4:49 p.m. UTC | #5

On Mon, Mar 03, 2025 at 08:47:13AM -0800, Kees Cook wrote:
> On Mon, Mar 03, 2025 at 12:08:49PM +0000, Lorenzo Stoakes wrote:
> >
> > On Mon, Mar 03, 2025 at 05:09:21AM +0000, [e-mail] wrote:
> > > From: Jeff Xu <[e-mail]>
> > >
> > > Add sysmap_is_sealed.c to test system mappings are sealed.
> > >
> > > Note: CONFIG_MSEAL_SYSTEM_MAPPINGS must be set, as indicated in
> > > config file.
> > >
> > > Signed-off-by: Jeff Xu <[e-mail]>
> >
> > We do need to add this to the general selftests Makefile, but this code is
> > fine, so have a:
> >
> > Reviewed-by: Lorenzo Stoakes <[e-mail]>
> >
> > Congratulations! :) and thanks for addressing the issues that were raised,
> > appreciate your efforts on this.
> >
> > Maybe you could send a fix patch? As it's such a small fix.
> >
> > Cheers, Lorenzo
> >
> >
> > > ---
> > >  .../mseal_system_mappings/.gitignore          |   2 +
> > >  .../selftests/mseal_system_mappings/Makefile  |   6 +
> > >  .../selftests/mseal_system_mappings/config    |   1 +
> > >  .../mseal_system_mappings/sysmap_is_sealed.c  | 113 ++++++++++++++++++
> >
> > Can you add this to tools/testing/selftests/Makefile? I _think_ adding:
> >
> > TARGETS += mm
>
> You mean this, I think?
>
> TARGETS += mseal_system_mappings

Yeah just corrected my dumb mistake :) Indeed this is what I meant!

>
> (But while we're here, why is "mm" where it is in that Makefile?
> Everything else is alphabetical?)

Because mm is very special perhaps :P

>
> -Kees
>
> --
> Kees Cook

Kees Cook March 3, 2025, 5:01 p.m. UTC | #6

On Mon, Mar 03, 2025 at 05:09:21AM +0000, [e-mail] wrote:
> From: Jeff Xu <[e-mail]>
> 
> Add sysmap_is_sealed.c to test system mappings are sealed.
> 
> Note: CONFIG_MSEAL_SYSTEM_MAPPINGS must be set, as indicated in
> config file.
> 
> Signed-off-by: Jeff Xu <[e-mail]>
> ---
>  .../mseal_system_mappings/.gitignore          |   2 +
>  .../selftests/mseal_system_mappings/Makefile  |   6 +
>  .../selftests/mseal_system_mappings/config    |   1 +
>  .../mseal_system_mappings/sysmap_is_sealed.c  | 113 ++++++++++++++++++
>  4 files changed, 122 insertions(+)
>  create mode 100644 tools/testing/selftests/mseal_system_mappings/.gitignore
>  create mode 100644 tools/testing/selftests/mseal_system_mappings/Makefile
>  create mode 100644 tools/testing/selftests/mseal_system_mappings/config
>  create mode 100644 tools/testing/selftests/mseal_system_mappings/sysmap_is_sealed.c
> 
> diff --git a/tools/testing/selftests/mseal_system_mappings/.gitignore b/tools/testing/selftests/mseal_system_mappings/.gitignore
> new file mode 100644
> index 000000000000..319c497a595e
> --- /dev/null
> +++ b/tools/testing/selftests/mseal_system_mappings/.gitignore
> @@ -0,0 +1,2 @@
> +# SPDX-License-Identifier: GPL-2.0-only
> +sysmap_is_sealed
> diff --git a/tools/testing/selftests/mseal_system_mappings/Makefile b/tools/testing/selftests/mseal_system_mappings/Makefile
> new file mode 100644
> index 000000000000..2b4504e2f52f
> --- /dev/null
> +++ b/tools/testing/selftests/mseal_system_mappings/Makefile
> @@ -0,0 +1,6 @@
> +# SPDX-License-Identifier: GPL-2.0-only
> +CFLAGS += -std=c99 -pthread -Wall $(KHDR_INCLUDES)
> +
> +TEST_GEN_PROGS := sysmap_is_sealed
> +
> +include ../lib.mk
> diff --git a/tools/testing/selftests/mseal_system_mappings/config b/tools/testing/selftests/mseal_system_mappings/config
> new file mode 100644
> index 000000000000..675cb9f37b86
> --- /dev/null
> +++ b/tools/testing/selftests/mseal_system_mappings/config
> @@ -0,0 +1 @@
> +CONFIG_MSEAL_SYSTEM_MAPPINGS=y
> diff --git a/tools/testing/selftests/mseal_system_mappings/sysmap_is_sealed.c b/tools/testing/selftests/mseal_system_mappings/sysmap_is_sealed.c
> new file mode 100644
> index 000000000000..c1e93794a58b
> --- /dev/null
> +++ b/tools/testing/selftests/mseal_system_mappings/sysmap_is_sealed.c
> @@ -0,0 +1,113 @@
> +// SPDX-License-Identifier: GPL-2.0-only
> +/*
> + * test system mappings are sealed when
> + * KCONFIG_MSEAL_SYSTEM_MAPPINGS=y
> + */
> +
> +#define _GNU_SOURCE
> +#include <stdio.h>
> +#include <errno.h>
> +#include <unistd.h>
> +#include <string.h>
> +#include <stdbool.h>
> +
> +#include "../kselftest.h"
> +#include "../kselftest_harness.h"
> +
> +#define VDSO_NAME "[vdso]"
> +#define VVAR_NAME "[vvar]"
> +#define VVAR_VCLOCK_NAME "[vvar_vclock]"
> +#define UPROBES_NAME "[uprobes]"
> +#define SIGPAGE_NAME "[sigpage]"
> +#define VECTORS_NAME "[vectors]"

These are only ever used once, and it feels like having them spelled out
right in the variant definitions would be more readable, but I'm not
sure I feel strongly enough about it to say it should be changed.
They're available via "variant->name" as well, which makes it unlikely
the macros will be used anywhere in the future? Maybe you have plans for
them. :)

> +#define VMFLAGS "VmFlags:"

This one gets a strlen() on it, so it feels better to have a macro.

> +#define MSEAL_FLAGS "sl"
> +#define MAX_LINE_LEN 512
> +
> +bool has_mapping(char *name, FILE *maps)
> +{
> +	char line[MAX_LINE_LEN];
> +
> +	while (fgets(line, sizeof(line), maps)) {
> +		if (strstr(line, name))
> +			return true;
> +	}
> +
> +	return false;
> +}
> +
> +bool mapping_is_sealed(char *name, FILE *maps)
> +{
> +	char line[MAX_LINE_LEN];
> +
> +	while (fgets(line, sizeof(line), maps)) {
> +		if (!strncmp(line, VMFLAGS, strlen(VMFLAGS))) {
> +			if (strstr(line, MSEAL_FLAGS))
> +				return true;
> +
> +			return false;
> +		}
> +	}
> +
> +	return false;
> +}
> +
> +FIXTURE(basic) {
> +	FILE *maps;
> +};
> +
> +FIXTURE_SETUP(basic)
> +{
> +	self->maps = fopen("/proc/self/smaps", "r");
> +	if (!self->maps)
> +		SKIP(return, "Could not open /proc/self/smap, errno=%d",
> +			errno);

Good SKIP usage, though I wonder if not having /proc should be a full
blown failure?

> +};
> +
> +FIXTURE_TEARDOWN(basic)
> +{
> +	if (self->maps)
> +		fclose(self->maps);
> +};
> +
> +FIXTURE_VARIANT(basic)
> +{
> +	char *name;
> +};
> +
> +FIXTURE_VARIANT_ADD(basic, vdso) {
> +	.name = VDSO_NAME,
> +};
> +
> +FIXTURE_VARIANT_ADD(basic, vvar) {
> +	.name = VVAR_NAME,
> +};
> +
> +FIXTURE_VARIANT_ADD(basic, vvar_vclock) {
> +	.name = VVAR_VCLOCK_NAME,
> +};
> +
> +FIXTURE_VARIANT_ADD(basic, sigpage) {
> +	.name = SIGPAGE_NAME,
> +};
> +
> +FIXTURE_VARIANT_ADD(basic, vectors) {
> +	.name = VECTORS_NAME,
> +};
> +
> +FIXTURE_VARIANT_ADD(basic, uprobes) {
> +	.name = UPROBES_NAME,
> +};

I love seeing variants used in the test harness. :)

> +
> +TEST_F(basic, is_sealed)
> +{
> +	if (!has_mapping(variant->name, self->maps)) {
> +		SKIP(return, "could not found the mapping, %s",

typo nit: "find" instead of "found"

> +			variant->name);
> +	}
> +
> +	EXPECT_TRUE(mapping_is_sealed(variant->name, self->maps));
> +};

This is a good "positive" test, but I'd like to see a negative test
added as well. (This adds robustness against something going "all wrong"
or "all right", like imagine that someone adds a VmFlags string named
"slow", suddenly this test will always pass due to matching "sl". With
a negative test added, it will fail when it finds "sl" when it's not
expected.) For example, also check "[stack]" and "[heap]" and expect
them NOT to be sealed.

You could update the variant as:

FIXTURE_VARIANT(basic)
{
	char *name;
	bool sealed;
};

FIXTURE_VARIANT_ADD(basic, vdso) {
	.name = "[vdso]",
	.sealed = true,
};

FIXTURE_VARIANT_ADD(basic, stack) {
	.name = "[stack]",
	.sealed = false,
};

And then update the is_sealed test to:

	EXPECT_EQ(variant->sealed, mapping_is_sealed(variant->name, self->maps));

Jeff Xu March 3, 2025, 7:46 p.m. UTC | #7

On Mon, Mar 3, 2025 at 8:43 AM Lorenzo Stoakes
<[e-mail]> wrote:
>
> On Mon, Mar 03, 2025 at 12:08:49PM +0000, Lorenzo Stoakes wrote:
> >
> > On Mon, Mar 03, 2025 at 05:09:21AM +0000, [e-mail] wrote:
> > > From: Jeff Xu <[e-mail]>
> > >
> > > Add sysmap_is_sealed.c to test system mappings are sealed.
> > >
> > > Note: CONFIG_MSEAL_SYSTEM_MAPPINGS must be set, as indicated in
> > > config file.
> > >
> > > Signed-off-by: Jeff Xu <[e-mail]>
> >
> > We do need to add this to the general selftests Makefile, but this code is
> > fine, so have a:
> >
> > Reviewed-by: Lorenzo Stoakes <[e-mail]>
> >
> > Congratulations! :) and thanks for addressing the issues that were raised,
> > appreciate your efforts on this.
> >
> > Maybe you could send a fix patch? As it's such a small fix.
> >
> > Cheers, Lorenzo
> >
> >
> > > ---
> > >  .../mseal_system_mappings/.gitignore          |   2 +
> > >  .../selftests/mseal_system_mappings/Makefile  |   6 +
> > >  .../selftests/mseal_system_mappings/config    |   1 +
> > >  .../mseal_system_mappings/sysmap_is_sealed.c  | 113 ++++++++++++++++++
> >
> > Can you add this to tools/testing/selftests/Makefile? I _think_ adding:
> >
> > TARGETS += mm
> >
> > Should do it. Thanks!
>
> Obviously I meant to say:
>
>         TARGETS += mseal_system_mappings
>
> Doh! :)
>
Yes. I will add that in v9.

This new selftest requires [e-mail]'s approval.

As previously discussed [1], the KCONFIG: MSEAL_SYSTEM_MAPPINGS is
disabled by default. Hopefully, the selftest automation will be able
to detect and parse the "config" in the new
selftests/mseal_system_mappings/config at runtime.

Thanks
-Jeff
[1] https://lore.kernel.org/all/[e-mail]/

> >
> > >  4 files changed, 122 insertions(+)
> > >  create mode 100644 tools/testing/selftests/mseal_system_mappings/.gitignore
> > >  create mode 100644 tools/testing/selftests/mseal_system_mappings/Makefile
> > >  create mode 100644 tools/testing/selftests/mseal_system_mappings/config
> > >  create mode 100644 tools/testing/selftests/mseal_system_mappings/sysmap_is_sealed.c
> > >
> > > diff --git a/tools/testing/selftests/mseal_system_mappings/.gitignore b/tools/testing/selftests/mseal_system_mappings/.gitignore
> > > new file mode 100644
> > > index 000000000000..319c497a595e
> > > --- /dev/null
> > > +++ b/tools/testing/selftests/mseal_system_mappings/.gitignore
> > > @@ -0,0 +1,2 @@
> > > +# SPDX-License-Identifier: GPL-2.0-only
> > > +sysmap_is_sealed
> > > diff --git a/tools/testing/selftests/mseal_system_mappings/Makefile b/tools/testing/selftests/mseal_system_mappings/Makefile
> > > new file mode 100644
> > > index 000000000000..2b4504e2f52f
> > > --- /dev/null
> > > +++ b/tools/testing/selftests/mseal_system_mappings/Makefile
> > > @@ -0,0 +1,6 @@
> > > +# SPDX-License-Identifier: GPL-2.0-only
> > > +CFLAGS += -std=c99 -pthread -Wall $(KHDR_INCLUDES)
> > > +
> > > +TEST_GEN_PROGS := sysmap_is_sealed
> > > +
> > > +include ../lib.mk
> > > diff --git a/tools/testing/selftests/mseal_system_mappings/config b/tools/testing/selftests/mseal_system_mappings/config
> > > new file mode 100644
> > > index 000000000000..675cb9f37b86
> > > --- /dev/null
> > > +++ b/tools/testing/selftests/mseal_system_mappings/config
> > > @@ -0,0 +1 @@
> > > +CONFIG_MSEAL_SYSTEM_MAPPINGS=y
> > > diff --git a/tools/testing/selftests/mseal_system_mappings/sysmap_is_sealed.c b/tools/testing/selftests/mseal_system_mappings/sysmap_is_sealed.c
> > > new file mode 100644
> > > index 000000000000..c1e93794a58b
> > > --- /dev/null
> > > +++ b/tools/testing/selftests/mseal_system_mappings/sysmap_is_sealed.c
> > > @@ -0,0 +1,113 @@
> > > +// SPDX-License-Identifier: GPL-2.0-only
> > > +/*
> > > + * test system mappings are sealed when
> > > + * KCONFIG_MSEAL_SYSTEM_MAPPINGS=y
> > > + */
> > > +
> > > +#define _GNU_SOURCE
> > > +#include <stdio.h>
> > > +#include <errno.h>
> > > +#include <unistd.h>
> > > +#include <string.h>
> > > +#include <stdbool.h>
> > > +
> > > +#include "../kselftest.h"
> > > +#include "../kselftest_harness.h"
> > > +
> > > +#define VDSO_NAME "[vdso]"
> > > +#define VVAR_NAME "[vvar]"
> > > +#define VVAR_VCLOCK_NAME "[vvar_vclock]"
> > > +#define UPROBES_NAME "[uprobes]"
> > > +#define SIGPAGE_NAME "[sigpage]"
> > > +#define VECTORS_NAME "[vectors]"
> > > +
> > > +#define VMFLAGS "VmFlags:"
> > > +#define MSEAL_FLAGS "sl"
> > > +#define MAX_LINE_LEN 512
> > > +
> > > +bool has_mapping(char *name, FILE *maps)
> > > +{
> > > +   char line[MAX_LINE_LEN];
> > > +
> > > +   while (fgets(line, sizeof(line), maps)) {
> > > +           if (strstr(line, name))
> > > +                   return true;
> > > +   }
> > > +
> > > +   return false;
> > > +}
> > > +
> > > +bool mapping_is_sealed(char *name, FILE *maps)
> > > +{
> > > +   char line[MAX_LINE_LEN];
> > > +
> > > +   while (fgets(line, sizeof(line), maps)) {
> > > +           if (!strncmp(line, VMFLAGS, strlen(VMFLAGS))) {
> > > +                   if (strstr(line, MSEAL_FLAGS))
> > > +                           return true;
> > > +
> > > +                   return false;
> > > +           }
> > > +   }
> > > +
> > > +   return false;
> > > +}
> > > +
> > > +FIXTURE(basic) {
> > > +   FILE *maps;
> > > +};
> > > +
> > > +FIXTURE_SETUP(basic)
> > > +{
> > > +   self->maps = fopen("/proc/self/smaps", "r");
> > > +   if (!self->maps)
> > > +           SKIP(return, "Could not open /proc/self/smap, errno=%d",
> > > +                   errno);
> > > +};
> > > +
> > > +FIXTURE_TEARDOWN(basic)
> > > +{
> > > +   if (self->maps)
> > > +           fclose(self->maps);
> > > +};
> > > +
> > > +FIXTURE_VARIANT(basic)
> > > +{
> > > +   char *name;
> > > +};
> > > +
> > > +FIXTURE_VARIANT_ADD(basic, vdso) {
> > > +   .name = VDSO_NAME,
> > > +};
> > > +
> > > +FIXTURE_VARIANT_ADD(basic, vvar) {
> > > +   .name = VVAR_NAME,
> > > +};
> > > +
> > > +FIXTURE_VARIANT_ADD(basic, vvar_vclock) {
> > > +   .name = VVAR_VCLOCK_NAME,
> > > +};
> > > +
> > > +FIXTURE_VARIANT_ADD(basic, sigpage) {
> > > +   .name = SIGPAGE_NAME,
> > > +};
> > > +
> > > +FIXTURE_VARIANT_ADD(basic, vectors) {
> > > +   .name = VECTORS_NAME,
> > > +};
> > > +
> > > +FIXTURE_VARIANT_ADD(basic, uprobes) {
> > > +   .name = UPROBES_NAME,
> > > +};
> > > +
> > > +TEST_F(basic, is_sealed)
> > > +{
> > > +   if (!has_mapping(variant->name, self->maps)) {
> > > +           SKIP(return, "could not found the mapping, %s",
> > > +                   variant->name);
> > > +   }
> > > +
> > > +   EXPECT_TRUE(mapping_is_sealed(variant->name, self->maps));
> > > +};
> > > +
> > > +TEST_HARNESS_MAIN
> > > --
> > > 2.48.1.711.g2feabab25a-goog
> > >

Jeff Xu March 3, 2025, 8:20 p.m. UTC | #8

On Mon, Mar 3, 2025 at 9:01 AM Kees Cook <[e-mail]> wrote:
>
> On Mon, Mar 03, 2025 at 05:09:21AM +0000, [e-mail] wrote:
> > From: Jeff Xu <[e-mail]>
> >
> > Add sysmap_is_sealed.c to test system mappings are sealed.
> >
> > Note: CONFIG_MSEAL_SYSTEM_MAPPINGS must be set, as indicated in
> > config file.
> >
> > Signed-off-by: Jeff Xu <[e-mail]>
> > ---
> >  .../mseal_system_mappings/.gitignore          |   2 +
> >  .../selftests/mseal_system_mappings/Makefile  |   6 +
> >  .../selftests/mseal_system_mappings/config    |   1 +
> >  .../mseal_system_mappings/sysmap_is_sealed.c  | 113 ++++++++++++++++++
> >  4 files changed, 122 insertions(+)
> >  create mode 100644 tools/testing/selftests/mseal_system_mappings/.gitignore
> >  create mode 100644 tools/testing/selftests/mseal_system_mappings/Makefile
> >  create mode 100644 tools/testing/selftests/mseal_system_mappings/config
> >  create mode 100644 tools/testing/selftests/mseal_system_mappings/sysmap_is_sealed.c
> >
> > diff --git a/tools/testing/selftests/mseal_system_mappings/.gitignore b/tools/testing/selftests/mseal_system_mappings/.gitignore
> > new file mode 100644
> > index 000000000000..319c497a595e
> > --- /dev/null
> > +++ b/tools/testing/selftests/mseal_system_mappings/.gitignore
> > @@ -0,0 +1,2 @@
> > +# SPDX-License-Identifier: GPL-2.0-only
> > +sysmap_is_sealed
> > diff --git a/tools/testing/selftests/mseal_system_mappings/Makefile b/tools/testing/selftests/mseal_system_mappings/Makefile
> > new file mode 100644
> > index 000000000000..2b4504e2f52f
> > --- /dev/null
> > +++ b/tools/testing/selftests/mseal_system_mappings/Makefile
> > @@ -0,0 +1,6 @@
> > +# SPDX-License-Identifier: GPL-2.0-only
> > +CFLAGS += -std=c99 -pthread -Wall $(KHDR_INCLUDES)
> > +
> > +TEST_GEN_PROGS := sysmap_is_sealed
> > +
> > +include ../lib.mk
> > diff --git a/tools/testing/selftests/mseal_system_mappings/config b/tools/testing/selftests/mseal_system_mappings/config
> > new file mode 100644
> > index 000000000000..675cb9f37b86
> > --- /dev/null
> > +++ b/tools/testing/selftests/mseal_system_mappings/config
> > @@ -0,0 +1 @@
> > +CONFIG_MSEAL_SYSTEM_MAPPINGS=y
> > diff --git a/tools/testing/selftests/mseal_system_mappings/sysmap_is_sealed.c b/tools/testing/selftests/mseal_system_mappings/sysmap_is_sealed.c
> > new file mode 100644
> > index 000000000000..c1e93794a58b
> > --- /dev/null
> > +++ b/tools/testing/selftests/mseal_system_mappings/sysmap_is_sealed.c
> > @@ -0,0 +1,113 @@
> > +// SPDX-License-Identifier: GPL-2.0-only
> > +/*
> > + * test system mappings are sealed when
> > + * KCONFIG_MSEAL_SYSTEM_MAPPINGS=y
> > + */
> > +
> > +#define _GNU_SOURCE
> > +#include <stdio.h>
> > +#include <errno.h>
> > +#include <unistd.h>
> > +#include <string.h>
> > +#include <stdbool.h>
> > +
> > +#include "../kselftest.h"
> > +#include "../kselftest_harness.h"
> > +
> > +#define VDSO_NAME "[vdso]"
> > +#define VVAR_NAME "[vvar]"
> > +#define VVAR_VCLOCK_NAME "[vvar_vclock]"
> > +#define UPROBES_NAME "[uprobes]"
> > +#define SIGPAGE_NAME "[sigpage]"
> > +#define VECTORS_NAME "[vectors]"
>
> These are only ever used once, and it feels like having them spelled out
> right in the variant definitions would be more readable, but I'm not
> sure I feel strongly enough about it to say it should be changed.
> They're available via "variant->name" as well, which makes it unlikely
> the macros will be used anywhere in the future? Maybe you have plans for
> them. :)
No plan for reuse them in other code, will move to Variant in v9.

>
> > +#define VMFLAGS "VmFlags:"
>
> This one gets a strlen() on it, so it feels better to have a macro.
>
Ok, thanks for the reasoning.

> > +#define MSEAL_FLAGS "sl"
> > +#define MAX_LINE_LEN 512
> > +
> > +bool has_mapping(char *name, FILE *maps)
> > +{
> > +     char line[MAX_LINE_LEN];
> > +
> > +     while (fgets(line, sizeof(line), maps)) {
> > +             if (strstr(line, name))
> > +                     return true;
> > +     }
> > +
> > +     return false;
> > +}
> > +
> > +bool mapping_is_sealed(char *name, FILE *maps)
> > +{
> > +     char line[MAX_LINE_LEN];
> > +
> > +     while (fgets(line, sizeof(line), maps)) {
> > +             if (!strncmp(line, VMFLAGS, strlen(VMFLAGS))) {
> > +                     if (strstr(line, MSEAL_FLAGS))
> > +                             return true;
> > +
> > +                     return false;
> > +             }
> > +     }
> > +
> > +     return false;
> > +}
> > +
> > +FIXTURE(basic) {
> > +     FILE *maps;
> > +};
> > +
> > +FIXTURE_SETUP(basic)
> > +{
> > +     self->maps = fopen("/proc/self/smaps", "r");
> > +     if (!self->maps)
> > +             SKIP(return, "Could not open /proc/self/smap, errno=%d",
> > +                     errno);
>
> Good SKIP usage, though I wonder if not having /proc should be a full
> blown failure?
>
Usually, the failure is used to report failures directly related to
what this code is testing. If /proc is unavailable, it's an
environment setup issue, which is more fitting for SKIP, otherwise, we
wouldn't need "SKIP" - we'd just report all environment requirements
checked as failures.

Unless you mean that "/proc" is always available and can never be
unavailable in any selftest environment? Then, I can change to use the
failure reporting.

> > +};
> > +
> > +FIXTURE_TEARDOWN(basic)
> > +{
> > +     if (self->maps)
> > +             fclose(self->maps);
> > +};
> > +
> > +FIXTURE_VARIANT(basic)
> > +{
> > +     char *name;
> > +};
> > +
> > +FIXTURE_VARIANT_ADD(basic, vdso) {
> > +     .name = VDSO_NAME,
> > +};
> > +
> > +FIXTURE_VARIANT_ADD(basic, vvar) {
> > +     .name = VVAR_NAME,
> > +};
> > +
> > +FIXTURE_VARIANT_ADD(basic, vvar_vclock) {
> > +     .name = VVAR_VCLOCK_NAME,
> > +};
> > +
> > +FIXTURE_VARIANT_ADD(basic, sigpage) {
> > +     .name = SIGPAGE_NAME,
> > +};
> > +
> > +FIXTURE_VARIANT_ADD(basic, vectors) {
> > +     .name = VECTORS_NAME,
> > +};
> > +
> > +FIXTURE_VARIANT_ADD(basic, uprobes) {
> > +     .name = UPROBES_NAME,
> > +};
>
> I love seeing variants used in the test harness. :)
>
Ya, copied from landlock selftest :-)

> > +
> > +TEST_F(basic, is_sealed)
> > +{
> > +     if (!has_mapping(variant->name, self->maps)) {
> > +             SKIP(return, "could not found the mapping, %s",
>
> typo nit: "find" instead of "found"
>
> > +                     variant->name);
> > +     }
> > +
> > +     EXPECT_TRUE(mapping_is_sealed(variant->name, self->maps));
> > +};
>
> This is a good "positive" test, but I'd like to see a negative test
> added as well. (This adds robustness against something going "all wrong"
> or "all right", like imagine that someone adds a VmFlags string named
> "slow", suddenly this test will always pass due to matching "sl". With
> a negative test added, it will fail when it finds "sl" when it's not
> expected.) For example, also check "[stack]" and "[heap]" and expect
> them NOT to be sealed.
>
> You could update the variant as:
>
> FIXTURE_VARIANT(basic)
> {
>         char *name;
>         bool sealed;
> };
>
> FIXTURE_VARIANT_ADD(basic, vdso) {
>         .name = "[vdso]",
>         .sealed = true,
> };
>
> FIXTURE_VARIANT_ADD(basic, stack) {
>         .name = "[stack]",
>         .sealed = false,
> };
>
> And then update the is_sealed test to:
>
>         EXPECT_EQ(variant->sealed, mapping_is_sealed(variant->name, self->maps));
>
The challenge is that I'm unsure how to detect
"CONFIG_MSEAL_SYSTEM_MAPPINGS" from selftest runtime.

Without that, the test can't reliably set the "sealed" flag. Lorenzo
suggested parsing /proc/config.gz, but I responded, "None of the
existing selftests use this pattern, and I'm not sure /proc/config.gz
is enabled in the default kernel config." [1].

To work around this, in this version, I add
selftests/mseal_system_mappings/config to indicate
CONFIG_MSEAL_SYSTEM_MAPPINGS=y is a mandatory requirement for running
this test. Therefore, this selftest assumes the ".sealed" is always
true, i.e. no negative test.

I'm looking to Linux Kernel Self-Test (LKST) and Shuah Khan for
guidance/suggestion on handling different kernel config variants
within selftest.

Thanks
-Jeff
[1] https://lore.kernel.org/all/[e-mail]/

> --
> Kees Cook

Jeff Xu March 4, 2025, 8:53 p.m. UTC | #9

On Mon, Mar 3, 2025 at 12:20 PM Jeff Xu <[e-mail]> wrote:
>
> On Mon, Mar 3, 2025 at 9:01 AM Kees Cook <[e-mail]> wrote:
> >
> > On Mon, Mar 03, 2025 at 05:09:21AM +0000, [e-mail] wrote:
> > > From: Jeff Xu <[e-mail]>
> > >
> > > Add sysmap_is_sealed.c to test system mappings are sealed.
> > >
> > > Note: CONFIG_MSEAL_SYSTEM_MAPPINGS must be set, as indicated in
> > > config file.
> > >
> > > Signed-off-by: Jeff Xu <[e-mail]>
> > > ---
> > >  .../mseal_system_mappings/.gitignore          |   2 +
> > >  .../selftests/mseal_system_mappings/Makefile  |   6 +
> > >  .../selftests/mseal_system_mappings/config    |   1 +
> > >  .../mseal_system_mappings/sysmap_is_sealed.c  | 113 ++++++++++++++++++
> > >  4 files changed, 122 insertions(+)
> > >  create mode 100644 tools/testing/selftests/mseal_system_mappings/.gitignore
> > >  create mode 100644 tools/testing/selftests/mseal_system_mappings/Makefile
> > >  create mode 100644 tools/testing/selftests/mseal_system_mappings/config
> > >  create mode 100644 tools/testing/selftests/mseal_system_mappings/sysmap_is_sealed.c
> > >
> > > diff --git a/tools/testing/selftests/mseal_system_mappings/.gitignore b/tools/testing/selftests/mseal_system_mappings/.gitignore
> > > new file mode 100644
> > > index 000000000000..319c497a595e
> > > --- /dev/null
> > > +++ b/tools/testing/selftests/mseal_system_mappings/.gitignore
> > > @@ -0,0 +1,2 @@
> > > +# SPDX-License-Identifier: GPL-2.0-only
> > > +sysmap_is_sealed
> > > diff --git a/tools/testing/selftests/mseal_system_mappings/Makefile b/tools/testing/selftests/mseal_system_mappings/Makefile
> > > new file mode 100644
> > > index 000000000000..2b4504e2f52f
> > > --- /dev/null
> > > +++ b/tools/testing/selftests/mseal_system_mappings/Makefile
> > > @@ -0,0 +1,6 @@
> > > +# SPDX-License-Identifier: GPL-2.0-only
> > > +CFLAGS += -std=c99 -pthread -Wall $(KHDR_INCLUDES)
> > > +
> > > +TEST_GEN_PROGS := sysmap_is_sealed
> > > +
> > > +include ../lib.mk
> > > diff --git a/tools/testing/selftests/mseal_system_mappings/config b/tools/testing/selftests/mseal_system_mappings/config
> > > new file mode 100644
> > > index 000000000000..675cb9f37b86
> > > --- /dev/null
> > > +++ b/tools/testing/selftests/mseal_system_mappings/config
> > > @@ -0,0 +1 @@
> > > +CONFIG_MSEAL_SYSTEM_MAPPINGS=y
> > > diff --git a/tools/testing/selftests/mseal_system_mappings/sysmap_is_sealed.c b/tools/testing/selftests/mseal_system_mappings/sysmap_is_sealed.c
> > > new file mode 100644
> > > index 000000000000..c1e93794a58b
> > > --- /dev/null
> > > +++ b/tools/testing/selftests/mseal_system_mappings/sysmap_is_sealed.c
> > > @@ -0,0 +1,113 @@
> > > +// SPDX-License-Identifier: GPL-2.0-only
> > > +/*
> > > + * test system mappings are sealed when
> > > + * KCONFIG_MSEAL_SYSTEM_MAPPINGS=y
> > > + */
> > > +
> > > +#define _GNU_SOURCE
> > > +#include <stdio.h>
> > > +#include <errno.h>
> > > +#include <unistd.h>
> > > +#include <string.h>
> > > +#include <stdbool.h>
> > > +
> > > +#include "../kselftest.h"
> > > +#include "../kselftest_harness.h"
> > > +
> > > +#define VDSO_NAME "[vdso]"
> > > +#define VVAR_NAME "[vvar]"
> > > +#define VVAR_VCLOCK_NAME "[vvar_vclock]"
> > > +#define UPROBES_NAME "[uprobes]"
> > > +#define SIGPAGE_NAME "[sigpage]"
> > > +#define VECTORS_NAME "[vectors]"
> >
> > These are only ever used once, and it feels like having them spelled out
> > right in the variant definitions would be more readable, but I'm not
> > sure I feel strongly enough about it to say it should be changed.
> > They're available via "variant->name" as well, which makes it unlikely
> > the macros will be used anywhere in the future? Maybe you have plans for
> > them. :)
> No plan for reuse them in other code, will move to Variant in v9.
>
> >
> > > +#define VMFLAGS "VmFlags:"
> >
> > This one gets a strlen() on it, so it feels better to have a macro.
> >
> Ok, thanks for the reasoning.
>
> > > +#define MSEAL_FLAGS "sl"
> > > +#define MAX_LINE_LEN 512
> > > +
> > > +bool has_mapping(char *name, FILE *maps)
> > > +{
> > > +     char line[MAX_LINE_LEN];
> > > +
> > > +     while (fgets(line, sizeof(line), maps)) {
> > > +             if (strstr(line, name))
> > > +                     return true;
> > > +     }
> > > +
> > > +     return false;
> > > +}
> > > +
> > > +bool mapping_is_sealed(char *name, FILE *maps)
> > > +{
> > > +     char line[MAX_LINE_LEN];
> > > +
> > > +     while (fgets(line, sizeof(line), maps)) {
> > > +             if (!strncmp(line, VMFLAGS, strlen(VMFLAGS))) {
> > > +                     if (strstr(line, MSEAL_FLAGS))
> > > +                             return true;
> > > +
> > > +                     return false;
> > > +             }
> > > +     }
> > > +
> > > +     return false;
> > > +}
> > > +
> > > +FIXTURE(basic) {
> > > +     FILE *maps;
> > > +};
> > > +
> > > +FIXTURE_SETUP(basic)
> > > +{
> > > +     self->maps = fopen("/proc/self/smaps", "r");
> > > +     if (!self->maps)
> > > +             SKIP(return, "Could not open /proc/self/smap, errno=%d",
> > > +                     errno);
> >
> > Good SKIP usage, though I wonder if not having /proc should be a full
> > blown failure?
> >
> Usually, the failure is used to report failures directly related to
> what this code is testing. If /proc is unavailable, it's an
> environment setup issue, which is more fitting for SKIP, otherwise, we
> wouldn't need "SKIP" - we'd just report all environment requirements
> checked as failures.
>
> Unless you mean that "/proc" is always available and can never be
> unavailable in any selftest environment? Then, I can change to use the
> failure reporting.
>
> > > +};
> > > +
> > > +FIXTURE_TEARDOWN(basic)
> > > +{
> > > +     if (self->maps)
> > > +             fclose(self->maps);
> > > +};
> > > +
> > > +FIXTURE_VARIANT(basic)
> > > +{
> > > +     char *name;
> > > +};
> > > +
> > > +FIXTURE_VARIANT_ADD(basic, vdso) {
> > > +     .name = VDSO_NAME,
> > > +};
> > > +
> > > +FIXTURE_VARIANT_ADD(basic, vvar) {
> > > +     .name = VVAR_NAME,
> > > +};
> > > +
> > > +FIXTURE_VARIANT_ADD(basic, vvar_vclock) {
> > > +     .name = VVAR_VCLOCK_NAME,
> > > +};
> > > +
> > > +FIXTURE_VARIANT_ADD(basic, sigpage) {
> > > +     .name = SIGPAGE_NAME,
> > > +};
> > > +
> > > +FIXTURE_VARIANT_ADD(basic, vectors) {
> > > +     .name = VECTORS_NAME,
> > > +};
> > > +
> > > +FIXTURE_VARIANT_ADD(basic, uprobes) {
> > > +     .name = UPROBES_NAME,
> > > +};
> >
> > I love seeing variants used in the test harness. :)
> >
> Ya, copied from landlock selftest :-)
>
> > > +
> > > +TEST_F(basic, is_sealed)
> > > +{
> > > +     if (!has_mapping(variant->name, self->maps)) {
> > > +             SKIP(return, "could not found the mapping, %s",
> >
> > typo nit: "find" instead of "found"
> >
> > > +                     variant->name);
> > > +     }
> > > +
> > > +     EXPECT_TRUE(mapping_is_sealed(variant->name, self->maps));
> > > +};
> >
> > This is a good "positive" test, but I'd like to see a negative test
> > added as well. (This adds robustness against something going "all wrong"
> > or "all right", like imagine that someone adds a VmFlags string named
> > "slow", suddenly this test will always pass due to matching "sl". With
> > a negative test added, it will fail when it finds "sl" when it's not
> > expected.) For example, also check "[stack]" and "[heap]" and expect
> > them NOT to be sealed.
> >
> > You could update the variant as:
> >
> > FIXTURE_VARIANT(basic)
> > {
> >         char *name;
> >         bool sealed;
> > };
> >
> > FIXTURE_VARIANT_ADD(basic, vdso) {
> >         .name = "[vdso]",
> >         .sealed = true,
> > };
> >
> > FIXTURE_VARIANT_ADD(basic, stack) {
> >         .name = "[stack]",
> >         .sealed = false,
> > };
> >
> > And then update the is_sealed test to:
> >
> >         EXPECT_EQ(variant->sealed, mapping_is_sealed(variant->name, self->maps));
> >
> The challenge is that I'm unsure how to detect
> "CONFIG_MSEAL_SYSTEM_MAPPINGS" from selftest runtime.
>
> Without that, the test can't reliably set the "sealed" flag. Lorenzo
> suggested parsing /proc/config.gz, but I responded, "None of the
> existing selftests use this pattern, and I'm not sure /proc/config.gz
> is enabled in the default kernel config." [1].
>
> To work around this, in this version, I add
> selftests/mseal_system_mappings/config to indicate
> CONFIG_MSEAL_SYSTEM_MAPPINGS=y is a mandatory requirement for running
> this test. Therefore, this selftest assumes the ".sealed" is always
> true, i.e. no negative test.
>
> I'm looking to Linux Kernel Self-Test (LKST) and Shuah Khan for
> guidance/suggestion on handling different kernel config variants
> within selftest.
>
After connecting with Kees offline, it seems I misunderstood. We need
to add a check for heap so the test can use it as a negative test.
I'll add that in V9.

We can postpone the discussion of detecting KCONFIG during the
selftest runtime, which isn't necessary for this selftest. Instead, it
can rely on the added config file.

I will start working on V9.

Thanks
-Jeff



> Thanks
> -Jeff
> [1] https://lore.kernel.org/all/[e-mail]/
>
> > --
> > Kees Cook

Patch

diff --git a/tools/testing/selftests/mseal_system_mappings/.gitignore b/tools/testing/selftests/mseal_system_mappings/.gitignore
new file mode 100644
index 000000000000..319c497a595e
--- /dev/null
+++ b/tools/testing/selftests/mseal_system_mappings/.gitignore
@@ -0,0 +1,2 @@ 
+# SPDX-License-Identifier: GPL-2.0-only
+sysmap_is_sealed
diff --git a/tools/testing/selftests/mseal_system_mappings/Makefile b/tools/testing/selftests/mseal_system_mappings/Makefile
new file mode 100644
index 000000000000..2b4504e2f52f
--- /dev/null
+++ b/tools/testing/selftests/mseal_system_mappings/Makefile
@@ -0,0 +1,6 @@ 
+# SPDX-License-Identifier: GPL-2.0-only
+CFLAGS += -std=c99 -pthread -Wall $(KHDR_INCLUDES)
+
+TEST_GEN_PROGS := sysmap_is_sealed
+
+include ../lib.mk
diff --git a/tools/testing/selftests/mseal_system_mappings/config b/tools/testing/selftests/mseal_system_mappings/config
new file mode 100644
index 000000000000..675cb9f37b86
--- /dev/null
+++ b/tools/testing/selftests/mseal_system_mappings/config
@@ -0,0 +1 @@ 
+CONFIG_MSEAL_SYSTEM_MAPPINGS=y
diff --git a/tools/testing/selftests/mseal_system_mappings/sysmap_is_sealed.c b/tools/testing/selftests/mseal_system_mappings/sysmap_is_sealed.c
new file mode 100644
index 000000000000..c1e93794a58b
--- /dev/null
+++ b/tools/testing/selftests/mseal_system_mappings/sysmap_is_sealed.c
@@ -0,0 +1,113 @@ 
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * test system mappings are sealed when
+ * KCONFIG_MSEAL_SYSTEM_MAPPINGS=y
+ */
+
+#define _GNU_SOURCE
+#include <stdio.h>
+#include <errno.h>
+#include <unistd.h>
+#include <string.h>
+#include <stdbool.h>
+
+#include "../kselftest.h"
+#include "../kselftest_harness.h"
+
+#define VDSO_NAME "[vdso]"
+#define VVAR_NAME "[vvar]"
+#define VVAR_VCLOCK_NAME "[vvar_vclock]"
+#define UPROBES_NAME "[uprobes]"
+#define SIGPAGE_NAME "[sigpage]"
+#define VECTORS_NAME "[vectors]"
+
+#define VMFLAGS "VmFlags:"
+#define MSEAL_FLAGS "sl"
+#define MAX_LINE_LEN 512
+
+bool has_mapping(char *name, FILE *maps)
+{
+	char line[MAX_LINE_LEN];
+
+	while (fgets(line, sizeof(line), maps)) {
+		if (strstr(line, name))
+			return true;
+	}
+
+	return false;
+}
+
+bool mapping_is_sealed(char *name, FILE *maps)
+{
+	char line[MAX_LINE_LEN];
+
+	while (fgets(line, sizeof(line), maps)) {
+		if (!strncmp(line, VMFLAGS, strlen(VMFLAGS))) {
+			if (strstr(line, MSEAL_FLAGS))
+				return true;
+
+			return false;
+		}
+	}
+
+	return false;
+}
+
+FIXTURE(basic) {
+	FILE *maps;
+};
+
+FIXTURE_SETUP(basic)
+{
+	self->maps = fopen("/proc/self/smaps", "r");
+	if (!self->maps)
+		SKIP(return, "Could not open /proc/self/smap, errno=%d",
+			errno);
+};
+
+FIXTURE_TEARDOWN(basic)
+{
+	if (self->maps)
+		fclose(self->maps);
+};
+
+FIXTURE_VARIANT(basic)
+{
+	char *name;
+};
+
+FIXTURE_VARIANT_ADD(basic, vdso) {
+	.name = VDSO_NAME,
+};
+
+FIXTURE_VARIANT_ADD(basic, vvar) {
+	.name = VVAR_NAME,
+};
+
+FIXTURE_VARIANT_ADD(basic, vvar_vclock) {
+	.name = VVAR_VCLOCK_NAME,
+};
+
+FIXTURE_VARIANT_ADD(basic, sigpage) {
+	.name = SIGPAGE_NAME,
+};
+
+FIXTURE_VARIANT_ADD(basic, vectors) {
+	.name = VECTORS_NAME,
+};
+
+FIXTURE_VARIANT_ADD(basic, uprobes) {
+	.name = UPROBES_NAME,
+};
+
+TEST_F(basic, is_sealed)
+{
+	if (!has_mapping(variant->name, self->maps)) {
+		SKIP(return, "could not found the mapping, %s",
+			variant->name);
+	}
+
+	EXPECT_TRUE(mapping_is_sealed(variant->name, self->maps));
+};
+
+TEST_HARNESS_MAIN