Re: [LTP] [PATCH v6] Refactor fork14 using new LTP API

[Petr Vorel <pvorel@suse.cz>]

Hi Andrea,

> Hi!

> On 5/6/24 22:26, Petr Vorel wrote:
> > Hi Andrea, Martin,

> >> Hi,
> >> Reviewed-by: Martin Doucha <mdoucha@suse.cz>
> > +1

> > ...

> > Reviewed-by: Petr Vorel <pvorel@suse.cz>
> >>> +
> >>> +static struct tst_test test = {
> >>> +	.test_all = run,
> >>> +	.setup = setup,
> >>> +	.cleanup = cleanup,
> >>> +	.forks_child = 1,
> >>> +	.skip_in_compat = 1,
> > BTW test on x86 (i.e. true 64 bit) behaves exactly the same as on compat mode:

> > tst_test.c:1614: TINFO: Timeout per run is 0h 00m 30s
> > fork14.c:46: TINFO: mmap() failed
> > fork14.c:46: TINFO: mmap() failed
> > fork14.c:46: TINFO: mmap() failed
> > fork14.c:46: TINFO: mmap() failed
> > fork14.c:46: TINFO: mmap() failed
> > fork14.c:46: TINFO: mmap() failed
> > fork14.c:46: TINFO: mmap() failed
> > fork14.c:46: TINFO: mmap() failed
> > fork14.c:46: TINFO: mmap() failed
> > fork14.c:46: TINFO: mmap() failed
> > fork14.c:46: TINFO: mmap() failed
> > fork14.c:49: TCONF: mmap() fails too many times, so it's almost impossible to get a vm_area_struct sized 16TB.

> > IMHO we should whitelist it as well, I can change before merge with diff below.

> > (More elegant way would be to add .skip_in_32bit.)

> > Kind regards,
> > Petr

> > +++ testcases/kernel/syscalls/fork/fork14.c
> > @@ -18,6 +18,8 @@
> >   #include <stdlib.h>
> >   #include <sys/wait.h>

> > +#ifndef __i386__
> TST_ABI32 is not enough? What about ".skip_in_compat"?

Yes, TST_ABI32 is actually better than limit to __i386__.
Do you give ack for these changes below? Or feel free to send new version?
IMHO the problem is 16TB is too much for 32 bit kernel (4 GiB limit).

.skip_in_compat would not be enough, because the problem is on pure 32 bit
distro e.g. with 32bit kernel (yes, there are people who use them), but
.skip_in_compat is for 64 bit kernel with 32bit userspace (compiled with -m32 in
CFLAGS and LDFLAGS).

Also, I suggested in the patch below to remove .skip_in_compat, but for info
version it would be good to keep it. I suppose it's not worth to add new flag
.skip_in_32bit for this single case (it'd be good for doc purposes).
@Cyril WDYT?

+++ testcases/kernel/syscalls/fork/fork14.c
@@ -7,17 +7,21 @@
 /*\
  * [Description]
  *
- * This test is a reporducer for this patch:
- * https://lore.kernel.org/lkml/1335289853-2923-1-git-send-email-siddhesh.poyarekar@gmail.com/
- * Since vma length in dup_mmap is calculated and stored in a unsigned
+ * This test is a reproducer for kernel 3.5:
+ * 7edc8b0ac16c ("mm/fork: fix overflow in vma length when copying mmap on clone")
+ *
+ * Since VMA length in dup_mmap() is calculated and stored in a unsigned
  * int, it will overflow when length of mmaped memory > 16 TB. When
  * overflow occurs, fork will incorrectly succeed. The patch above fixed it.
  */
 
+#include "lapi/abisize.h"
 #include "tst_test.h"
 #include <stdlib.h>
 #include <sys/wait.h>
 
+#ifndef TST_ABI32
+
 #define LARGE		(16 * 1024)
 #define EXTENT		(16 * 1024 + 10)
 
@@ -48,7 +52,7 @@ static void run(void)
 			if (failures > 10) {
 				tst_brk(TCONF, "mmap() fails too many "
 					"times, so it's almost impossible to "
-					"get a vm_area_struct sized 16TB.");
+					"get a vm_area_struct sized 16TB");
 			}
 
 			continue;
@@ -115,9 +119,11 @@ static struct tst_test test = {
 	.setup = setup,
 	.cleanup = cleanup,
 	.forks_child = 1,
-	.skip_in_compat = 1,
 	.tags = (const struct tst_tag[]) {
 		{"linux-git", "7edc8b0ac16c"},
 		{}
 	}
 };
+#else
+TST_TEST_TCONF("Not supported on x86 in 32-bit");
+#endif

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