| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <20120416195258.GA14982@phenom.dumpdata.com>
Date: Mon, 16 Apr 2012 15:52:58 -0400
From: Konrad Rzeszutek Wilk <konrad.wilk@...cle.com>
To: Stefano Stabellini <stefano.stabellini@...citrix.com>
Cc: xen-devel@...ts.xensource.com, linux-kernel@...r.kernel.org
Subject: Re: [PATCH v3] xen-blkfront: set pages are FOREIGN_FRAME when
sharing them
On Tue, Apr 10, 2012 at 05:25:19PM +0100, Stefano Stabellini wrote:
> Set pages as FOREIGN_FRAME whenever blkfront shares them with another
> domain. Then when blkfront un-share them, also removes the
> FOREIGN_FRAME_BIT from the p2m.
>
> We do it so that when the source and the destination domain are the same
> (blkfront connected to blkback in the same domain) we can more easily
Hm, however you mention qdisk which does not use blkback?
> recognize which ones are the source pfns and which ones are the
> destination pfns (both are going to be pointing to the same mfns).
OK, so what happens if we do not do this?
>
> Without this patch enstablishing a connection between blkfront and QEMU
> qdisk in the same domain causes QEMU to hang and never return.
What is the reason behind it..?
Just to make sure I got it right:
The scenario is where a PV guest launches and inside it, it runs
QEMU exporting its disk (xvda, or some sda if iSCSI or FCoE) to some
other domain. In other words - a stubdomain, right? That would
imply that we have xen-blkfront, xen-blkback [or not?], and QEMU
using gntdev on the same page at some point.
So if we have a request from the other guest to read, it would be:
- QEMU allocates some pages.
- QEMU qdisk getting kicked.
- QEMU using gntdev to IOCTL_GNTDEV_MAP_GRANT_REF the appropiate
page
- gntdev ends up calling gnttab_map_refs. The gnttab_map_refs then
uses the m2p_add_override, which calls set_phys_to_machine(pfn, FOREIGN_FRAME(mfn)).
- QEMU passes the mentioned page to the kernel using the libaio.
- libaio does its syscall to sys_aio? which then maps the 'struct page'
in the kernel space. The PFN has FOREIGN_FRAME set.
- kernel aio code passes the 'struct page' to xen-blkfront.
- xen-blkfront now (with your patch) makes sure to re-stamp FOREIGN_FRAME on the
PFN.
- once xen-blkfront is done, it returns to libaio. libaio calls
QEMU and it calls gntdev to unmap. The grant dev does it, and calls
m2p_remove_override on the PFN [the one that had the FOREIGN_FRAME bit set].
Hm, I think I lost myself here. Is the case here that QEMU does not
do the IOCTL_GNTDEV_MAP_GRANT_REF so in reality the PFN that is provided
to xen-blkfront does not have FOREIGN_FRAME stamped?
>
>
> Changes in v3:
> - only set_phys_to_machine if xen_pv_domain.
>
>
> Signed-off-by: Stefano Stabellini <stefano.stabellini@...citrix.com>
> ---
> drivers/block/xen-blkfront.c | 33 ++++++++++++++++++++++++++++-----
> 1 files changed, 28 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/block/xen-blkfront.c b/drivers/block/xen-blkfront.c
> index 2f22874..cabfbbb 100644
> --- a/drivers/block/xen-blkfront.c
> +++ b/drivers/block/xen-blkfront.c
> @@ -262,7 +262,7 @@ static int blkif_ioctl(struct block_device *bdev, fmode_t mode,
> static int blkif_queue_request(struct request *req)
> {
> struct blkfront_info *info = req->rq_disk->private_data;
> - unsigned long buffer_mfn;
> + unsigned long buffer_mfn, buffer_pfn;
> struct blkif_request *ring_req;
> unsigned long id;
> unsigned int fsect, lsect;
> @@ -321,7 +321,8 @@ static int blkif_queue_request(struct request *req)
> BLKIF_MAX_SEGMENTS_PER_REQUEST);
>
> for_each_sg(info->sg, sg, ring_req->u.rw.nr_segments, i) {
> - buffer_mfn = pfn_to_mfn(page_to_pfn(sg_page(sg)));
> + buffer_pfn = page_to_pfn(sg_page(sg));
> + buffer_mfn = pfn_to_mfn(buffer_pfn);
> fsect = sg->offset >> 9;
> lsect = fsect + (sg->length >> 9) - 1;
> /* install a grant reference. */
> @@ -340,6 +341,17 @@ static int blkif_queue_request(struct request *req)
> .gref = ref,
> .first_sect = fsect,
> .last_sect = lsect };
> + /*
> + * Set the page as foreign, considering that we are giving
> + * it to a foreign domain.
> + * This is important in case the destination domain is
> + * ourselves, so that we can more easily recognize the
> + * source pfn from destination pfn, both mapping to the same
> + * mfn.
> + */
> + if (xen_pv_domain())
Should this also do the auto_translate bit check? Or do we not care
about that since the set_phys_to_machine does it for us?
> + set_phys_to_machine(buffer_pfn,
> + FOREIGN_FRAME(buffer_mfn));
> }
> }
>
> @@ -715,8 +727,12 @@ static void blkif_completion(struct blk_shadow *s)
> int i;
> /* Do not let BLKIF_OP_DISCARD as nr_segment is in the same place
> * flag. */
> - for (i = 0; i < s->req.u.rw.nr_segments; i++)
> + for (i = 0; i < s->req.u.rw.nr_segments; i++) {
> gnttab_end_foreign_access(s->req.u.rw.seg[i].gref, 0, 0UL);
> + if (xen_pv_domain())
> + set_phys_to_machine(s->frame[i],
> + get_phys_to_machine(s->frame[i]) & ~FOREIGN_FRAME_BIT);
> + }
> }
>
> static irqreturn_t blkif_interrupt(int irq, void *dev_id)
> @@ -1051,13 +1067,20 @@ static int blkif_recover(struct blkfront_info *info)
> memcpy(&info->shadow[req->u.rw.id], ©[i], sizeof(copy[i]));
>
> if (req->operation != BLKIF_OP_DISCARD) {
> + unsigned long buffer_pfn;
> + unsigned long buffer_mfn;
> /* Rewrite any grant references invalidated by susp/resume. */
> - for (j = 0; j < req->u.rw.nr_segments; j++)
> + for (j = 0; j < req->u.rw.nr_segments; j++) {
> + buffer_pfn = info->shadow[req->u.rw.id].frame[j];
> + buffer_mfn = pfn_to_mfn(buffer_pfn);
> gnttab_grant_foreign_access_ref(
> req->u.rw.seg[j].gref,
> info->xbdev->otherend_id,
> - pfn_to_mfn(info->shadow[req->u.rw.id].frame[j]),
> + buffer_mfn,
> rq_data_dir(info->shadow[req->u.rw.id].request));
> + if (xen_pv_domain())
> + set_phys_to_machine(buffer_pfn, FOREIGN_FRAME(buffer_mfn));
> + }
> }
> info->shadow[req->u.rw.id].req = *req;
>
> --
> 1.7.2.5
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Powered by blists - more mailing lists