Re: btrfs: qgroup: Try our best to delete qgroup relations (bug report)

[Qu Wenruo <quwenruo.btrfs@gmx.com>]

On 2019/8/6 下午9:49, Colin Ian King wrote:
> On 06/08/2019 14:48, Colin Ian King wrote:
>> Hi,
>>
>> Static analysis with Coverity on linux-next picked up a potential issue
>> with the following commit:
>>
>> commit 035087b3c256741be367747eab866505cece31fb
>> Author: Qu Wenruo <wqu@suse.com>
>> Date:   Sat Aug 3 14:45:59 2019 +0800
>>
>>     btrfs: qgroup: Try our best to delete qgroup relations
>>
>> The static analysis report is as follows:
>>
>> 1334         */
>>
>>     3. Condition !member, taking true branch.
>>     4. var_compare_op: Comparing member to null implies that member
>> might be null.
>>     5. Condition !parent, taking false branch.
>>
>> 1335        if (!member && !parent)
>> 1336                goto delete_item;
>> 1337
>> 1338        /* check if such qgroup relation exist firstly */
>>
>>
>> CID 85026 (#1 of 1): Dereference after null check (FORWARD_NULL)
>>
>> 6. var_deref_op: Dereferencing null pointer member.
>>
>> 1339        list_for_each_entry(list, &member->groups, next_group) {
>> 1340                if (list->group == parent) {
>> 1341                        found = true;
>> 1342                        break;
>> 1343                }
>> 1344        }
>>
>> An example of the issue that if member is NULL and parent is not null
>> then  the list_for_each_entry loop with dereference the NULL member
>> pointer.  The changed logic in the patch on line 1335 is the root cause
>> of this regression.  I believe it should still be:
>>
>> 	if (!member && !parent)
>> 		goto delete_item;
>
> oops, I mean:
>
> 	if (!member || !parent)
> 		goto delete_item;

Right, thanks for catching this!

I'll update the patch soon.

Thanks,
Qu
>
>>
>> Colin
>>
>