linux-kernel - Re: [PATCH] fs: introduce is_dot

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20191202200302.GN20752@bombadil.infradead.org>
Date:   Mon, 2 Dec 2019 12:03:02 -0800
From:   Matthew Wilcox <willy@...radead.org>
To:     Tiezhu Yang <yangtiezhu@...ngson.cn>
Cc:     Alexander Viro <viro@...iv.linux.org.uk>,
        "Theodore Y. Ts'o" <tytso@....edu>,
        Jaegeuk Kim <jaegeuk@...nel.org>, Chao Yu <yuchao0@...wei.com>,
        Eric Biggers <ebiggers@...nel.org>,
        Tyler Hicks <tyhicks@...onical.com>,
        linux-fsdevel@...r.kernel.org, ecryptfs@...r.kernel.org,
        linux-fscrypt@...r.kernel.org,
        linux-f2fs-devel@...ts.sourceforge.net,
        linux-kernel@...r.kernel.org
Subject: Re: [PATCH] fs: introduce is_dot_dotdot helper for cleanup

On Mon, Dec 02, 2019 at 06:10:13PM +0800, Tiezhu Yang wrote:
> There exists many similar and duplicate codes to check "." and "..",
> so introduce is_dot_dotdot helper to make the code more clean.

The idea is good.  The implementation is, I'm afraid, badly chosen.
Did you benchmark this change at all?  In general, you should prefer the
core kernel implementation to that of some less-interesting filesystems.
I measured the performance with the attached test program on my laptop
(Core-i7 Kaby Lake):

qstr . time_1 0.020531 time_2 0.005786
qstr .. time_1 0.017892 time_2 0.008798
qstr a time_1 0.017633 time_2 0.003634
qstr matthew time_1 0.011820 time_2 0.003605
qstr .a time_1 0.017909 time_2 0.008710
qstr , time_1 0.017631 time_2 0.003619

The results are quite stable:

qstr . time_1 0.021137 time_2 0.005780
qstr .. time_1 0.017964 time_2 0.008675
qstr a time_1 0.017899 time_2 0.003654
qstr matthew time_1 0.011821 time_2 0.003620
qstr .a time_1 0.017889 time_2 0.008662
qstr , time_1 0.017764 time_2 0.003613

Feel free to suggest some different strings we could use for testing.
These seemed like interesting strings to test with.  It's always possible
I've messed up something with this benchmark that causes it to not
accurately represent the performance of each algorithm, so please check
that too.

> +bool is_dot_dotdot(const struct qstr *str)
> +{
> +	if (str->len == 1 && str->name[0] == '.')
> +		return true;
> +
> +	if (str->len == 2 && str->name[0] == '.' && str->name[1] == '.')
> +		return true;
> +
> +	return false;
> +}
> +EXPORT_SYMBOL(is_dot_dotdot);
> diff --git a/fs/namei.c b/fs/namei.c
> index 2dda552..7730a3b 100644
> --- a/fs/namei.c
> +++ b/fs/namei.c
> @@ -2458,10 +2458,8 @@ static int lookup_one_len_common(const char *name, struct dentry *base,
>  	if (!len)
>  		return -EACCES;
>  
> -	if (unlikely(name[0] == '.')) {
> -		if (len < 2 || (len == 2 && name[1] == '.'))
> -			return -EACCES;
> -	}
> +	if (unlikely(is_dot_dotdot(this)))
> +		return -EACCES;
>  
>  	while (len--) {
>  		unsigned int c = *(const unsigned char *)name++;

View attachment "dotdotdot.c" of type "text/plain" (2054 bytes)