Re: [PATCH RFC] NFSD: fix cannot umounting mount points under pseudo root

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Cc Steve, Viro,

On 5/1/2015 5:36 AM, J. Bruce Fields wrote:
> On Thu, Apr 30, 2015 at 07:52:25AM +1000, NeilBrown wrote:
>> On Wed, 29 Apr 2015 15:19:34 -0400 "J. Bruce Fields" <bfields@xxxxxxxxxxxx>
>> wrote:
>>> Maybe drop the locking from nfsd_buffered_readdir and *just* take the
>>> i_mutex around lookup_one_len(), if that's the only place we need it?

As description in other thread, before the upcall to rpc.mountd,
nfsd have call lookup_one_len() for the file, but why rpc.mountd
also blocked in lookup ?

There is a bug in rpc.mountd when checking sub-directory, 
it sets bad patch length for child.

If parent if "/nfs/xfs" and child is "/nfs/test", the child name
will be truncated to "/nfs/tes" for strlen(parent), "/nfs/test"
have exist in kernel's cache for the lookup_one_len(), but
"/nfs/tes" is a bad path, which needs lookup_slow(), so blocked.

static int is_subdirectory(char *child, char *parent)
{
        /* Check is child is strictly a subdirectory of
         * parent or a more distant descendant.
         */
        size_t l = strlen(parent);

        if (strcmp(parent, "/") == 0 && child[1] != 0)
                return 1;

        return (same_path(child, parent, l) && child[l] == '/');
}

The following path makes a correct path, not a truncated path.
Have be tested, everything is OK.

thanks,
Kinglong Mee

-----------------------------------------------------------------------------------
>From 70b9d1d93a24db8a7837998cb7eb0ff4e98480a6 Mon Sep 17 00:00:00 2001
From: Kinglong Mee <kinglongmee@xxxxxxxxx>
Date: Tue, 5 May 2015 11:47:20 +0800
Subject: [PATCH] mountd: Case-insensitive path length must equal to parent

Commit 6091c0a4c4 (mountd: add support for case-insensitive file names)
introdues a bug cause mutex race when looking bad path.

Signed-off-by: Kinglong Mee <kinglongmee@xxxxxxxxx>
---
 utils/mountd/cache.c | 22 +++++++++++++++++++++-
 1 file changed, 21 insertions(+), 1 deletion(-)

diff --git a/utils/mountd/cache.c b/utils/mountd/cache.c
index 7d250f9..9d9a1bb 100644
--- a/utils/mountd/cache.c
+++ b/utils/mountd/cache.c
@@ -468,16 +468,36 @@ fallback:
 	return 1;
 }
 
+static int subdir_len(char *name, int count_slashes)
+{
+	char *ptr = NULL;
+	int i;
+
+	ptr = name;
+	for (i = 0; i < count_slashes + 1; i++) {
+		ptr = strchr(ptr, '/');
+		if (NULL == ptr)
+			return strlen(name);
+		ptr++;
+	}
+
+	return ptr - name;
+}
+
 static int is_subdirectory(char *child, char *parent)
 {
 	/* Check is child is strictly a subdirectory of
 	 * parent or a more distant descendant.
 	 */
-	size_t l = strlen(parent);
+	size_t l = subdir_len(child, count_slashes(parent));
 
 	if (strcmp(parent, "/") == 0 && child[1] != 0)
 		return 1;
 
+	/* Case-insensitive path length must equal to parent */
+	if (l != strlen(parent))
+		return 0;
+
 	return (same_path(child, parent, l) && child[l] == '/');
 }
 
-- 
2.4.0

--
To unsubscribe from this list: send the line "unsubscribe linux-nfs" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html




[Index of Archives]     [Linux Filesystem Development]     [Linux USB Development]     [Linux Media Development]     [Video for Linux]     [Linux NILFS]     [Linux Audio Users]     [Yosemite Info]     [Linux SCSI]

  Powered by Linux