| Message ID | 20240304150709.30260-3-andrea.cervesato@suse.de |
|---|---|
| State | Superseded |
| Headers | show |
| Series | SysV IPC bug reproducer | expand |
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, > +};
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); +} + +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, +};