[LTP] [PATCH] hugeshmctl02: Skipped EFAULT tests for libc variant

[Dylan Dai-Rong Jhong(鍾岱融)]

Hi Li Wang,

Thanks for the review.

Please ignore my email I just replied to you, I forgot to click “reply all”.
Please consider this email the primary email.

From: Li Wang <liwang@redhat.com>
Sent: Monday, June 26, 2023 2:57 PM
To: Dylan Dai-Rong Jhong(鍾岱融) <dylan@andestech.com>
Cc: ltp@lists.linux.it; Mina Hui-Min Chou(周慧敏) <minachou@andestech.com>; Tim Shih-Ting OuYang(歐陽士庭) <tim609@andestech.com>; x5710999x@gmail.com; Cyril Hrubis <chrubis@suse.cz>
Subject: Re: [LTP] [PATCH] hugeshmctl02: Skipped EFAULT tests for libc variant



On Thu, Jun 15, 2023 at 5:40 PM Dylan Jhong <dylan@andestech.com<mailto:dylan@andestech.com>> wrote:
When testing hugeshmctl02 under the 32bit architecture, a segmentation fault
will occur. This patch will skip EFAULT tests for libc variant.

Hugeshmctl02 will intentionally pass "(struct shmid_ds *)-1" to shmctl(), but
glibc will perform an additional conversion function when the architecture is
32bit, which will try to copy all items in (struct shmid_ds *) to another
structure[*1]. In the process of copying, it is necessary to dereference
"(struct shmid_ds *)-1", resulting in segmentation fault.

The LTP also has similar problems before, this patch is reference from the
shmctl03 patch[*2].

[*1]: https://github.com/bminor/glibc/blob/master/sysdeps/unix/sysv/linux/shmctl.c#L37
[*2]: https://github.com/linux-test-project/ltp/commit/a5a80aa8485a7cb017f96aba8d7b5ea79f1ba4d4

Signed-off-by: Dylan Jhong <dylan@andestech.com<mailto:dylan@andestech.com>>
---
 .../mem/hugetlb/hugeshmctl/hugeshmctl02.c     | 35 ++++++++++++++++++-
 1 file changed, 34 insertions(+), 1 deletion(-)

diff --git a/testcases/kernel/mem/hugetlb/hugeshmctl/hugeshmctl02.c b/testcases/kernel/mem/hugetlb/hugeshmctl/hugeshmctl02.c
index 0bc9ffd74..e9c2e9fc8 100644
--- a/testcases/kernel/mem/hugetlb/hugeshmctl/hugeshmctl02.c
+++ b/testcases/kernel/mem/hugetlb/hugeshmctl/hugeshmctl02.c
@@ -27,6 +27,7 @@
 #include <pwd.h>
 #include <limits.h>
 #include "hugetlb.h"
+#include "lapi/syscalls.h"

 static size_t shm_size;
 static int shm_id_1 = -1;
@@ -50,9 +51,37 @@ struct tcase {
        {&shm_id_2, -1, &buf, EINVAL},
 };

+static int libc_shmctl(int shmid, int cmd, void *buf)
+{
+       return shmctl(shmid, cmd, buf);
+}
+
+static int sys_shmctl(int shmid, int cmd, void *buf)
+{
+       return tst_syscall(__NR_shmctl, shmid, cmd, buf);
+}
+
+static struct test_variants
+{
+       int (*shmctl)(int shmid, int cmd, void *buf);
+       char *desc;
+} variants[] = {
+       { .shmctl = libc_shmctl, .desc = "libc shmctl()"},
+#if (__NR_shmctl != __LTP__NR_INVALID_SYSCALL)
+       { .shmctl = sys_shmctl,  .desc = "__NR_shmctl syscall"},
+#endif
+};
+
 static void test_hugeshmctl(unsigned int i)
 {
-       TEST(shmctl(*(tcases[i].shmid), tcases[i].cmd, tcases[i].sbuf));
+       struct test_variants *tv = &variants[tst_variant];
+
+       if (tcases[i].error == EFAULT && tv->shmctl == libc_shmctl) {

You pointed the segment fault only exists on 32bit when use
libc wrapper, but this condition skips for both 64and32 bits,
isn't it?

I guess the strict condition should be as below?

    if (tcases[i].error == EFAULT && tv->shmctl == libc_shmctl && tst_kernel_bits() == 32) {
    ...
    }


Indeed, this only happens on 32-bit architectures, so the condition you added is no problem.
But in other LTP test cases, this condition is not considered on both 32/64 bit.
For example:
    - msgctl04: https://github.com/linux-test-project/ltp/blob/master/testcases/kernel/syscalls/ipc/msgctl/msgctl04.c#L74
    - semctl04: https://github.com/linux-test-project/ltp/blob/master/testcases/kernel/syscalls/ipc/semctl/semctl03.c#L78
    - sched_rr_get_interval03.c: https://github.com/linux-test-project/ltp/blob/master/testcases/kernel/syscalls/sched_rr_get_interval/sched_rr_get_interval03.c#L71

Not sure if the original developers didn't think of this or LTP just wanted to ignore this condition.
Do you think the condition needs to be modified to only ignore 32 bits, or designed to be the same as the other LTP test cases so that 32/64 bits are ignored?

Thanks,
Dylan

+               tst_res(TCONF, "EFAULT is skipped for libc variant");
+               return;
+       }
+
+       TEST(tv->shmctl(*(tcases[i].shmid), tcases[i].cmd, tcases[i].sbuf));
        if (TST_RET != -1) {
                tst_res(TFAIL, "shmctl succeeded unexpectedly");
                return;
@@ -70,8 +99,11 @@ static void test_hugeshmctl(unsigned int i)

 static void setup(void)
 {
+       struct test_variants *tv = &variants[tst_variant];
        long hpage_size;

+       tst_res(TINFO, "Testing variant: %s", tv->desc);
+
        if (tst_hugepages == 0)
                tst_brk(TCONF, "No enough hugepages for testing.");

@@ -101,6 +133,7 @@ static void cleanup(void)

 static struct tst_test test = {
        .test = test_hugeshmctl,
+       .test_variants = ARRAY_SIZE(variants),
        .tcnt = ARRAY_SIZE(tcases),
        .needs_root = 1,
        .needs_tmpdir = 1,
--
2.34.1


--
Mailing list info: https://lists.linux.it/listinfo/ltp


--
Regards,
Li Wang
CONFIDENTIALITY NOTICE:

This e-mail (and its attachments) may contain confidential and legally privileged information or information protected from disclosure. If you are not the intended recipient, you are hereby notified that any disclosure, copying, distribution, or use of the information contained herein is strictly prohibited. In this case, please immediately notify the sender by return e-mail, delete the message (and any accompanying documents) and destroy all printed hard copies. Thank you for your cooperation.

Copyright ANDES TECHNOLOGY CORPORATION - All Rights Reserved.