Received-SPF: pass (google.com: domain of linux-nfs-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
Date:   Thu, 25 Mar 2021 16:09:26 -0400
From:   "J. Bruce Fields" <bfields@fieldses.org>
To:     Ondrej Mosnacek <omosnace@redhat.com>
Cc:     Steve Dickson <steved@redhat.com>,
        linux-nfs <linux-nfs@vger.kernel.org>
Subject: Re: [PATCH] exportfs: fix unexporting of '/'
Message-ID: <20210325200926.GB18351@fieldses.org>
References: <20210322210238.96915-1-omosnace@redhat.com>
 <20210325191903.GA18351@fieldses.org>
 <CAFqZXNsg7XTEbzxEz+7oO9DLFTu=6-LzwXJpOdTZTzj5Td_2Ng@mail.gmail.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <CAFqZXNsg7XTEbzxEz+7oO9DLFTu=6-LzwXJpOdTZTzj5Td_2Ng@mail.gmail.com>
User-Agent: Mutt/1.5.21 (2010-09-15)
Precedence: bulk

On Thu, Mar 25, 2021 at 08:48:57PM +0100, Ondrej Mosnacek wrote:
> On Thu, Mar 25, 2021 at 8:27 PM J. Bruce Fields <bfields@fieldses.org> wrote:
> > On Mon, Mar 22, 2021 at 10:02:38PM +0100, Ondrej Mosnacek wrote:
> > > The code that has been added to strip trailing slashes from path in
> > > unexportfs_parsed() forgot to account for the case of the root
> > > directory, which is simply '/'. In that case it accesses path[-1] and
> > > reduces the path to an empty string, which then fails to match any
> > > export.
> > >
> > > Fix it by stopping the stripping when the path is just a single
> > > character - it doesn't matter if it's a '/' or not, we want to keep it
> > > either way in that case.
> >
> > Makes sense to me.
> >
> > (Note nfs-exporting / is often a bad idea.  I assume you had some good
> > reason in this case.)
> 
> I hit it in a test, so if the only issue is that it exposes more than
> necessary, then I guess it's fine in this case :)

Yes, in a lot of typical setups, exporting "/" would expose files to the
network that you don't really want to.

But, anyway, it should work.  Looks like a good fix.

--b.

> 
> >
> > --b.
> >
> > >
> > > Reproducer:
> > >
> > >     exportfs localhost:/
> > >     exportfs -u localhost:/
> > >
> > > Without this patch, the unexport step fails with "exportfs: Could not
> > > find 'localhost:/' to unexport."
> > >
> > > Fixes: a9a7728d8743 ("exportfs: Deal with path's trailing "/" in unexportfs_parsed()")
> > > Link: https://bugzilla.redhat.com/show_bug.cgi?id=1941171
> > > Signed-off-by: Ondrej Mosnacek <omosnace@redhat.com>
> > > ---
> > >  utils/exportfs/exportfs.c | 2 +-
> > >  1 file changed, 1 insertion(+), 1 deletion(-)
> > >
> > > diff --git a/utils/exportfs/exportfs.c b/utils/exportfs/exportfs.c
> > > index 262dd19a..1aedd3d6 100644
> > > --- a/utils/exportfs/exportfs.c
> > > +++ b/utils/exportfs/exportfs.c
> > > @@ -383,7 +383,7 @@ unexportfs_parsed(char *hname, char *path, int verbose)
> > >        * so need to deal with it.
> > >       */
> > >       size_t nlen = strlen(path);
> > > -     while (path[nlen - 1] == '/')
> > > +     while (nlen > 1 && path[nlen - 1] == '/')
> > >               nlen--;
> > >
> > >       for (exp = exportlist[htype].p_head; exp; exp = exp->m_next) {
> > > --
> > > 2.30.2
> >
> 
> 
> --
> Ondrej Mosnacek
> Software Engineer, Linux Security - SELinux kernel
> Red Hat, Inc.