All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] x86/mmuext: don't allow copying/clearing non-RAM pages
@ 2017-06-21 10:10 Jan Beulich
  2017-06-21 10:51 ` Andrew Cooper
  0 siblings, 1 reply; 3+ messages in thread
From: Jan Beulich @ 2017-06-21 10:10 UTC (permalink / raw)
  To: xen-devel; +Cc: Andrew Cooper

[-- Attachment #1: Type: text/plain, Size: 2106 bytes --]

The two operations really aren't meant for anything else.

Signed-off-by: Jan Beulich <jbeulich@suse.com>

--- a/xen/arch/x86/mm.c
+++ b/xen/arch/x86/mm.c
@@ -3229,6 +3229,7 @@ long do_mmuext_op(
         switch ( op.cmd )
         {
             struct page_info *page;
+            p2m_type_t p2mt;
 
         case MMUEXT_PIN_L1_TABLE:
             type = PGT_l1_page_table;
@@ -3528,7 +3529,12 @@ long do_mmuext_op(
         }
 
         case MMUEXT_CLEAR_PAGE:
-            page = get_page_from_gfn(pg_owner, op.arg1.mfn, NULL, P2M_ALLOC);
+            page = get_page_from_gfn(pg_owner, op.arg1.mfn, &p2mt, P2M_ALLOC);
+            if ( unlikely(p2mt != p2m_ram_rw) && page )
+            {
+                put_page(page);
+                page = NULL;
+            }
             if ( !page || !get_page_type(page, PGT_writable_page) )
             {
                 if ( page )
@@ -3551,8 +3557,13 @@ long do_mmuext_op(
         {
             struct page_info *src_page, *dst_page;
 
-            src_page = get_page_from_gfn(pg_owner, op.arg2.src_mfn, NULL,
+            src_page = get_page_from_gfn(pg_owner, op.arg2.src_mfn, &p2mt,
                                          P2M_ALLOC);
+            if ( unlikely(p2mt != p2m_ram_rw) && src_page )
+            {
+                put_page(src_page);
+                src_page = NULL;
+            }
             if ( unlikely(!src_page) )
             {
                 gdprintk(XENLOG_WARNING,
@@ -3562,8 +3573,13 @@ long do_mmuext_op(
                 break;
             }
 
-            dst_page = get_page_from_gfn(pg_owner, op.arg1.mfn, NULL,
+            dst_page = get_page_from_gfn(pg_owner, op.arg1.mfn, &p2mt,
                                          P2M_ALLOC);
+            if ( unlikely(p2mt != p2m_ram_rw) && dst_page )
+            {
+                put_page(dst_page);
+                dst_page = NULL;
+            }
             rc = (dst_page &&
                   get_page_type(dst_page, PGT_writable_page)) ? 0 : -EINVAL;
             if ( unlikely(rc) )




[-- Attachment #2: x86-mmuext-copy-clear-RAM-only.patch --]
[-- Type: text/plain, Size: 2158 bytes --]

x86/mmuext: don't allow copying/clearing non-RAM pages

The two operations really aren't meant for anything else.

Signed-off-by: Jan Beulich <jbeulich@suse.com>

--- a/xen/arch/x86/mm.c
+++ b/xen/arch/x86/mm.c
@@ -3229,6 +3229,7 @@ long do_mmuext_op(
         switch ( op.cmd )
         {
             struct page_info *page;
+            p2m_type_t p2mt;
 
         case MMUEXT_PIN_L1_TABLE:
             type = PGT_l1_page_table;
@@ -3528,7 +3529,12 @@ long do_mmuext_op(
         }
 
         case MMUEXT_CLEAR_PAGE:
-            page = get_page_from_gfn(pg_owner, op.arg1.mfn, NULL, P2M_ALLOC);
+            page = get_page_from_gfn(pg_owner, op.arg1.mfn, &p2mt, P2M_ALLOC);
+            if ( unlikely(p2mt != p2m_ram_rw) && page )
+            {
+                put_page(page);
+                page = NULL;
+            }
             if ( !page || !get_page_type(page, PGT_writable_page) )
             {
                 if ( page )
@@ -3551,8 +3557,13 @@ long do_mmuext_op(
         {
             struct page_info *src_page, *dst_page;
 
-            src_page = get_page_from_gfn(pg_owner, op.arg2.src_mfn, NULL,
+            src_page = get_page_from_gfn(pg_owner, op.arg2.src_mfn, &p2mt,
                                          P2M_ALLOC);
+            if ( unlikely(p2mt != p2m_ram_rw) && src_page )
+            {
+                put_page(src_page);
+                src_page = NULL;
+            }
             if ( unlikely(!src_page) )
             {
                 gdprintk(XENLOG_WARNING,
@@ -3562,8 +3573,13 @@ long do_mmuext_op(
                 break;
             }
 
-            dst_page = get_page_from_gfn(pg_owner, op.arg1.mfn, NULL,
+            dst_page = get_page_from_gfn(pg_owner, op.arg1.mfn, &p2mt,
                                          P2M_ALLOC);
+            if ( unlikely(p2mt != p2m_ram_rw) && dst_page )
+            {
+                put_page(dst_page);
+                dst_page = NULL;
+            }
             rc = (dst_page &&
                   get_page_type(dst_page, PGT_writable_page)) ? 0 : -EINVAL;
             if ( unlikely(rc) )

[-- Attachment #3: Type: text/plain, Size: 127 bytes --]

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [PATCH] x86/mmuext: don't allow copying/clearing non-RAM pages
  2017-06-21 10:10 [PATCH] x86/mmuext: don't allow copying/clearing non-RAM pages Jan Beulich
@ 2017-06-21 10:51 ` Andrew Cooper
  2017-06-21 11:47   ` Jan Beulich
  0 siblings, 1 reply; 3+ messages in thread
From: Andrew Cooper @ 2017-06-21 10:51 UTC (permalink / raw)
  To: Jan Beulich, xen-devel

On 21/06/17 11:10, Jan Beulich wrote:
> The two operations really aren't meant for anything else.
>
> Signed-off-by: Jan Beulich <jbeulich@suse.com>

Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com>, however...

>
> --- a/xen/arch/x86/mm.c
> +++ b/xen/arch/x86/mm.c
> @@ -3229,6 +3229,7 @@ long do_mmuext_op(
>          switch ( op.cmd )
>          {
>              struct page_info *page;
> +            p2m_type_t p2mt;
>  
>          case MMUEXT_PIN_L1_TABLE:
>              type = PGT_l1_page_table;
> @@ -3528,7 +3529,12 @@ long do_mmuext_op(
>          }
>  
>          case MMUEXT_CLEAR_PAGE:
> -            page = get_page_from_gfn(pg_owner, op.arg1.mfn, NULL, P2M_ALLOC);
> +            page = get_page_from_gfn(pg_owner, op.arg1.mfn, &p2mt, P2M_ALLOC);
> +            if ( unlikely(p2mt != p2m_ram_rw) && page )

... it would seem more natural to have the null pointer check before the
p2mt check.

~Andrew

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [PATCH] x86/mmuext: don't allow copying/clearing non-RAM pages
  2017-06-21 10:51 ` Andrew Cooper
@ 2017-06-21 11:47   ` Jan Beulich
  0 siblings, 0 replies; 3+ messages in thread
From: Jan Beulich @ 2017-06-21 11:47 UTC (permalink / raw)
  To: Andrew Cooper; +Cc: xen-devel

>>> On 21.06.17 at 12:51, <andrew.cooper3@citrix.com> wrote:
> On 21/06/17 11:10, Jan Beulich wrote:
>> The two operations really aren't meant for anything else.
>>
>> Signed-off-by: Jan Beulich <jbeulich@suse.com>
> 
> Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com>, however...
> 
>>
>> --- a/xen/arch/x86/mm.c
>> +++ b/xen/arch/x86/mm.c
>> @@ -3229,6 +3229,7 @@ long do_mmuext_op(
>>          switch ( op.cmd )
>>          {
>>              struct page_info *page;
>> +            p2m_type_t p2mt;
>>  
>>          case MMUEXT_PIN_L1_TABLE:
>>              type = PGT_l1_page_table;
>> @@ -3528,7 +3529,12 @@ long do_mmuext_op(
>>          }
>>  
>>          case MMUEXT_CLEAR_PAGE:
>> -            page = get_page_from_gfn(pg_owner, op.arg1.mfn, NULL, P2M_ALLOC);
>> +            page = get_page_from_gfn(pg_owner, op.arg1.mfn, &p2mt, P2M_ALLOC);
>> +            if ( unlikely(p2mt != p2m_ram_rw) && page )
> 
> ... it would seem more natural to have the null pointer check before the
> p2mt check.

Since the checks are independent, the order doesn't really matter,
and with that it seemed better to put the unlikely() first (to get the
other check off the fast path).

Jan


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2017-06-21 11:47 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-06-21 10:10 [PATCH] x86/mmuext: don't allow copying/clearing non-RAM pages Jan Beulich
2017-06-21 10:51 ` Andrew Cooper
2017-06-21 11:47   ` Jan Beulich

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.