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?
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