[LTP] [PATCH 1/1] mmap20: Fix invalid address

[LTP] [PATCH 1/1] mmap20: Fix invalid address

[Petr Vorel]

1 << 7 (0x80) is defined as MAP_LOCKED on ppc64le and mips.
Use 1 << 9 (0x200), which looks to be unused on all archs.

Fixes: 2764e9e6a ("mmap20: New test for mmap() with MAP_SHARED_VALIDATE")

Signed-off-by: Petr Vorel <pvorel@suse.cz>
---
NOTE: (1 << 10) would work as well.

 testcases/kernel/syscalls/mmap/mmap20.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/testcases/kernel/syscalls/mmap/mmap20.c b/testcases/kernel/syscalls/mmap/mmap20.c
index c346df5d1..5269179e5 100644
--- a/testcases/kernel/syscalls/mmap/mmap20.c
+++ b/testcases/kernel/syscalls/mmap/mmap20.c
@@ -20,7 +20,7 @@
 
 #define TEST_FILE "file_to_mmap"
 #define TEST_FILE_SIZE 1024
-#define INVALID_FLAG (1 << 7)
+#define INVALID_FLAG (1 << 9)
 
 static int fd = -1;
 static void *addr;
-- 
2.40.0


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

Re: [LTP] [PATCH 1/1] mmap20: Fix invalid address

[Jan Stancek]

On Tue, Apr 4, 2023 at 1:29 PM Petr Vorel <pvorel@suse.cz> wrote:
>
> 1 << 7 (0x80) is defined as MAP_LOCKED on ppc64le and mips.
> Use 1 << 9 (0x200), which looks to be unused on all archs.
>
> Fixes: 2764e9e6a ("mmap20: New test for mmap() with MAP_SHARED_VALIDATE")
>
> Signed-off-by: Petr Vorel <pvorel@suse.cz>

Should work. Alternative is we don't map a file, and then add MAP_SYNC to flags,
which looking at LEGACY_MAP_MASK should also give us -EOPNOTSUPP.

I'd go with the patch, we can tweak it later if someone has better idea:
Acked-by: Jan Stancek <jstancek@redhat.com>

> ---
> NOTE: (1 << 10) would work as well.
>
>  testcases/kernel/syscalls/mmap/mmap20.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/testcases/kernel/syscalls/mmap/mmap20.c b/testcases/kernel/syscalls/mmap/mmap20.c
> index c346df5d1..5269179e5 100644
> --- a/testcases/kernel/syscalls/mmap/mmap20.c
> +++ b/testcases/kernel/syscalls/mmap/mmap20.c
> @@ -20,7 +20,7 @@
>
>  #define TEST_FILE "file_to_mmap"
>  #define TEST_FILE_SIZE 1024
> -#define INVALID_FLAG (1 << 7)
> +#define INVALID_FLAG (1 << 9)
>
>  static int fd = -1;
>  static void *addr;
> --
> 2.40.0
>


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

Re: [LTP] [PATCH 1/1] mmap20: Fix invalid address

[Li Wang]

Hi Petr,

(1<<9) is also used by another flag on alpha, but it said not
linux-specific,
so I guess it won't be a problem (at least for now).
  alpha/include/uapi/asm/mman.h:#define _MAP_HASSEMAPHORE 0x0200

Reviewed-by: Li Wang <liwang@redhat.com>


On Tue, Apr 4, 2023 at 7:29 PM Petr Vorel <pvorel@suse.cz> wrote:

> 1 << 7 (0x80) is defined as MAP_LOCKED on ppc64le and mips.
> Use 1 << 9 (0x200), which looks to be unused on all archs.
>
> Fixes: 2764e9e6a ("mmap20: New test for mmap() with MAP_SHARED_VALIDATE")
>
> Signed-off-by: Petr Vorel <pvorel@suse.cz>
> ---
> NOTE: (1 << 10) would work as well.
>
>  testcases/kernel/syscalls/mmap/mmap20.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/testcases/kernel/syscalls/mmap/mmap20.c
> b/testcases/kernel/syscalls/mmap/mmap20.c
> index c346df5d1..5269179e5 100644
> --- a/testcases/kernel/syscalls/mmap/mmap20.c
> +++ b/testcases/kernel/syscalls/mmap/mmap20.c
> @@ -20,7 +20,7 @@
>
>  #define TEST_FILE "file_to_mmap"
>  #define TEST_FILE_SIZE 1024
> -#define INVALID_FLAG (1 << 7)
> +#define INVALID_FLAG (1 << 9)
>
>  static int fd = -1;
>  static void *addr;
> --
> 2.40.0
>
>

-- 
Regards,
Li Wang

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

Re: [LTP] [PATCH 1/1] mmap20: Fix invalid address

[Petr Vorel]

Hi all,

> Hi Petr,

> (1<<9) is also used by another flag on alpha, but it said not
> linux-specific,
> so I guess it won't be a problem (at least for now).
>   alpha/include/uapi/asm/mman.h:#define _MAP_HASSEMAPHORE 0x0200

Good point! I checked only glibc/musl headers, obviously checking kernel sources
is better as it contains old archs. It affects alpha, ia64 and sparc
(well, they might not even compile current LTP, we were dropping ia64 fixes
anyway).

$ git grep -w 0x0200 arch/*/include/uapi/asm/mman.h
arch/alpha/include/uapi/asm/mman.h:#define _MAP_HASSEMAPHORE 0x0200
arch/ia64/include/uapi/asm/mman.h:#define MAP_GROWSUP   0x0200          /* register stack-like segment */
arch/sparc/include/uapi/asm/mman.h:#define MAP_GROWSDOWN        0x0200          /* stack-like segment */

But as 0x400 (1 << 10) is not used anywhere, I merged it with this value.

It'd be good to follow Jan's solution (don't map a file, and then add MAP_SYNC
to flags), but I'll postpone it to later/when needed/never (there are other
tests which needs to be fixed.

Thanks a lot both!

Kind regards,
Petr

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

Re: [LTP] [PATCH 1/1] mmap20: Fix invalid address

[Li Wang]

On Thu, Apr 6, 2023 at 5:49 PM Petr Vorel <pvorel@suse.cz> wrote:

> Hi all,
>
> > Hi Petr,
>
> > (1<<9) is also used by another flag on alpha, but it said not
> > linux-specific,
> > so I guess it won't be a problem (at least for now).
> >   alpha/include/uapi/asm/mman.h:#define _MAP_HASSEMAPHORE 0x0200
>
> Good point! I checked only glibc/musl headers, obviously checking kernel
> sources
> is better as it contains old archs. It affects alpha, ia64 and sparc
> (well, they might not even compile current LTP, we were dropping ia64 fixes
> anyway).
>

Okay, I only noticed that alpha is not defined 0x0200 for Linux
specifically.



>
> $ git grep -w 0x0200 arch/*/include/uapi/asm/mman.h
> arch/alpha/include/uapi/asm/mman.h:#define _MAP_HASSEMAPHORE 0x0200
> arch/ia64/include/uapi/asm/mman.h:#define MAP_GROWSUP   0x0200          /*
> register stack-like segment */
> arch/sparc/include/uapi/asm/mman.h:#define MAP_GROWSDOWN        0x0200
>       /* stack-like segment */
>
> But as 0x400 (1 << 10) is not used anywhere, I merged it with this value.
>


Hmm, I didn't understand here why merged with 0x0400,
aren't there still duplicated flags?

$ git grep -w 0x0400 arch/*/include/uapi/asm/mman.h
arch/alpha/include/uapi/asm/mman.h:#define _MAP_INHERIT 0x0400
arch/mips/include/uapi/asm/mman.h:#define MAP_NORESERVE 0x0400          /*
don't check for reservations */
arch/xtensa/include/uapi/asm/mman.h:#define MAP_NORESERVE       0x0400
     /* don't check for reservations */



>
> It'd be good to follow Jan's solution (don't map a file, and then add
> MAP_SYNC
> to flags), but I'll postpone it to later/when needed/never (there are other
> tests which needs to be fixed.
>
> Thanks a lot both!
>
> Kind regards,
> Petr
>
>

-- 
Regards,
Li Wang

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

Re: [LTP] [PATCH 1/1] mmap20: Fix invalid address

[Petr Vorel]

Hi all,

> On Thu, Apr 6, 2023 at 5:49 PM Petr Vorel <pvorel@suse.cz> wrote:

> > Hi all,

> > > Hi Petr,

> > > (1<<9) is also used by another flag on alpha, but it said not
> > > linux-specific,
> > > so I guess it won't be a problem (at least for now).
> > >   alpha/include/uapi/asm/mman.h:#define _MAP_HASSEMAPHORE 0x0200

> > Good point! I checked only glibc/musl headers, obviously checking kernel
> > sources
> > is better as it contains old archs. It affects alpha, ia64 and sparc
> > (well, they might not even compile current LTP, we were dropping ia64 fixes
> > anyway).


> Okay, I only noticed that alpha is not defined 0x0200 for Linux
> specifically.

> > $ git grep -w 0x0200 arch/*/include/uapi/asm/mman.h
> > arch/alpha/include/uapi/asm/mman.h:#define _MAP_HASSEMAPHORE 0x0200
> > arch/ia64/include/uapi/asm/mman.h:#define MAP_GROWSUP   0x0200          /*
> > register stack-like segment */
> > arch/sparc/include/uapi/asm/mman.h:#define MAP_GROWSDOWN        0x0200
> >       /* stack-like segment */

> > But as 0x400 (1 << 10) is not used anywhere, I merged it with this value.



> Hmm, I didn't understand here why merged with 0x0400,
> aren't there still duplicated flags?

> $ git grep -w 0x0400 arch/*/include/uapi/asm/mman.h
> arch/alpha/include/uapi/asm/mman.h:#define _MAP_INHERIT 0x0400
> arch/mips/include/uapi/asm/mman.h:#define MAP_NORESERVE 0x0400          /*
> don't check for reservations */
> arch/xtensa/include/uapi/asm/mman.h:#define MAP_NORESERVE       0x0400
>      /* don't check for reservations */

I'm sorry I now don't know what I greped, because there are results for 0x0400 :(.
Obviously last minute change without ack is a bad practise.

You're right, linux specific 0x0400 is in mips and xtensa (it's non-linux
specific on alpha). OTOH ia64 and sparc define:

arch/ia64/include/uapi/asm/mman.h
#define MAP_GROWSUP	0x0200		/* register stack-like segment */

arch/sparc/include/uapi/asm/mman.h
#define MAP_GROWSDOWN	0x0200		/* stack-like segment */

Are these used as flags? If yes none of the values is optimal as it breaks some
arch. If you agree, I'd keep 0x0400 and follow Jan's suggestion (don't map a
file, and then add MAP_SYNC to flags).

Kind regards,
Petr

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

Re: [LTP] [PATCH 1/1] mmap20: Fix invalid address

[Li Wang]

On Thu, Apr 6, 2023 at 7:13 PM Petr Vorel <pvorel@suse.cz> wrote:

> Hi all,
>
> > On Thu, Apr 6, 2023 at 5:49 PM Petr Vorel <pvorel@suse.cz> wrote:
>
> > > Hi all,
>
> > > > Hi Petr,
>
> > > > (1<<9) is also used by another flag on alpha, but it said not
> > > > linux-specific,
> > > > so I guess it won't be a problem (at least for now).
> > > >   alpha/include/uapi/asm/mman.h:#define _MAP_HASSEMAPHORE 0x0200
>
> > > Good point! I checked only glibc/musl headers, obviously checking
> kernel
> > > sources
> > > is better as it contains old archs. It affects alpha, ia64 and sparc
> > > (well, they might not even compile current LTP, we were dropping ia64
> fixes
> > > anyway).
>
>
> > Okay, I only noticed that alpha is not defined 0x0200 for Linux
> > specifically.
>
> > > $ git grep -w 0x0200 arch/*/include/uapi/asm/mman.h
> > > arch/alpha/include/uapi/asm/mman.h:#define _MAP_HASSEMAPHORE 0x0200
> > > arch/ia64/include/uapi/asm/mman.h:#define MAP_GROWSUP   0x0200
>   /*
> > > register stack-like segment */
> > > arch/sparc/include/uapi/asm/mman.h:#define MAP_GROWSDOWN        0x0200
> > >       /* stack-like segment */
>
> > > But as 0x400 (1 << 10) is not used anywhere, I merged it with this
> value.
>
>
>
> > Hmm, I didn't understand here why merged with 0x0400,
> > aren't there still duplicated flags?
>
> > $ git grep -w 0x0400 arch/*/include/uapi/asm/mman.h
> > arch/alpha/include/uapi/asm/mman.h:#define _MAP_INHERIT 0x0400
> > arch/mips/include/uapi/asm/mman.h:#define MAP_NORESERVE 0x0400
> /*
> > don't check for reservations */
> > arch/xtensa/include/uapi/asm/mman.h:#define MAP_NORESERVE       0x0400
> >      /* don't check for reservations */
>
> I'm sorry I now don't know what I greped, because there are results for
> 0x0400 :(.
> Obviously last minute change without ack is a bad practise.
>
> You're right, linux specific 0x0400 is in mips and xtensa (it's non-linux
> specific on alpha). OTOH ia64 and sparc define:
>
> arch/ia64/include/uapi/asm/mman.h
> #define MAP_GROWSUP     0x0200          /* register stack-like segment */
>
> arch/sparc/include/uapi/asm/mman.h
> #define MAP_GROWSDOWN   0x0200          /* stack-like segment */
>
> Are these used as flags? If yes none of the values is optimal as it breaks
> some
>

The answer is yes, MAP_GROWSDOWN/GROWSUP is
widely used for stack growth direction.

It is hard to choose an unused value for INVALID_FLAG,
as it there possibly be duplicated in the future we don't know.



> arch. If you agree, I'd keep 0x0400 and follow Jan's suggestion (don't map
> a
> file, and then add MAP_SYNC to flags).
>

Add MAP_SYNC sounds like a good practice, we can have a try.

-- 
Regards,
Li Wang

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