| Message ID | 20240226113510.29937-1-andrea.cervesato@suse.de |
|---|---|
| State | Superseded |
| Headers | show |
| Series | [v1] Add shmat04 SysV IPC bug reproducer | expand |
Hi! > 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..fb7857345 > --- /dev/null > +++ b/testcases/kernel/syscalls/ipc/shmat/shmat04.c > @@ -0,0 +1,91 @@ > +// 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" > + > +#define PAGE_SIZE 4096UL This has to be a variable initialized by getpagesize() in the test setup. There are several architectures out there that are using 64k pages some even by deafult and this will not work there. > +#define KEY_ID 0x1234 Hardcoding keys does not work well either, we do have GETIPCKEY() in the libnewipc LTP library to make sure that tests does not collidate with any existing keys or possible other tests. > +#define MAGIC_ADDR 290816 If anything this would be magic offset, since we are adding it to the pointer to the shared memory. Also this seems to be exactly 71 pages, so maybe this needs to be defined as a multiple of page size? > +static int segment_id = -1; > + > +static void run(void) > +{ > + char *sh_mem; > + struct shmid_ds shmid_ds; > + size_t segment_size = 16UL << 20; Does the segment size need to scale with the PAGE_SIZE as well? It seems to be long enough so maybe it does not, but I haven't checked the kernel to be sure. > + size_t addr_step = 4 * PAGE_SIZE; > + > + segment_id = SAFE_SHMGET( > + KEY_ID, > + segment_size, > + IPC_CREAT | IPC_EXCL | 0600); > + > + sh_mem = (char *)SAFE_SHMAT(segment_id, NULL, 0); > + > + tst_res(TINFO, "Segment ID: %d - Segment Size: %lu", segment_id, segment_size); > + tst_res(TINFO, "Attached at %p", sh_mem); > + > + SAFE_SHMCTL(segment_id, IPC_STAT, &shmid_ds); > + > + tst_res(TINFO, "Number of attaches: %lu", shmid_ds.shm_nattch); > + tst_res(TINFO, "Disable memory access"); > + > + for (char *addr = sh_mem; addr < sh_mem + segment_size; addr += addr_step) > + mprotect(addr, PAGE_SIZE, PROT_NONE); > + > + tst_res(TINFO, "Write memory protection at %d", MAGIC_ADDR); > + > + /* This breaks it - anything but PROT_NONE */ > + mprotect(sh_mem + MAGIC_ADDR, 8192, 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; > + > + if (shmid_ds.shm_nattch) > + tst_res(TFAIL, "The system is affected by the SysV IPC bug"); > + else > + tst_res(TPASS, "Test passed"); > +} > + > +static void cleanup(void) > +{ > + if (segment_id != -1) > + SAFE_SHMCTL(segment_id, IPC_RMID, NULL); > +} > + > +static struct tst_test test = { > + .test_all = run, > + .cleanup = cleanup, And once the kernel patch hits the stable we should add the right tags here.
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..fb7857345 --- /dev/null +++ b/testcases/kernel/syscalls/ipc/shmat/shmat04.c @@ -0,0 +1,91 @@ +// 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" + +#define PAGE_SIZE 4096UL +#define KEY_ID 0x1234 +#define MAGIC_ADDR 290816 + +static int segment_id = -1; + +static void run(void) +{ + char *sh_mem; + struct shmid_ds shmid_ds; + size_t segment_size = 16UL << 20; + size_t addr_step = 4 * PAGE_SIZE; + + segment_id = SAFE_SHMGET( + KEY_ID, + segment_size, + IPC_CREAT | IPC_EXCL | 0600); + + sh_mem = (char *)SAFE_SHMAT(segment_id, NULL, 0); + + tst_res(TINFO, "Segment ID: %d - Segment Size: %lu", segment_id, segment_size); + tst_res(TINFO, "Attached at %p", sh_mem); + + SAFE_SHMCTL(segment_id, IPC_STAT, &shmid_ds); + + tst_res(TINFO, "Number of attaches: %lu", shmid_ds.shm_nattch); + tst_res(TINFO, "Disable memory access"); + + for (char *addr = sh_mem; addr < sh_mem + segment_size; addr += addr_step) + mprotect(addr, PAGE_SIZE, PROT_NONE); + + tst_res(TINFO, "Write memory protection at %d", MAGIC_ADDR); + + /* This breaks it - anything but PROT_NONE */ + mprotect(sh_mem + MAGIC_ADDR, 8192, 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; + + if (shmid_ds.shm_nattch) + tst_res(TFAIL, "The system is affected by the SysV IPC bug"); + else + tst_res(TPASS, "Test passed"); +} + +static void cleanup(void) +{ + if (segment_id != -1) + SAFE_SHMCTL(segment_id, IPC_RMID, NULL); +} + +static struct tst_test test = { + .test_all = run, + .cleanup = cleanup, +};