On Tue, May 30, 2023 at 12:16 AM Manfred Spraul manfred@colorfullife.com wrote:
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/kerne...
> > 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.
Thank you Manfred for the suggestion, let me send a patch fix it in LTP.