linux-kernel - Re: [Xen-devel] [PATCH 3/3] xen-blkfront: revoke foreign access for grants not mapped by the backend

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

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:	Fri, 9 Aug 2013 11:08:37 -0400
From:	Konrad Rzeszutek Wilk <konrad.wilk@...cle.com>
To:	David Vrabel <david.vrabel@...rix.com>, roger.pau@...rix.com
Cc:	Roger Pau Monne <roger.pau@...rix.com>,
	linux-kernel@...r.kernel.org, xen-devel@...ts.xen.org,
	boris.ostrovsky@...cle.com
Subject: Re: [Xen-devel] [PATCH 3/3] xen-blkfront: revoke foreign access for
 grants not mapped by the backend

On Thu, Aug 01, 2013 at 11:10:15AM +0100, David Vrabel wrote:
> On 31/07/13 16:00, Roger Pau Monne wrote:
> > There's no need to keep the foreign access in a grant if it is not
> > persistently mapped by the backend. This allows us to free grants that
> > are not mapped by the backend, thus preventing blkfront from hoarding
> > all grants.
> > 
> > The main effect of this is that blkfront will only persistently map
> > the same grants as the backend, and it will always try to use grants
> > that are already mapped by the backend. Also the number of persistent
> > grants in blkfront is the same as in blkback (and is controlled by the
> > value in blkback).
> > 
> > Signed-off-by: Roger Pau Monné <roger.pau@...rix.com>
> 
> Reviewed-by: David Vrabel <david.vrabel@...rix.com>

Roger,

Could you repost patch #2 and #3 (as #1 is in v3.11-rc4) with the
comments and the Ack from Matt and Roger's Review-by tag addressed?

Thanks.
> 
> But please see the documentation updates needed below.
> 
> > --- a/drivers/block/xen-blkfront.c
> > +++ b/drivers/block/xen-blkfront.c
> > @@ -1016,13 +1016,38 @@ static void blkif_completion(struct blk_shadow *s, struct blkfront_info *info,
> >  	}
> >  	/* Add the persistent grant into the list of free grants */
> >  	for (i = 0; i < nseg; i++) {
> > -		list_add(&s->grants_used[i]->node, &info->persistent_gnts);
> > -		info->persistent_gnts_c++;
> > +		if (gnttab_query_foreign_access(s->grants_used[i]->gref)) {
> > +			/*
> > +			 * If the grant is still mapped by the backend (the
> > +			 * backend has chosen to make this grant persistent)
> > +			 * we add it at the head of the list, so it will be
> > +			 * reused first.
> > +			 */
> > +			list_add(&s->grants_used[i]->node, &info->persistent_gnts);
> > +			info->persistent_gnts_c++;
> > +		} else {
> > +			/*
> > +			 * If the grant is not mapped by the backend we end the
> > +			 * foreign access and add it to the tail of the list,
> > +			 * so it will not be picked again unless we run out of
> > +			 * persistent grants.
> > +			 */
> > +			gnttab_end_foreign_access(s->grants_used[i]->gref, 0, 0UL);
> > +			s->grants_used[i]->gref = GRANT_INVALID_REF;
> > +			list_add_tail(&s->grants_used[i]->node, &info->persistent_gnts);
> > +		}
> 
> Because you only reclaim grants when a request is completed, can you add
> text similar to the following to xen/include/public/io/blkif.h (and the
> Linux copy).
> 
> feature-persistent:
> ...
>    When the backend driver needs to unmap a persistent grant it should
>    do so prior to completing a request that used that grant reference.
>    If a persistent grant is unmapped at any other time, the frontend
>    driver may not notice and may be unable to reclaim the grant
>    reference.
> 
> Thanks.
> 
> David
> 
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@...ts.xen.org
> http://lists.xen.org/xen-devel
--
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/