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

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: <ca0946de-5343-aaf9-10f2-007e341cb8a4@canonical.com>
Date:   Tue, 6 Aug 2019 14:49:52 +0100
From:   Colin Ian King <colin.king@...onical.com>
To:     Qu Wenruo <wqu@...e.com>, David Sterba <dsterba@...e.com>,
        Chris Mason <clm@...com>, Josef Bacik <josef@...icpanda.com>,
        linux-btrfs@...r.kernel.org
Cc:     "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>
Subject: Re: btrfs: qgroup: Try our best to delete qgroup relations (bug
 report)

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@...e.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;

> 
> Colin
>