Received-SPF: pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67;
Subject: Re: btrfs: qgroup: Try our best to delete qgroup relations (bug
 report)
To:     Colin Ian King <colin.king@canonical.com>,
        Qu Wenruo <wqu@suse.com>, David Sterba <dsterba@suse.com>,
        Chris Mason <clm@fb.com>, Josef Bacik <josef@toxicpanda.com>,
        linux-btrfs@vger.kernel.org
Cc:     "linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>
References: <ea7f88e7-108e-ad2a-232f-b18715607bf3@canonical.com>
 <ca0946de-5343-aaf9-10f2-007e341cb8a4@canonical.com>
From:   Qu Wenruo <quwenruo.btrfs@gmx.com>
Openpgp: preference=signencrypt
Autocrypt: addr=quwenruo.btrfs@gmx.com; prefer-encrypt=mutual; keydata=
 mQENBFnVga8BCACyhFP3ExcTIuB73jDIBA/vSoYcTyysFQzPvez64TUSCv1SgXEByR7fju3o
 8RfaWuHCnkkea5luuTZMqfgTXrun2dqNVYDNOV6RIVrc4YuG20yhC1epnV55fJCThqij0MRL
 1NxPKXIlEdHvN0Kov3CtWA+R1iNN0RCeVun7rmOrrjBK573aWC5sgP7YsBOLK79H3tmUtz6b
 9Imuj0ZyEsa76Xg9PX9Hn2myKj1hfWGS+5og9Va4hrwQC8ipjXik6NKR5GDV+hOZkktU81G5
 gkQtGB9jOAYRs86QG/b7PtIlbd3+pppT0gaS+wvwMs8cuNG+Pu6KO1oC4jgdseFLu7NpABEB
 AAG0IlF1IFdlbnJ1byA8cXV3ZW5ydW8uYnRyZnNAZ214LmNvbT6JAVQEEwEIAD4CGwMFCwkI
 BwIGFQgJCgsCBBYCAwECHgECF4AWIQQt33LlpaVbqJ2qQuHCPZHzoSX+qAUCWdWCnQUJCWYC
 bgAKCRDCPZHzoSX+qAR8B/94VAsSNygx1C6dhb1u1Wp1Jr/lfO7QIOK/nf1PF0VpYjTQ2au8
 ihf/RApTna31sVjBx3jzlmpy+lDoPdXwbI3Czx1PwDbdhAAjdRbvBmwM6cUWyqD+zjVm4RTG
 rFTPi3E7828YJ71Vpda2qghOYdnC45xCcjmHh8FwReLzsV2A6FtXsvd87bq6Iw2axOHVUax2
 FGSbardMsHrya1dC2jF2R6n0uxaIc1bWGweYsq0LXvLcvjWH+zDgzYCUB0cfb+6Ib/ipSCYp
 3i8BevMsTs62MOBmKz7til6Zdz0kkqDdSNOq8LgWGLOwUTqBh71+lqN2XBpTDu1eLZaNbxSI
 ilaVuQENBFnVga8BCACqU+th4Esy/c8BnvliFAjAfpzhI1wH76FD1MJPmAhA3DnX5JDORcga
 CbPEwhLj1xlwTgpeT+QfDmGJ5B5BlrrQFZVE1fChEjiJvyiSAO4yQPkrPVYTI7Xj34FnscPj
 /IrRUUka68MlHxPtFnAHr25VIuOS41lmYKYNwPNLRz9Ik6DmeTG3WJO2BQRNvXA0pXrJH1fN
 GSsRb+pKEKHKtL1803x71zQxCwLh+zLP1iXHVM5j8gX9zqupigQR/Cel2XPS44zWcDW8r7B0
 q1eW4Jrv0x19p4P923voqn+joIAostyNTUjCeSrUdKth9jcdlam9X2DziA/DHDFfS5eq4fEv
 ABEBAAGJATwEGAEIACYWIQQt33LlpaVbqJ2qQuHCPZHzoSX+qAUCWdWBrwIbDAUJA8JnAAAK
 CRDCPZHzoSX+qA3xB/4zS8zYh3Cbm3FllKz7+RKBw/ETBibFSKedQkbJzRlZhBc+XRwF61mi
 f0SXSdqKMbM1a98fEg8H5kV6GTo62BzvynVrf/FyT+zWbIVEuuZttMk2gWLIvbmWNyrQnzPl
 mnjK4AEvZGIt1pk+3+N/CMEfAZH5Aqnp0PaoytRZ/1vtMXNgMxlfNnb96giC3KMR6U0E+siA
 4V7biIoyNoaN33t8m5FwEwd2FQDG9dAXWhG13zcm9gnk63BN3wyCQR+X5+jsfBaS4dvNzvQv
 h8Uq/YGjCoV1ofKYh3WKMY8avjq25nlrhzD/Nto9jHp8niwr21K//pXVA81R2qaXqGbql+zo
Message-ID: <8f8edc81-0c54-da50-0902-663e7c4f26ca@gmx.com>
Date:   Tue, 6 Aug 2019 21:55:57 +0800
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:60.0) Gecko/20100101
 Thunderbird/60.8.0
MIME-Version: 1.0
In-Reply-To: <ca0946de-5343-aaf9-10f2-007e341cb8a4@canonical.com>
Content-Type: text/plain; charset=utf-8
Content-Language: en-US
Content-Transfer-Encoding: quoted-printable
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk


On 2019/8/6 =E4=B8=8B=E5=8D=889: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 =3D=3D parent) {
>> 1341                        found =3D 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
>>
>