linux-kernel - Re: [PATCH 2/2] afs: Fix afs_xattr_get_yfs() to not try freeing an error value

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: <5CD8697B.6010004@bfs.de>
Date:   Sun, 12 May 2019 20:44:11 +0200
From:   walter harms <wharms@....de>
To:     David Howells <dhowells@...hat.com>
CC:     colin.king@...onical.com, joe@...ches.com, jaltman@...istor.com,
        linux-afs@...ts.infradead.org, kernel-janitors@...r.kernel.org,
        linux-kernel@...r.kernel.org
Subject: Re: [PATCH 2/2] afs: Fix afs_xattr_get_yfs() to not try freeing an
 error value

Am 12.05.2019 20:10, schrieb David Howells:
> walter harms <wharms@....de> wrote:
> 
>>> +	ret = dsize;
>>> +	if (size > 0) {
>>> +		if (dsize > size) {
>>> +			ret = -ERANGE;
>>> +			goto error_key;
>>>  		}
>>> +		memcpy(buffer, data, dsize);
>>>  	}
>>>  
>>
>> i am confused: if size is <= 0 then the error is in dsize ?
> 
> See this bit, before that hunk:
> 
>> +	if (ret < 0)
>> +		goto error_key;
> 
> David
> 

Sorry, you misunderstood me, my fault, i did not see that size is unsigned.
NTL i do not think size=0 is useful.

You get size from outside, and if i follow the flow correct
the first use of it is to check size>0.
perhaps you can check size at start and simply return.
Now if size==0 it will return dsize and give the impression
that buffer is used (it is not).

while you are there:
  flags |= YFS_ACL_WANT_ACL is always flags = YFS_ACL_WANT_ACL;
since flags is 0 at this point.
IMHO that sould be moved to the strcmp() section.

hope that helps,

re,
 wh