* re: btrfs: qgroup: Try our best to delete qgroup relations (bug report)
@ 2019-08-06 13:48 Colin Ian King
2019-08-06 13:49 ` Colin Ian King
0 siblings, 1 reply; 3+ messages in thread
From: Colin Ian King @ 2019-08-06 13:48 UTC (permalink / raw)
To: Qu Wenruo, David Sterba, Chris Mason, Josef Bacik, linux-btrfs
Cc: linux-kernel
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;
Colin
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: btrfs: qgroup: Try our best to delete qgroup relations (bug report)
2019-08-06 13:48 btrfs: qgroup: Try our best to delete qgroup relations (bug report) Colin Ian King
@ 2019-08-06 13:49 ` Colin Ian King
2019-08-06 13:55 ` Qu Wenruo
0 siblings, 1 reply; 3+ messages in thread
From: Colin Ian King @ 2019-08-06 13:49 UTC (permalink / raw)
To: Qu Wenruo, David Sterba, Chris Mason, Josef Bacik, linux-btrfs
Cc: linux-kernel
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;
>
> Colin
>
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: btrfs: qgroup: Try our best to delete qgroup relations (bug report)
2019-08-06 13:49 ` Colin Ian King
@ 2019-08-06 13:55 ` Qu Wenruo
0 siblings, 0 replies; 3+ messages in thread
From: Qu Wenruo @ 2019-08-06 13:55 UTC (permalink / raw)
To: Colin Ian King, Qu Wenruo, David Sterba, Chris Mason,
Josef Bacik, linux-btrfs
Cc: linux-kernel
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
>>
>
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2019-08-06 13:56 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-08-06 13:48 btrfs: qgroup: Try our best to delete qgroup relations (bug report) Colin Ian King
2019-08-06 13:49 ` Colin Ian King
2019-08-06 13:55 ` Qu Wenruo
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).