[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Xen-devel] [PATCH v9 3/4] mm: make pages allocated with MEMF_no_refcount safe to assign
 
- To: Jan Beulich <jbeulich@xxxxxxxx>
 
- From: Julien Grall <julien@xxxxxxx>
 
- Date: Thu, 6 Feb 2020 14:30:07 +0000
 
- Cc: Stefano Stabellini <sstabellini@xxxxxxxxxx>, Wei Liu <wl@xxxxxxx>, Konrad Rzeszutek Wilk <konrad.wilk@xxxxxxxxxx>, George Dunlap <George.Dunlap@xxxxxxxxxxxxx>, Andrew Cooper <andrew.cooper3@xxxxxxxxxx>, Ian Jackson <ian.jackson@xxxxxxxxxxxxx>, "Durrant, Paul" <pdurrant@xxxxxxxxxxxx>, "xen-devel@xxxxxxxxxxxxxxxxxxxx" <xen-devel@xxxxxxxxxxxxxxxxxxxx>, Volodymyr Babchuk <Volodymyr_Babchuk@xxxxxxxx>, Roger Pau Monné <roger.pau@xxxxxxxxxx>
 
- Delivery-date: Thu, 06 Feb 2020 14:30:12 +0000
 
- List-id: Xen developer discussion <xen-devel.lists.xenproject.org>
 
 
 
On 06/02/2020 11:43, Jan Beulich wrote:
 
On 06.02.2020 11:12, Durrant, Paul wrote:
 
From: Julien Grall <julien@xxxxxxx>
Sent: 06 February 2020 10:04
On 03/02/2020 10:56, Paul Durrant wrote:
 
@@ -2332,11 +2350,23 @@ struct page_info *alloc_domheap_pages(
                                     memflags, d)) == NULL)) )
            return NULL;
-    if ( d && !(memflags & MEMF_no_owner) &&
-         assign_pages(d, pg, order, memflags) )
+    if ( d && !(memflags & MEMF_no_owner) )
       {
-        free_heap_pages(pg, order, memflags & MEMF_no_scrub);
-        return NULL;
+        if ( memflags & MEMF_no_refcount )
+        {
+            unsigned long i;
+
+            for ( i = 0; i < (1ul << order); i++ )
+            {
+                ASSERT(!pg[i].count_info);
+                pg[i].count_info = PGC_extra;
 
... this is pursuing the wrongness of the code above and not safe
against offlining.
We could argue this is an already existing bug, however I am a bit
unease to add more abuse in the code. Jan, what do you think?
 
 
I'd consider a straightforward patch-clash. If this patch goes in
after yours then it needs to be modified accordingly, or vice versa.
 
 
While generally I advocate for not widening existing issues, I agree
with Paul here. His patch should not be penalized by us _later_
having found an issue (which is quite a bit wider).
 
 
Fair enough. For the Arm bits:
Acked-by: Julien Grall <julien@xxxxxxx>
Cheers,
--
Julien Grall
_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/xen-devel 
 
    
     |