linux-kernel - Re: [PATCH -next v2 1/2] fs/ntfs3: Don't use uni1 uninitialized in ntfs_d

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: <f5c18c15-6fdf-d7fe-627b-080dd7af93cc@paragon-software.com>
Date:   Wed, 26 Oct 2022 19:56:41 +0300
From:   Konstantin Komarov <almaz.alexandrovich@...agon-software.com>
To:     Nathan Chancellor <nathan@...nel.org>
CC:     Nick Desaulniers <ndesaulniers@...gle.com>,
        Tom Rix <trix@...hat.com>, <ntfs3@...ts.linux.dev>,
        <llvm@...ts.linux.dev>, <patches@...ts.linux.dev>,
        <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH -next v2 1/2] fs/ntfs3: Don't use uni1 uninitialized in
 ntfs_d_compare()



On 10/21/22 20:10, Nathan Chancellor wrote:
> Ping? This is still breaking our builds in -next...
> 

Thanks for patch, applied!

> On Tue, Oct 04, 2022 at 04:23:58PM -0700, Nathan Chancellor wrote:
>> Clang warns:
>>
>>    fs/ntfs3/namei.c:445:7: error: variable 'uni1' is used uninitialized whenever 'if' condition is true [-Werror,-Wsometimes-uninitialized]
>>                    if (toupper(c1) != toupper(c2)) {
>>                        ^~~~~~~~~~~~~~~~~~~~~~~~~~
>>    ./include/linux/ctype.h:64:20: note: expanded from macro 'toupper'
>>    #define toupper(c) __toupper(c)
>>                      ^
>>    fs/ntfs3/namei.c:487:12: note: uninitialized use occurs here
>>            __putname(uni1);
>>                      ^~~~
>>    ./include/linux/fs.h:2789:65: note: expanded from macro '__putname'
>>    #define __putname(name)         kmem_cache_free(names_cachep, (void *)(name))
>>                                                                          ^~~~
>>    fs/ntfs3/namei.c:445:3: note: remove the 'if' if its condition is always false
>>                    if (toupper(c1) != toupper(c2)) {
>>                    ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>>    fs/ntfs3/namei.c:434:7: error: variable 'uni1' is used uninitialized whenever 'if' condition is true [-Werror,-Wsometimes-uninitialized]
>>                    if (!lm--) {
>>                        ^~~~~
>>    fs/ntfs3/namei.c:487:12: note: uninitialized use occurs here
>>            __putname(uni1);
>>                      ^~~~
>>    ./include/linux/fs.h:2789:65: note: expanded from macro '__putname'
>>    #define __putname(name)         kmem_cache_free(names_cachep, (void *)(name))
>>                                                                          ^~~~
>>    fs/ntfs3/namei.c:434:3: note: remove the 'if' if its condition is always false
>>                    if (!lm--) {
>>                    ^~~~~~~~~~~~
>>    fs/ntfs3/namei.c:430:22: note: initialize the variable 'uni1' to silence this warning
>>            struct cpu_str *uni1, *uni2;
>>                                ^
>>                                = NULL
>>    2 errors generated.
>>
>> There is no point in calling __putname() in these particular error
>> paths, as there has been no corresponding __getname() call yet. Just
>> return directly in these blocks to clear up the warning.
>>
>> Fixes: a3a956c78efa ("fs/ntfs3: Add option "nocase"")
>> Link: https://github.com/ClangBuiltLinux/linux/issues/1729
>> Reviewed-by: Nick Desaulniers <ndesaulniers@...gle.com>
>> Signed-off-by: Nathan Chancellor <nathan@...nel.org>
>> ---
>>
>> v2:
>>      * Pick up Nick's reviewed-by tag.
>> v1: https://lore.kernel.org/20221004144145.1345772-1-nathan@kernel.org/
>>
>>   fs/ntfs3/namei.c | 12 ++++--------
>>   1 file changed, 4 insertions(+), 8 deletions(-)
>>
>> diff --git a/fs/ntfs3/namei.c b/fs/ntfs3/namei.c
>> index 315763eb05ff..5d3a6ce3f05f 100644
>> --- a/fs/ntfs3/namei.c
>> +++ b/fs/ntfs3/namei.c
>> @@ -431,10 +431,8 @@ static int ntfs_d_compare(const struct dentry *dentry, unsigned int len1,
>>   
>>   	/* First try fast implementation. */
>>   	for (;;) {
>> -		if (!lm--) {
>> -			ret = len1 == len2 ? 0 : 1;
>> -			goto out;
>> -		}
>> +		if (!lm--)
>> +			return len1 == len2 ? 0 : 1;
>>   
>>   		if ((c1 = *n1++) == (c2 = *n2++))
>>   			continue;
>> @@ -442,10 +440,8 @@ static int ntfs_d_compare(const struct dentry *dentry, unsigned int len1,
>>   		if (c1 >= 0x80 || c2 >= 0x80)
>>   			break;
>>   
>> -		if (toupper(c1) != toupper(c2)) {
>> -			ret = 1;
>> -			goto out;
>> -		}
>> +		if (toupper(c1) != toupper(c2))
>> +			return 1;
>>   	}
>>   
>>   	/*
>>
>> base-commit: d45da67caedacd500879de5e649360cc70777af7
>> -- 
>> 2.37.3
>>
>>