[PATCH] mm: redefine the MAP_SHARED_VALIDATE to other value

[PATCH] mm: redefine the MAP_SHARED_VALIDATE to other value

[zhong jiang]

As the mman manual says, mmap should return fails when we assign
the flags to MAP_SHARED | MAP_PRIVATE.

But In fact, We run the code successfully and unexpected.
It is because MAP_SHARED_VALIDATE is introduced and equal to
MAP_SHARED | MAP_PRIVATE.

Signed-off-by: zhong jiang <zhongjiang@huawei.com>
---
 include/uapi/linux/mman.h                          | 2 +-
 tools/include/uapi/asm-generic/mman-common-tools.h | 2 +-
 tools/include/uapi/linux/mman.h                    | 2 +-
 3 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/include/uapi/linux/mman.h b/include/uapi/linux/mman.h
index fc1a64c..1d3098e 100644
--- a/include/uapi/linux/mman.h
+++ b/include/uapi/linux/mman.h
@@ -14,7 +14,7 @@
 
 #define MAP_SHARED	0x01		/* Share changes */
 #define MAP_PRIVATE	0x02		/* Changes are private */
-#define MAP_SHARED_VALIDATE 0x03	/* share + validate extension flags */
+#define MAP_SHARED_VALIDATE 0x04	/* share + validate extension flags */
 
 /*
  * Huge page size encoding when MAP_HUGETLB is specified, and a huge page
diff --git a/tools/include/uapi/asm-generic/mman-common-tools.h b/tools/include/uapi/asm-generic/mman-common-tools.h
index af7d0d3..4fc44d2 100644
--- a/tools/include/uapi/asm-generic/mman-common-tools.h
+++ b/tools/include/uapi/asm-generic/mman-common-tools.h
@@ -18,6 +18,6 @@
 #ifndef MAP_SHARED
 #define MAP_SHARED	0x01		/* Share changes */
 #define MAP_PRIVATE	0x02		/* Changes are private */
-#define MAP_SHARED_VALIDATE 0x03	/* share + validate extension flags */
+#define MAP_SHARED_VALIDATE 0x04	/* share + validate extension flags */
 #endif
 #endif // __ASM_GENERIC_MMAN_COMMON_TOOLS_ONLY_H
diff --git a/tools/include/uapi/linux/mman.h b/tools/include/uapi/linux/mman.h
index fc1a64c..1d3098e 100644
--- a/tools/include/uapi/linux/mman.h
+++ b/tools/include/uapi/linux/mman.h
@@ -14,7 +14,7 @@
 
 #define MAP_SHARED	0x01		/* Share changes */
 #define MAP_PRIVATE	0x02		/* Changes are private */
-#define MAP_SHARED_VALIDATE 0x03	/* share + validate extension flags */
+#define MAP_SHARED_VALIDATE 0x04	/* share + validate extension flags */
 
 /*
  * Huge page size encoding when MAP_HUGETLB is specified, and a huge page
-- 
1.7.12.4

Re: [PATCH] mm: redefine the MAP_SHARED_VALIDATE to other value

[Michal Hocko]

[Cc Dan]

On Mon 08-07-19 16:05:41, zhong jiang wrote:
> As the mman manual says, mmap should return fails when we assign
> the flags to MAP_SHARED | MAP_PRIVATE.
> 
> But In fact, We run the code successfully and unexpected.

What is the code that you are running and what is the code version.

> It is because MAP_SHARED_VALIDATE is introduced and equal to
> MAP_SHARED | MAP_PRIVATE.

This was a deliberate decision IIRC. Have a look at 1c9725974074 ("mm:
introduce MAP_SHARED_VALIDATE, a mechanism to safely define new mmap
flags").

> Signed-off-by: zhong jiang <zhongjiang@huawei.com>
> ---
>  include/uapi/linux/mman.h                          | 2 +-
>  tools/include/uapi/asm-generic/mman-common-tools.h | 2 +-
>  tools/include/uapi/linux/mman.h                    | 2 +-
>  3 files changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/include/uapi/linux/mman.h b/include/uapi/linux/mman.h
> index fc1a64c..1d3098e 100644
> --- a/include/uapi/linux/mman.h
> +++ b/include/uapi/linux/mman.h
> @@ -14,7 +14,7 @@
>  
>  #define MAP_SHARED	0x01		/* Share changes */
>  #define MAP_PRIVATE	0x02		/* Changes are private */
> -#define MAP_SHARED_VALIDATE 0x03	/* share + validate extension flags */
> +#define MAP_SHARED_VALIDATE 0x04	/* share + validate extension flags */
>  
>  /*
>   * Huge page size encoding when MAP_HUGETLB is specified, and a huge page
> diff --git a/tools/include/uapi/asm-generic/mman-common-tools.h b/tools/include/uapi/asm-generic/mman-common-tools.h
> index af7d0d3..4fc44d2 100644
> --- a/tools/include/uapi/asm-generic/mman-common-tools.h
> +++ b/tools/include/uapi/asm-generic/mman-common-tools.h
> @@ -18,6 +18,6 @@
>  #ifndef MAP_SHARED
>  #define MAP_SHARED	0x01		/* Share changes */
>  #define MAP_PRIVATE	0x02		/* Changes are private */
> -#define MAP_SHARED_VALIDATE 0x03	/* share + validate extension flags */
> +#define MAP_SHARED_VALIDATE 0x04	/* share + validate extension flags */
>  #endif
>  #endif // __ASM_GENERIC_MMAN_COMMON_TOOLS_ONLY_H
> diff --git a/tools/include/uapi/linux/mman.h b/tools/include/uapi/linux/mman.h
> index fc1a64c..1d3098e 100644
> --- a/tools/include/uapi/linux/mman.h
> +++ b/tools/include/uapi/linux/mman.h
> @@ -14,7 +14,7 @@
>  
>  #define MAP_SHARED	0x01		/* Share changes */
>  #define MAP_PRIVATE	0x02		/* Changes are private */
> -#define MAP_SHARED_VALIDATE 0x03	/* share + validate extension flags */
> +#define MAP_SHARED_VALIDATE 0x04	/* share + validate extension flags */
>  
>  /*
>   * Huge page size encoding when MAP_HUGETLB is specified, and a huge page
> -- 
> 1.7.12.4

-- 
Michal Hocko
SUSE Labs

Re: [PATCH] mm: redefine the MAP_SHARED_VALIDATE to other value

[zhong jiang]

On 2019/7/8 17:20, Michal Hocko wrote:
> [Cc Dan]
>
> On Mon 08-07-19 16:05:41, zhong jiang wrote:
>> As the mman manual says, mmap should return fails when we assign
>> the flags to MAP_SHARED | MAP_PRIVATE.
>>
>> But In fact, We run the code successfully and unexpected.
> What is the code that you are running and what is the code version.
Just an following code, For example,
addr = mmap(ADDR, PAGE_SIZE, PROT_WRITE|PROT_EXEC, MAP_SHARED|MAP_PRIVATE, fildes, OFFSET);

We test it and works well in linux 4.19.   As the mmap manual says,  it should fails.
>> It is because MAP_SHARED_VALIDATE is introduced and equal to
>> MAP_SHARED | MAP_PRIVATE.
> This was a deliberate decision IIRC. Have a look at 1c9725974074 ("mm:
> introduce MAP_SHARED_VALIDATE, a mechanism to safely define new mmap
> flags").
I  has seen the patch,  It introduce the issue.  but it only define the MAP_SHARED_VALIDATE incorrectly.
Maybe the author miss the condition that MAP_SHARED_VALIDATE is equal to MAP_PRIVATE | MAP_SHARE.


Thanks,
zhong jiang
>> Signed-off-by: zhong jiang <zhongjiang@huawei.com>
>> ---
>>  include/uapi/linux/mman.h                          | 2 +-
>>  tools/include/uapi/asm-generic/mman-common-tools.h | 2 +-
>>  tools/include/uapi/linux/mman.h                    | 2 +-
>>  3 files changed, 3 insertions(+), 3 deletions(-)
>>
>> diff --git a/include/uapi/linux/mman.h b/include/uapi/linux/mman.h
>> index fc1a64c..1d3098e 100644
>> --- a/include/uapi/linux/mman.h
>> +++ b/include/uapi/linux/mman.h
>> @@ -14,7 +14,7 @@
>>  
>>  #define MAP_SHARED	0x01		/* Share changes */
>>  #define MAP_PRIVATE	0x02		/* Changes are private */
>> -#define MAP_SHARED_VALIDATE 0x03	/* share + validate extension flags */
>> +#define MAP_SHARED_VALIDATE 0x04	/* share + validate extension flags */
>>  
>>  /*
>>   * Huge page size encoding when MAP_HUGETLB is specified, and a huge page
>> diff --git a/tools/include/uapi/asm-generic/mman-common-tools.h b/tools/include/uapi/asm-generic/mman-common-tools.h
>> index af7d0d3..4fc44d2 100644
>> --- a/tools/include/uapi/asm-generic/mman-common-tools.h
>> +++ b/tools/include/uapi/asm-generic/mman-common-tools.h
>> @@ -18,6 +18,6 @@
>>  #ifndef MAP_SHARED
>>  #define MAP_SHARED	0x01		/* Share changes */
>>  #define MAP_PRIVATE	0x02		/* Changes are private */
>> -#define MAP_SHARED_VALIDATE 0x03	/* share + validate extension flags */
>> +#define MAP_SHARED_VALIDATE 0x04	/* share + validate extension flags */
>>  #endif
>>  #endif // __ASM_GENERIC_MMAN_COMMON_TOOLS_ONLY_H
>> diff --git a/tools/include/uapi/linux/mman.h b/tools/include/uapi/linux/mman.h
>> index fc1a64c..1d3098e 100644
>> --- a/tools/include/uapi/linux/mman.h
>> +++ b/tools/include/uapi/linux/mman.h
>> @@ -14,7 +14,7 @@
>>  
>>  #define MAP_SHARED	0x01		/* Share changes */
>>  #define MAP_PRIVATE	0x02		/* Changes are private */
>> -#define MAP_SHARED_VALIDATE 0x03	/* share + validate extension flags */
>> +#define MAP_SHARED_VALIDATE 0x04	/* share + validate extension flags */
>>  
>>  /*
>>   * Huge page size encoding when MAP_HUGETLB is specified, and a huge page
>> -- 
>> 1.7.12.4

Re: [PATCH] mm: redefine the MAP_SHARED_VALIDATE to other value

[Matthew Wilcox]

On Mon, Jul 08, 2019 at 04:05:41PM +0800, zhong jiang wrote:
> As the mman manual says, mmap should return fails when we assign
> the flags to MAP_SHARED | MAP_PRIVATE.
> 
> But In fact, We run the code successfully and unexpected.
> It is because MAP_SHARED_VALIDATE is introduced and equal to
> MAP_SHARED | MAP_PRIVATE.

No, you don't understand.  Look back at the introduction of
MAP_SHARED_VALIDATE.

Re: [PATCH] mm: redefine the MAP_SHARED_VALIDATE to other value

[Michal Hocko]

On Mon 08-07-19 21:52:53, zhong jiang wrote:
> On 2019/7/8 17:20, Michal Hocko wrote:
> > [Cc Dan]
> >
> > On Mon 08-07-19 16:05:41, zhong jiang wrote:
> >> As the mman manual says, mmap should return fails when we assign
> >> the flags to MAP_SHARED | MAP_PRIVATE.
> >>
> >> But In fact, We run the code successfully and unexpected.
> > What is the code that you are running and what is the code version.
> Just an following code, For example,
> addr = mmap(ADDR, PAGE_SIZE, PROT_WRITE|PROT_EXEC, MAP_SHARED|MAP_PRIVATE, fildes, OFFSET);

Is this a real code that relies on the failure or merely a simple test
to reflect the semantic you expect mmap to have?

> We test it and works well in linux 4.19.   As the mmap manual says,  it should fails.
> >> It is because MAP_SHARED_VALIDATE is introduced and equal to
> >> MAP_SHARED | MAP_PRIVATE.
> > This was a deliberate decision IIRC. Have a look at 1c9725974074 ("mm:
> > introduce MAP_SHARED_VALIDATE, a mechanism to safely define new mmap
> > flags").
> I  has seen the patch,  It introduce the issue.  but it only define the MAP_SHARED_VALIDATE incorrectly.
> Maybe the author miss the condition that MAP_SHARED_VALIDATE is equal to MAP_PRIVATE | MAP_SHARE.

No you are missing the point as Willy pointed out in a different email.
This is intentional. No real application could have used the combination
of two flags because it doesn't make any sense. And therefore the
combination has been chosen to chnage the mmap semantic and check for
valid mapping flags. LWN has a nice coverage[1].


[1] https://lwn.net/Articles/758594/
-- 
Michal Hocko
SUSE Labs

Re: [PATCH] mm: redefine the MAP_SHARED_VALIDATE to other value

[zhong jiang]

On 2019/7/9 0:43, Michal Hocko wrote:
> On Mon 08-07-19 21:52:53, zhong jiang wrote:
>> On 2019/7/8 17:20, Michal Hocko wrote:
>>> [Cc Dan]
>>>
>>> On Mon 08-07-19 16:05:41, zhong jiang wrote:
>>>> As the mman manual says, mmap should return fails when we assign
>>>> the flags to MAP_SHARED | MAP_PRIVATE.
>>>>
>>>> But In fact, We run the code successfully and unexpected.
>>> What is the code that you are running and what is the code version.
>> Just an following code, For example,
>> addr = mmap(ADDR, PAGE_SIZE, PROT_WRITE|PROT_EXEC, MAP_SHARED|MAP_PRIVATE, fildes, OFFSET);
> Is this a real code that relies on the failure or merely a simple test
> to reflect the semantic you expect mmap to have?
>
>> We test it and works well in linux 4.19.   As the mmap manual says,  it should fails.
>>>> It is because MAP_SHARED_VALIDATE is introduced and equal to
>>>> MAP_SHARED | MAP_PRIVATE.
>>> This was a deliberate decision IIRC. Have a look at 1c9725974074 ("mm:
>>> introduce MAP_SHARED_VALIDATE, a mechanism to safely define new mmap
>>> flags").
>> I  has seen the patch,  It introduce the issue.  but it only define the MAP_SHARED_VALIDATE incorrectly.
>> Maybe the author miss the condition that MAP_SHARED_VALIDATE is equal to MAP_PRIVATE | MAP_SHARE.
> No you are missing the point as Willy pointed out in a different email.
> This is intentional. No real application could have used the combination
> of two flags because it doesn't make any sense. And therefore the
> combination has been chosen to chnage the mmap semantic and check for
> valid mapping flags. LWN has a nice coverage[1].
Thanks you for pointing out.   I will look at the patch deeply.

Sincerely,
zhong jiang
>
> [1] https://lwn.net/Articles/758594/

Re: [PATCH] mm: redefine the MAP_SHARED_VALIDATE to other value

[zhong jiang]

On 2019/7/8 22:43, Matthew Wilcox wrote:
> On Mon, Jul 08, 2019 at 04:05:41PM +0800, zhong jiang wrote:
>> As the mman manual says, mmap should return fails when we assign
>> the flags to MAP_SHARED | MAP_PRIVATE.
>>
>> But In fact, We run the code successfully and unexpected.
>> It is because MAP_SHARED_VALIDATE is introduced and equal to
>> MAP_SHARED | MAP_PRIVATE.
> No, you don't understand.  Look back at the introduction of
> MAP_SHARED_VALIDATE.
>
> .
>
Thanks,  I will look at the patch deeply.

Sincerely,
zhong jiang