Re: port-xen/47057: Xen NetBSD DomU file system trash under Linux Dom0

[Manuel Bouyer <bouyer%antioche.eu.org@localhost>]

The following reply was made to PR port-xen/47057; it has been noted by GNATS.

From: Manuel Bouyer <bouyer%antioche.eu.org@localhost>
To: Roger Pau =?iso-8859-1?Q?Monn=E9?= <roger.pau%citrix.com@localhost>
Cc: Roger Pau =?iso-8859-1?Q?Monn=E9?= <royger%NetBSD.org@localhost>,
        "gnats-bugs%NetBSD.org@localhost" <gnats-bugs%NetBSD.org@localhost>,
        "port-xen-maintainer%netbsd.org@localhost" 
<port-xen-maintainer%NetBSD.org@localhost>,
        "gnats-admin%netbsd.org@localhost" <gnats-admin%NetBSD.org@localhost>,
        "netbsd-bugs%netbsd.org@localhost" <netbsd-bugs%NetBSD.org@localhost>
Subject: Re: port-xen/47057: Xen NetBSD DomU file system trash under Linux
 Dom0
Date: Tue, 23 Oct 2012 21:57:48 +0200

 On Tue, Oct 23, 2012 at 04:01:06PM +0200, Roger Pau Monné wrote:
 > Found the problem, grants from 0 to 8 (both included), shouldn't be
 > used, they are reserved for the tools. I guess thats xenstore,
 > xenconsole and friends, so that's where the corruption came from, and
 > that's why the problem seemed to be related to xengnt_more_entries,
 > because it gets called when those low grants are used. The attached
 > patch solves the problem for me.
 
 I guess it's new behavior of the tools ? Otherwise I guess we should have hit
 this sooner. I see messages saying the kernel grows the grant entries
 pool on a regular basis.
 
 Anyway, good catch. one comment about the patch below.
 
 
 > >From b80f10a3c3d0b95d3cd2a60a4669a2118fdbb9ef Mon Sep 17 00:00:00 2001
 > From: Roger Pau Monne <roger.pau%citrix.com@localhost>
 > Date: Tue, 23 Oct 2012 15:21:18 +0200
 > Subject: [PATCH] xen: don't use grants 0-9
 > 
 > Not all grants from the first frame can be used, grants from 0 to 8
 > (both included) are reserved for external tools. Using this grants
 > caused system crashes and fs corruption.
 > ---
 >  sys/arch/xen/xen/xengnt.c |   15 +++++++++++----
 >  1 files changed, 11 insertions(+), 4 deletions(-)
 > 
 > diff --git a/sys/arch/xen/xen/xengnt.c b/sys/arch/xen/xen/xengnt.c
 > index 621d2dc..2de4fd3 100644
 > --- a/sys/arch/xen/xen/xengnt.c
 > +++ b/sys/arch/xen/xen/xengnt.c
 > @@ -51,6 +51,9 @@ __KERNEL_RCSID(0, "$NetBSD: xengnt.c,v 1.24 2012/06/30 
 > 23:36:20 jym Exp $");
 >  
 >  #define NR_GRANT_ENTRIES_PER_PAGE (PAGE_SIZE / sizeof(grant_entry_t))
 >  
 > +/* External tools reserve first few grant table entries. */
 > +#define NR_RESERVED_ENTRIES 8
 > +
 >  /* Current number of frames making up the grant table */
 >  int gnt_nr_grant_frames;
 >  /* Maximum number of frames that can make up the grant table */
 > @@ -161,7 +164,7 @@ xengnt_more_entries(void)
 >      gnttab_setup_table_t setup;
 >      u_long *pages;
 >      int nframes_new = gnt_nr_grant_frames + 1;
 > -    int i;
 > +    int i, start_gnt;
 >      KASSERT(mutex_owned(&grant_lock));
 >  
 >      if (gnt_nr_grant_frames == gnt_max_grant_frames)
 > @@ -204,9 +207,13 @@ xengnt_more_entries(void)
 >  
 >      /*
 >       * add the grant entries associated to the last grant table frame
 > -     * and mark them as free
 > +     * and mark them as free. Prevent using the first grants (from 0 to 8)
 > +     * since they are used by the tools.
 >       */
 > -    for (i = gnt_nr_grant_frames * NR_GRANT_ENTRIES_PER_PAGE;
 > +    start_gnt = (gnt_nr_grant_frames * NR_GRANT_ENTRIES_PER_PAGE) <
 > +                            NR_RESERVED_ENTRIES + 1 ? NR_RESERVED_ENTRIES + 
 > 1 :
 > +                            (gnt_nr_grant_frames * 
 > NR_GRANT_ENTRIES_PER_PAGE);
 
 please rewrite with parenthesis:
 +      start_gnt = (gnt_nr_grant_frames * NR_GRANT_ENTRIES_PER_PAGE) <
 +                              (NR_RESERVED_ENTRIES + 1) ?
                                (NR_RESERVED_ENTRIES + 1) :
 +                              (gnt_nr_grant_frames * 
NR_GRANT_ENTRIES_PER_PAGE);
 
 then please commit and request pullups for netbsd-5 and -6.
 
 -- 
 Manuel Bouyer <bouyer%antioche.eu.org@localhost>
      NetBSD: 26 ans d'experience feront toujours la difference
 --