Messages in this thread | | | Date | Mon, 29 May 2023 18:16:29 +0200 | Subject | Re: LTP: shmget02 fails on compat mode - 64-bit kernel and 32-bit userspace | From | Manfred Spraul <> |
| |
Hi Li,
On 5/20/23 05:58, Li Wang wrote: > Hi Manfred, > > On Sat, May 20, 2023 at 1:55 AM Manfred Spraul > <manfred@colorfullife.com> wrote: > > Hi all, > > On 5/19/23 12:57, Arnd Bergmann wrote: > > On Fri, May 19, 2023, at 11:17, Naresh Kamboju wrote: > >> LTP running on compat mode where the tests run on > >> 64-bit kernel and 32-bit userspace are noticed on a list of > failures. > >> > >> What would be the best way to handle this rare combination of > failures ? > >> > >> * arm64: juno-r2-compat, qemu_arm64-compat and qemu_x86_64-compat > >> - shmget02 > >> > >> Reported-by: Linux Kernel Functional Testing <lkft@linaro.org> > >> > >> tst_hugepage.c:83: TINFO: 0 hugepage(s) reserved > >> tst_test.c:1558: TINFO: Timeout per run is 0h 02m 30s > >> tst_kconfig.c:87: TINFO: Parsing kernel config '/proc/config.gz' > >> shmget02.c:95: TPASS: shmget(1644199826, 2048, 1024) : ENOENT (2) > >> shmget02.c:95: TPASS: shmget(1627422610, 2048, 1536) : EEXIST (17) > >> <4>[ 84.678150] __vm_enough_memory: pid: 513, comm: shmget02, not > >> enough memory for the allocation > >> shmget02.c:95: TPASS: shmget(1644199826, 0, 1536) : EINVAL (22) > >> shmget02.c:95: TFAIL: shmget(1644199826, 4278190080, 1536) expected > >> EINVAL: ENOMEM (12) > > Adding Liam Howlett, Davidlohr Bueso and Manfred Spraul to Cc, they > > have worked on the shm code in the past few years. > > > > This is the line > > > > {&shmkey1, SHMMAX + 1, IPC_CREAT | IPC_EXCL, 0, 0, EINVAL}, > > > > from > > > > > https://github.com/linux-test-project/ltp/blob/04e8f2f4fd949/testcases/kernel/syscalls/ipc/shmget/shmget02.c#LL59C1-L59C61 > > > > right? > > > > I think this is a result of SHMMAX being defined as > > #define SHMMAX (ULONG_MAX - (1UL << 24)), so the kernel would > > likely use a large 64-bit value here, while the 32-bit user > > space uses a much smaller limit. > > > > The expected return code likely comes from > > > > static int newseg(struct ipc_namespace *ns, struct ipc_params > *params) > > { > > ... > > if (size < SHMMIN || size > ns->shm_ctlmax) > > return -EINVAL; > > > > but if ns->shm_ctlmax is probably set to the 64-bit value here. > > It would then trigger the accounting limit in __shmem_file_setup(): > > > > if (shmem_acct_size(flags, size)) > > return ERR_PTR(-ENOMEM); > > > > My feeling is that the kernel in this case works as expected, > > and I wouldn't see this as a bug. On the other hand, this > > can probably be addressed in the kernel by adding a check for > > compat tasks like > > > > --- a/ipc/shm.c > > +++ b/ipc/shm.c > > @@ -714,7 +714,8 @@ static int newseg(struct ipc_namespace *ns, > struct ipc_params *params) > > char name[13]; > > vm_flags_t acctflag = 0; > > > > - if (size < SHMMIN || size > ns->shm_ctlmax) > > + if (size < SHMMIN || size > ns->shm_ctlmax || > > + in_compat_syscall() && size > COMPAT_SHMMAX)) > > return -EINVAL; > > > > if (numpages << PAGE_SHIFT < size) > > > I would consider this as ugly: ns->shm_ctlmax can be configured by > writing to /proc/sys/kernel/shmmax. > > You can break the test case on 64-bit as well, just by writing > SHMMAX+1 > to /proc/sys/kernel/shmmax > > Thus I think the test case is flawed: > > It is testing the overflow behavior for a configurable value by > testing > with default+1. But sometimes the actual value is not the default. > > Are the tests running as root? > > > Yes. > > > What about intentionally setting the value to something useful? > > > > This suggest sounds reasonable, but I have a question: > is there any upper limit for setting the /proc/sys/kernel/shmmax? > The real limit is 0x7fffffffffffffff. Even if the value of shmmax is higher, shmget() fails.
I think this is due to MAX_LFS_FILESIZE in __shmem_file_setup(). I didn't attach a debugger, thus I cannot rule out that there is another check that also rejects >= 0x800<...>0
The maximum useful size is probably even lower, shmat() would fail since the virtual memory size is even smaller.
> > The test seems to try to test the bounder and as a > corner case for covering that scenario. But then just reduce shmmax:
- test that shmget(5000) works
- echo "4999" > /proc/sys/kernel/shmmax
- test that shmget(5000) fails
- echo "5000" > /proc/sys/kernel/shmmax
- test that shmget(5000) works again.
> > tmp=$(cat /proc/sys/kernel/shmmax) > > echo "1234" > /proc/sys/kernel/shmmax > > semget() based on {&shmkey1, 1234 + 1, IPC_CREAT | IPC_EXCL, 0, 0, > EINVAL}, > echo $tmp >/proc/sys/kernel/shmmax > > Or, alternatively: read /proc/sys/kernel/shmmax, and skip the test if > the value is larger than ULONG_MAX-1. > > -- > Manfred > > > > > -- > Regards, > Li Wang
| |