* [PATCH] xen: privcmd: do not return pages which we have failed to unmap
@ 2013-12-04 15:52 Ian Campbell
2013-12-04 15:58 ` David Vrabel
0 siblings, 1 reply; 7+ messages in thread
From: Ian Campbell @ 2013-12-04 15:52 UTC (permalink / raw)
To: xen-devel
Cc: Ian Campbell, Stefano Stabellini, David Vrabel, xen-devel,
Boris Ostrovsky
This failure represents a hypervisor issue, but if it does occur then nothing
good can come of returning pages which still refer to a foreign owned page
into the general allocation pool.
Instead we are foced to leak them. Log that we have done so.
The potential for failure only exists for autotranslated guest (e.g. ARM and
x86 PVH).
Signed-off-by: Ian Campbell <ian.campbell@citrix.com>
Cc: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
Cc: Boris Ostrovsky <boris.ostrovsky@oracle.com>
Cc: David Vrabel <david.vrabel@citrix.com>
Cc: Stefano Stabellini <stefano.stabellini@eu.citrix.com>
Cc: xen-devel@lists.xenproject.org
---
drivers/xen/privcmd.c | 8 +++++++-
1 file changed, 7 insertions(+), 1 deletion(-)
diff --git a/drivers/xen/privcmd.c b/drivers/xen/privcmd.c
index 8e74590..2efc720 100644
--- a/drivers/xen/privcmd.c
+++ b/drivers/xen/privcmd.c
@@ -533,11 +533,17 @@ static void privcmd_close(struct vm_area_struct *vma)
{
struct page **pages = vma->vm_private_data;
int numpgs = (vma->vm_end - vma->vm_start) >> PAGE_SHIFT;
+ int rc;
if (!xen_feature(XENFEAT_auto_translated_physmap) || !numpgs || !pages)
return;
- xen_unmap_domain_mfn_range(vma, numpgs, pages);
+ rc = xen_unmap_domain_mfn_range(vma, numpgs, pages);
+ if (rc < 0) {
+ pr_crit("unable to unmap MFN range: leaking %d pages\n",
+ numpgs);
+ return;
+ }
free_xenballooned_pages(numpgs, pages);
kfree(pages);
}
--
1.7.10.4
^ permalink raw reply related [flat|nested] 7+ messages in thread* Re: [PATCH] xen: privcmd: do not return pages which we have failed to unmap
2013-12-04 15:52 [PATCH] xen: privcmd: do not return pages which we have failed to unmap Ian Campbell
@ 2013-12-04 15:58 ` David Vrabel
2013-12-04 16:03 ` Ian Campbell
0 siblings, 1 reply; 7+ messages in thread
From: David Vrabel @ 2013-12-04 15:58 UTC (permalink / raw)
To: Ian Campbell; +Cc: xen-devel, Boris Ostrovsky, Stefano Stabellini, xen-devel
On 04/12/13 15:52, Ian Campbell wrote:
> This failure represents a hypervisor issue, but if it does occur then nothing
> good can come of returning pages which still refer to a foreign owned page
> into the general allocation pool.
>
> Instead we are foced to leak them. Log that we have done so.
^forced
>
> The potential for failure only exists for autotranslated guest (e.g. ARM and
> x86 PVH).
[...]
> --- a/drivers/xen/privcmd.c
> +++ b/drivers/xen/privcmd.c
> @@ -533,11 +533,17 @@ static void privcmd_close(struct vm_area_struct *vma)
> {
> struct page **pages = vma->vm_private_data;
> int numpgs = (vma->vm_end - vma->vm_start) >> PAGE_SHIFT;
> + int rc;
>
> if (!xen_feature(XENFEAT_auto_translated_physmap) || !numpgs || !pages)
> return;
>
> - xen_unmap_domain_mfn_range(vma, numpgs, pages);
> + rc = xen_unmap_domain_mfn_range(vma, numpgs, pages);
> + if (rc < 0) {
> + pr_crit("unable to unmap MFN range: leaking %d pages\n",
> + numpgs);
kfree(pages) here? I think that would be safe. Although at this point
it probably doesn't really matter.
David
^ permalink raw reply [flat|nested] 7+ messages in thread* Re: [PATCH] xen: privcmd: do not return pages which we have failed to unmap
2013-12-04 15:58 ` David Vrabel
@ 2013-12-04 16:03 ` Ian Campbell
2013-12-04 16:12 ` David Vrabel
2013-12-06 17:58 ` Stefano Stabellini
0 siblings, 2 replies; 7+ messages in thread
From: Ian Campbell @ 2013-12-04 16:03 UTC (permalink / raw)
To: David Vrabel; +Cc: xen-devel, Boris Ostrovsky, Stefano Stabellini, xen-devel
> > - xen_unmap_domain_mfn_range(vma, numpgs, pages);
> > + rc = xen_unmap_domain_mfn_range(vma, numpgs, pages);
> > + if (rc < 0) {
> > + pr_crit("unable to unmap MFN range: leaking %d pages\n",
> > + numpgs);
>
> kfree(pages) here? I think that would be safe. Although at this point
> it probably doesn't really matter.
I suppose we might as well not make it any worse than it needs to be and
it's easy enough to arrange.
---------------------8<---------------------
>From 900f1e903bacf376800b078aef03e8d5ff524562 Mon Sep 17 00:00:00 2001
From: Ian Campbell <ian.campbell@citrix.com>
Date: Wed, 4 Dec 2013 14:19:52 +0000
Subject: [PATCH] xen: privcmd: do not return pages which we have failed to
unmap
This failure represents a hypervisor issue, but if it does occur then nothing
good can come of returning pages which still refer to a foreign owned page
into the general allocation pool.
Instead we are forced to leak them. Log that we have done so.
The potential for failure only exists for autotranslated guest (e.g. ARM and
x86 PVH).
Signed-off-by: Ian Campbell <ian.campbell@citrix.com>
Cc: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
Cc: Boris Ostrovsky <boris.ostrovsky@oracle.com>
Cc: David Vrabel <david.vrabel@citrix.com>
Cc: Stefano Stabellini <stefano.stabellini@eu.citrix.com>
Cc: xen-devel@lists.xenproject.org
---
v2: Don't leak the actual pages array as well
Log rc
---
drivers/xen/privcmd.c | 9 +++++++--
1 file changed, 7 insertions(+), 2 deletions(-)
diff --git a/drivers/xen/privcmd.c b/drivers/xen/privcmd.c
index 8e74590..569a13b 100644
--- a/drivers/xen/privcmd.c
+++ b/drivers/xen/privcmd.c
@@ -533,12 +533,17 @@ static void privcmd_close(struct vm_area_struct *vma)
{
struct page **pages = vma->vm_private_data;
int numpgs = (vma->vm_end - vma->vm_start) >> PAGE_SHIFT;
+ int rc;
if (!xen_feature(XENFEAT_auto_translated_physmap) || !numpgs || !pages)
return;
- xen_unmap_domain_mfn_range(vma, numpgs, pages);
- free_xenballooned_pages(numpgs, pages);
+ rc = xen_unmap_domain_mfn_range(vma, numpgs, pages);
+ if (rc == 0)
+ free_xenballooned_pages(numpgs, pages);
+ else
+ pr_crit("unable to unmap MFN range: leaking %d pages. rc=%d\n",
+ numpgs, rc);
kfree(pages);
}
--
1.7.10.4
^ permalink raw reply related [flat|nested] 7+ messages in thread* Re: [PATCH] xen: privcmd: do not return pages which we have failed to unmap
2013-12-04 16:03 ` Ian Campbell
@ 2013-12-04 16:12 ` David Vrabel
2013-12-06 17:58 ` Stefano Stabellini
1 sibling, 0 replies; 7+ messages in thread
From: David Vrabel @ 2013-12-04 16:12 UTC (permalink / raw)
To: Ian Campbell; +Cc: xen-devel, Boris Ostrovsky, Stefano Stabellini
On 04/12/13 16:03, Ian Campbell wrote:
> Subject: [PATCH] xen: privcmd: do not return pages which we have failed to
> unmap
>
> This failure represents a hypervisor issue, but if it does occur then nothing
> good can come of returning pages which still refer to a foreign owned page
> into the general allocation pool.
>
> Instead we are forced to leak them. Log that we have done so.
>
> The potential for failure only exists for autotranslated guest (e.g. ARM and
> x86 PVH).
Reviewed-by: David Vrabel <david.vrabel@citrix.com>
David
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] xen: privcmd: do not return pages which we have failed to unmap
2013-12-04 16:03 ` Ian Campbell
2013-12-04 16:12 ` David Vrabel
@ 2013-12-06 17:58 ` Stefano Stabellini
2013-12-06 20:08 ` Konrad Rzeszutek Wilk
1 sibling, 1 reply; 7+ messages in thread
From: Stefano Stabellini @ 2013-12-06 17:58 UTC (permalink / raw)
To: Ian Campbell
Cc: Stefano Stabellini, xen-devel, David Vrabel, xen-devel,
Boris Ostrovsky
On Wed, 4 Dec 2013, Ian Campbell wrote:
> >From 900f1e903bacf376800b078aef03e8d5ff524562 Mon Sep 17 00:00:00 2001
> From: Ian Campbell <ian.campbell@citrix.com>
> Date: Wed, 4 Dec 2013 14:19:52 +0000
> Subject: [PATCH] xen: privcmd: do not return pages which we have failed to
> unmap
>
> This failure represents a hypervisor issue, but if it does occur then nothing
> good can come of returning pages which still refer to a foreign owned page
> into the general allocation pool.
>
> Instead we are forced to leak them. Log that we have done so.
>
> The potential for failure only exists for autotranslated guest (e.g. ARM and
> x86 PVH).
>
> Signed-off-by: Ian Campbell <ian.campbell@citrix.com>
> Cc: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
> Cc: Boris Ostrovsky <boris.ostrovsky@oracle.com>
> Cc: David Vrabel <david.vrabel@citrix.com>
> Cc: Stefano Stabellini <stefano.stabellini@eu.citrix.com>
> Cc: xen-devel@lists.xenproject.org
> ---
> v2: Don't leak the actual pages array as well
> Log rc
Acked-by: Stefano Stabellini <stefano.stabellini@eu.citrix.com>
Konrad, you can go ahead and add it to xentip, unless you would rather
have me do it.
> drivers/xen/privcmd.c | 9 +++++++--
> 1 file changed, 7 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/xen/privcmd.c b/drivers/xen/privcmd.c
> index 8e74590..569a13b 100644
> --- a/drivers/xen/privcmd.c
> +++ b/drivers/xen/privcmd.c
> @@ -533,12 +533,17 @@ static void privcmd_close(struct vm_area_struct *vma)
> {
> struct page **pages = vma->vm_private_data;
> int numpgs = (vma->vm_end - vma->vm_start) >> PAGE_SHIFT;
> + int rc;
>
> if (!xen_feature(XENFEAT_auto_translated_physmap) || !numpgs || !pages)
> return;
>
> - xen_unmap_domain_mfn_range(vma, numpgs, pages);
> - free_xenballooned_pages(numpgs, pages);
> + rc = xen_unmap_domain_mfn_range(vma, numpgs, pages);
> + if (rc == 0)
> + free_xenballooned_pages(numpgs, pages);
> + else
> + pr_crit("unable to unmap MFN range: leaking %d pages. rc=%d\n",
> + numpgs, rc);
> kfree(pages);
> }
>
> --
> 1.7.10.4
>
>
>
^ permalink raw reply [flat|nested] 7+ messages in thread* Re: [PATCH] xen: privcmd: do not return pages which we have failed to unmap
2013-12-06 17:58 ` Stefano Stabellini
@ 2013-12-06 20:08 ` Konrad Rzeszutek Wilk
2013-12-06 20:43 ` Stefano Stabellini
0 siblings, 1 reply; 7+ messages in thread
From: Konrad Rzeszutek Wilk @ 2013-12-06 20:08 UTC (permalink / raw)
To: Stefano Stabellini
Cc: xen-devel, Boris Ostrovsky, xen-devel, Ian Campbell, David Vrabel
On Fri, Dec 06, 2013 at 05:58:25PM +0000, Stefano Stabellini wrote:
> On Wed, 4 Dec 2013, Ian Campbell wrote:
> > >From 900f1e903bacf376800b078aef03e8d5ff524562 Mon Sep 17 00:00:00 2001
> > From: Ian Campbell <ian.campbell@citrix.com>
> > Date: Wed, 4 Dec 2013 14:19:52 +0000
> > Subject: [PATCH] xen: privcmd: do not return pages which we have failed to
> > unmap
> >
> > This failure represents a hypervisor issue, but if it does occur then nothing
> > good can come of returning pages which still refer to a foreign owned page
> > into the general allocation pool.
> >
> > Instead we are forced to leak them. Log that we have done so.
> >
> > The potential for failure only exists for autotranslated guest (e.g. ARM and
> > x86 PVH).
> >
> > Signed-off-by: Ian Campbell <ian.campbell@citrix.com>
> > Cc: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
> > Cc: Boris Ostrovsky <boris.ostrovsky@oracle.com>
> > Cc: David Vrabel <david.vrabel@citrix.com>
> > Cc: Stefano Stabellini <stefano.stabellini@eu.citrix.com>
> > Cc: xen-devel@lists.xenproject.org
> > ---
> > v2: Don't leak the actual pages array as well
> > Log rc
>
>
> Acked-by: Stefano Stabellini <stefano.stabellini@eu.citrix.com>
>
> Konrad, you can go ahead and add it to xentip, unless you would rather
> have me do it.
Why don't you do it. Thanks!
>
>
> > drivers/xen/privcmd.c | 9 +++++++--
> > 1 file changed, 7 insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/xen/privcmd.c b/drivers/xen/privcmd.c
> > index 8e74590..569a13b 100644
> > --- a/drivers/xen/privcmd.c
> > +++ b/drivers/xen/privcmd.c
> > @@ -533,12 +533,17 @@ static void privcmd_close(struct vm_area_struct *vma)
> > {
> > struct page **pages = vma->vm_private_data;
> > int numpgs = (vma->vm_end - vma->vm_start) >> PAGE_SHIFT;
> > + int rc;
> >
> > if (!xen_feature(XENFEAT_auto_translated_physmap) || !numpgs || !pages)
> > return;
> >
> > - xen_unmap_domain_mfn_range(vma, numpgs, pages);
> > - free_xenballooned_pages(numpgs, pages);
> > + rc = xen_unmap_domain_mfn_range(vma, numpgs, pages);
> > + if (rc == 0)
> > + free_xenballooned_pages(numpgs, pages);
> > + else
> > + pr_crit("unable to unmap MFN range: leaking %d pages. rc=%d\n",
> > + numpgs, rc);
> > kfree(pages);
> > }
> >
> > --
> > 1.7.10.4
> >
> >
> >
^ permalink raw reply [flat|nested] 7+ messages in thread* Re: [PATCH] xen: privcmd: do not return pages which we have failed to unmap
2013-12-06 20:08 ` Konrad Rzeszutek Wilk
@ 2013-12-06 20:43 ` Stefano Stabellini
0 siblings, 0 replies; 7+ messages in thread
From: Stefano Stabellini @ 2013-12-06 20:43 UTC (permalink / raw)
To: Konrad Rzeszutek Wilk
Cc: Ian Campbell, Stefano Stabellini, xen-devel, David Vrabel,
xen-devel, Boris Ostrovsky
On Fri, 6 Dec 2013, Konrad Rzeszutek Wilk wrote:
> On Fri, Dec 06, 2013 at 05:58:25PM +0000, Stefano Stabellini wrote:
> > On Wed, 4 Dec 2013, Ian Campbell wrote:
> > > >From 900f1e903bacf376800b078aef03e8d5ff524562 Mon Sep 17 00:00:00 2001
> > > From: Ian Campbell <ian.campbell@citrix.com>
> > > Date: Wed, 4 Dec 2013 14:19:52 +0000
> > > Subject: [PATCH] xen: privcmd: do not return pages which we have failed to
> > > unmap
> > >
> > > This failure represents a hypervisor issue, but if it does occur then nothing
> > > good can come of returning pages which still refer to a foreign owned page
> > > into the general allocation pool.
> > >
> > > Instead we are forced to leak them. Log that we have done so.
> > >
> > > The potential for failure only exists for autotranslated guest (e.g. ARM and
> > > x86 PVH).
> > >
> > > Signed-off-by: Ian Campbell <ian.campbell@citrix.com>
> > > Cc: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
> > > Cc: Boris Ostrovsky <boris.ostrovsky@oracle.com>
> > > Cc: David Vrabel <david.vrabel@citrix.com>
> > > Cc: Stefano Stabellini <stefano.stabellini@eu.citrix.com>
> > > Cc: xen-devel@lists.xenproject.org
> > > ---
> > > v2: Don't leak the actual pages array as well
> > > Log rc
> >
> >
> > Acked-by: Stefano Stabellini <stefano.stabellini@eu.citrix.com>
> >
> > Konrad, you can go ahead and add it to xentip, unless you would rather
> > have me do it.
>
> Why don't you do it. Thanks!
done
> >
> > > drivers/xen/privcmd.c | 9 +++++++--
> > > 1 file changed, 7 insertions(+), 2 deletions(-)
> > >
> > > diff --git a/drivers/xen/privcmd.c b/drivers/xen/privcmd.c
> > > index 8e74590..569a13b 100644
> > > --- a/drivers/xen/privcmd.c
> > > +++ b/drivers/xen/privcmd.c
> > > @@ -533,12 +533,17 @@ static void privcmd_close(struct vm_area_struct *vma)
> > > {
> > > struct page **pages = vma->vm_private_data;
> > > int numpgs = (vma->vm_end - vma->vm_start) >> PAGE_SHIFT;
> > > + int rc;
> > >
> > > if (!xen_feature(XENFEAT_auto_translated_physmap) || !numpgs || !pages)
> > > return;
> > >
> > > - xen_unmap_domain_mfn_range(vma, numpgs, pages);
> > > - free_xenballooned_pages(numpgs, pages);
> > > + rc = xen_unmap_domain_mfn_range(vma, numpgs, pages);
> > > + if (rc == 0)
> > > + free_xenballooned_pages(numpgs, pages);
> > > + else
> > > + pr_crit("unable to unmap MFN range: leaking %d pages. rc=%d\n",
> > > + numpgs, rc);
> > > kfree(pages);
> > > }
> > >
> > > --
> > > 1.7.10.4
> > >
> > >
> > >
>
^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2013-12-06 20:43 UTC | newest]
Thread overview: 7+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2013-12-04 15:52 [PATCH] xen: privcmd: do not return pages which we have failed to unmap Ian Campbell
2013-12-04 15:58 ` David Vrabel
2013-12-04 16:03 ` Ian Campbell
2013-12-04 16:12 ` David Vrabel
2013-12-06 17:58 ` Stefano Stabellini
2013-12-06 20:08 ` Konrad Rzeszutek Wilk
2013-12-06 20:43 ` Stefano Stabellini
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).