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.
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 <[email protected]>
---
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
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.)
--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 <[email protected]>
> ---
> 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
On Thu, Mar 25, 2021 at 8:27 PM J. Bruce Fields <[email protected]> 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 :)
>
> --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 <[email protected]>
> > ---
> > 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.
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 <[email protected]> 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 <[email protected]>
> > > ---
> > > 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.
On 3/22/21 5:02 PM, 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.
>
> 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 <[email protected]>
Committed... (tag: nfs-utils-2-5-4-rc2)
steved.
> ---
> 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) {
>