[LTP] LTP: shmget02 fails on compat mode - 64-bit kernel and 32-bit userspace

[Li Wang]

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/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.
>

Thank you Manfred for the suggestion, let me send a patch fix it in LTP.

-- 
Regards,
Li Wang