[LTP] [PATCH v3 2/2] Add shmat04 SysV IPC bug reproducer

[Andrea Cervesato]

Hi,

On 3/4/24 16:07, Andrea Cervesato wrote:
> From: Andrea Cervesato <andrea.cervesato@suse.com>
>
> This is an LTP port of a SysV bug reproducer provided by Michal Hocko.
>
> When debugging issues with a workload using SysV shmem, Michal Hocko has
> come up with a reproducer that shows how a series of mprotect()
> operations can result in an elevated shm_nattch and thus leak of the
> resource.
>
> Signed-off-by: Andrea Cervesato <andrea.cervesato@suse.com>
> ---
> result of TST_EXP_EQ_LU() as test result
>
>   runtest/syscalls                              |   1 +
>   runtest/syscalls-ipc                          |   1 +
>   .../kernel/syscalls/ipc/shmat/.gitignore      |   1 +
>   testcases/kernel/syscalls/ipc/shmat/shmat04.c | 112 ++++++++++++++++++
>   4 files changed, 115 insertions(+)
>   create mode 100644 testcases/kernel/syscalls/ipc/shmat/shmat04.c
>
> diff --git a/runtest/syscalls b/runtest/syscalls
> index 7794f1465..cc0144379 100644
> --- a/runtest/syscalls
> +++ b/runtest/syscalls
> @@ -1445,6 +1445,7 @@ setxattr03 setxattr03
>   shmat01 shmat01
>   shmat02 shmat02
>   shmat03 shmat03
> +shmat04 shmat04
>   
>   shmctl01 shmctl01
>   shmctl02 shmctl02
> diff --git a/runtest/syscalls-ipc b/runtest/syscalls-ipc
> index df41140a7..9860394de 100644
> --- a/runtest/syscalls-ipc
> +++ b/runtest/syscalls-ipc
> @@ -49,6 +49,7 @@ semop03 semop03
>   
>   shmat01 shmat01
>   shmat02 shmat02
> +shmat04 shmat04
>   
>   shmctl01 shmctl01
>   shmctl02 shmctl02
> diff --git a/testcases/kernel/syscalls/ipc/shmat/.gitignore b/testcases/kernel/syscalls/ipc/shmat/.gitignore
> index 2b972f8f2..5600b2742 100644
> --- a/testcases/kernel/syscalls/ipc/shmat/.gitignore
> +++ b/testcases/kernel/syscalls/ipc/shmat/.gitignore
> @@ -1,3 +1,4 @@
>   /shmat01
>   /shmat02
>   /shmat03
> +/shmat04
> diff --git a/testcases/kernel/syscalls/ipc/shmat/shmat04.c b/testcases/kernel/syscalls/ipc/shmat/shmat04.c
> new file mode 100644
> index 000000000..caadee961
> --- /dev/null
> +++ b/testcases/kernel/syscalls/ipc/shmat/shmat04.c
> @@ -0,0 +1,112 @@
> +// SPDX-License-Identifier: GPL-2.0-or-later
> +/*
> + * Copyright (C) 2024 SUSE LLC
> + * Author: Michal Hocko <mhocko@suse.com>
> + * LTP port: Andrea Cervesato <andrea.cervesato@suse.com>
> + */
> +
> +/*\
> + * [Description]
> + *
> + * When debugging issues with a workload using SysV shmem, Michal Hocko has
> + * come up with a reproducer that shows how a series of mprotect()
> + * operations can result in an elevated shm_nattch and thus leak of the
> + * resource.
> + *
> + * The problem is caused by wrong assumptions in vma_merge() commit
> + * 714965ca8252 ("mm/mmap: start distinguishing if vma can be removed in
> + * mergeability test"). The shmem vmas have a vma_ops->close callback
> + * that decrements shm_nattch, and we remove the vma without calling it.
> + *
> + * Patch: https://lore.kernel.org/all/20240222215930.14637-2-vbabka@suse.cz/
> + */
> +
> +#include "tst_test.h"
> +#include "tst_safe_sysv_ipc.h"
> +#include "libnewipc.h"
> +
> +static int segment_id = -1;
> +static int key_id;
> +static int page_size;
> +static size_t segment_size;
> +
> +static void change_access(void *addr, int size, int prot)
> +{
> +	switch (prot) {
> +	case PROT_NONE:
> +		tst_res(TINFO, "Disable memory access. addr: %p - size: %d",
> +			addr, size);
> +		break;
> +	case PROT_WRITE:
> +		tst_res(TINFO, "Enable write memory access. addr: %p - size: %d",
> +			addr, size);
> +		break;
> +	default:
> +		tst_res(TINFO, "Change memory access. addr: %p - size: %d",
> +			addr, size);
> +		break;
> +	}
> +
> +	SAFE_MPROTECT(addr, size, prot);
> +}
There's an error here. I will fix on v4
> +
> +static void run(void)
> +{
> +	struct shmid_ds shmid_ds;
> +	void *sh_mem;
> +
> +	segment_id = SAFE_SHMGET(
> +		key_id,
> +		segment_size,
> +		IPC_CREAT | IPC_EXCL | 0600);
> +
> +	sh_mem = SAFE_SHMAT(segment_id, NULL, 0);
> +
> +	tst_res(TINFO, "Attached at %p. key: %d - size: %lu",
> +		sh_mem, segment_id, segment_size);
> +
> +	SAFE_SHMCTL(segment_id, IPC_STAT, &shmid_ds);
> +
> +	tst_res(TINFO, "Number of attaches: %lu", shmid_ds.shm_nattch);
> +
> +	change_access(sh_mem + page_size, page_size, PROT_NONE);
> +	change_access(sh_mem, 2 * page_size, PROT_WRITE);
> +
> +	SAFE_SHMCTL(segment_id, IPC_STAT, &shmid_ds);
> +
> +	tst_res(TINFO, "Number of attaches: %lu", shmid_ds.shm_nattch);
> +	tst_res(TINFO, "Delete attached memory");
> +
> +	SAFE_SHMDT(sh_mem);
> +	SAFE_SHMCTL(segment_id, IPC_STAT, &shmid_ds);
> +
> +	tst_res(TINFO, "Number of attaches: %lu", shmid_ds.shm_nattch);
> +
> +	SAFE_SHMCTL(segment_id, IPC_RMID, NULL);
> +	segment_id = -1;
> +
> +	TST_EXP_EQ_LU(shmid_ds.shm_nattch, 0);
> +}
> +
> +static void setup(void)
> +{
> +	key_id = GETIPCKEY();
> +	page_size = getpagesize();
> +
> +	tst_res(TINFO, "Key id: %d", key_id);
> +	tst_res(TINFO, "Page size: %d", page_size);
> +
> +	segment_size = 3 * page_size;
> +}
> +
> +static void cleanup(void)
> +{
> +	if (segment_id != -1)
> +		SAFE_SHMCTL(segment_id, IPC_RMID, NULL);
> +}
> +
> +static struct tst_test test = {
> +	.test_all = run,
> +	.setup = setup,
> +	.cleanup = cleanup,
> +};