2023-05-19 09:40:59

by Naresh Kamboju

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

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 <[email protected]>

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)
shmget02.c:95: TPASS: shmget(1627422610, 4096, 1024) : EINVAL (22)
shmget02.c:107: TPASS: shmget(1627422610, 2048, 256) : EACCES (13)
shmget02.c:107: TPASS: shmget(1644199826, 2048, 2560) : EPERM (1)
shmget02.c:95: TPASS: shmget(1644199826, 2048, 2560) : ENOMEM (12)

logs:
-----
- https://qa-reports.linaro.org/lkft/linux-mainline-master/build/v6.4-rc1-71-g105131df9c3b/testrun/16880333/suite/ltp-syscalls/test/shmget02/log
- https://qa-reports.linaro.org/lkft/linux-mainline-master/build/v6.4-rc1-71-g105131df9c3b/testrun/16880333/suite/ltp-syscalls/test/shmget02/history/


--
Linaro LKFT
https://lkft.linaro.org


2023-05-19 11:01:57

by Arnd Bergmann

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

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 <[email protected]>
>
> 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)


Arnd

2023-05-19 14:49:39

by Liam R. Howlett

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

* Arnd Bergmann <[email protected]> [230519 06:57]:
> 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.
> >

Is this new?

> > 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 <[email protected]>
> >
> > 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.

There was an issue with returning ENOMEM on 32bit which may be causing
this failure - if it's new.

>
> 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)
>
>
> Arnd

2023-05-19 18:25:17

by Manfred Spraul

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

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 <[email protected]>
>>
>> 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?

What about intentionally setting the value to something useful?

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



2023-05-29 16:25:18

by Manfred Spraul

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

Hi Li,

On 5/20/23 05:58, Li Wang wrote:
> Hi Manfred,
>
> On Sat, May 20, 2023 at 1:55 AM Manfred Spraul
> <[email protected]> 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 <[email protected]>
> >>
> >> 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