>>> On 12.04.19 at 15:41, <ta...@tklengyel.com> wrote: > On Fri, Apr 12, 2019 at 5:55 AM Jan Beulich <jbeul...@suse.com> wrote: >> >>> On 12.04.19 at 06:29, <ta...@tklengyel.com> wrote: >> > @@ -984,7 +976,7 @@ static int share_pages(struct domain *sd, gfn_t sgfn, >> > shr_handle_t sh, >> > * Don't change the type of rmap for the client page. */ >> > rmap_del(gfn, cpage, 0); >> > rmap_add(gfn, spage); >> > - put_page_and_type(cpage); >> > + put_count++; >> >> Since this sits in a while(), ... >> >> > @@ -1002,7 +994,9 @@ static int share_pages(struct domain *sd, gfn_t sgfn, >> > shr_handle_t sh, >> > /* Free the client page */ >> > if(test_and_clear_bit(_PGC_allocated, &cpage->count_info)) >> > put_page(cpage); >> > - put_page(cpage); >> > + >> > + while(put_count--) >> > + put_page_and_type(cpage); >> >> ... put_count may be zero before this while(). At which point the >> earlier put_page() would still be unsafe. > > The page should have at least one reference it got before when the > page went through page_make_sharable function. We also verified that > the page is still sharable so that reference is still there, so this > count is at least 1. We could certainly add an ASSERT(put_count) > before though.
I see - back when I created the change you're now trying to fix, I was wondering but couldn't find any proof. So yes, adding an ASSERT() would surely be helpful. Jan _______________________________________________ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
