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

[Kinglong Mee <kinglongmee@gmail.com>]

On 6/27/2015 07:35, NeilBrown wrote:
> On Sat, 27 Jun 2015 07:14:00 +0800 Kinglong Mee <kinglongmee@gmail.com>
> wrote:
> 
>> ping ....
>>
>>> -----------------------------------------------------------------------------------
>>> >From d831154bb7e527f9003e16ac049526be5ed90228 Mon Sep 17 00:00:00 2001
>>> From: Kinglong Mee <kinglongmee@gmail.com>
>>> Date: Tue, 5 May 2015 16:24:16 +0800
>>> Subject: [PATCH] mountd: Case-insensitive path length must equals to parent
>>>
>>> Commit 6091c0a4c4 (mountd: add support for case-insensitive file names)
>>> introduces looking up bad path which is easy to trigger a present mutex race.
>>>
> 
> How do you know that every file system that treats multiple different
> strings as the same, imposes the rule that the strings must be the same
> length?

Agree with you.
With your suggestion, there is a bug exist.

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);

# rpc.mountd get parent's length.

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

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

# will truncate child's name by parent's length, also checking child[l].
# I think the length should be calculated by the count of '/'.
}

> 
> Given how complex unicode case rules can be, I certainly wouldn't be
> certain of that.
> 
> If the only purpose of this patch is to avoid triggering a bug in a
> separate piece of code, then I am not in favour of it.

Yes,
This patch just change two condition's position.

Please ignore this patch,
maybe a new patch for NeilBrown's suggestion will be post.

thanks,
Kinglong Mee

> 
> Thanks,
> NeilBrown
> 
> 
>>> Signed-off-by: Kinglong Mee <kinglongmee@gmail.com>
>>> ---
>>>  utils/mountd/cache.c | 2 +-
>>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>>
>>> diff --git a/utils/mountd/cache.c b/utils/mountd/cache.c
>>> index 7d250f9..155695a 100644
>>> --- a/utils/mountd/cache.c
>>> +++ b/utils/mountd/cache.c
>>> @@ -478,7 +478,7 @@ static int is_subdirectory(char *child, char *parent)
>>>  	if (strcmp(parent, "/") == 0 && child[1] != 0)
>>>  		return 1;
>>>  
>>> -	return (same_path(child, parent, l) && child[l] == '/');
>>> +	return (child[l] == '/'	&& same_path(child, parent, l));
>>>  }
>>>  
>>>  static int path_matches(nfs_export *exp, char *path)
>>>
> 
>