Tue, Feb 07, 2017 at 11:33:31AM CET, dan.carpenter@xxxxxxxxxx wrote:
>On Tue, Feb 07, 2017 at 10:18:06AM +0300, Dan Carpenter wrote:
>> Hello Jiri Pirko,
>>
>> The patch 4cda7d8d7098: "mlxsw: core: Introduce flexible actions
>> support" from Feb 3, 2017, leads to the following static checker
>> warning:
>>
>> drivers/net/ethernet/mellanox/mlxsw/core_acl_flex_actions.c:387 mlxsw_afa_block_commit()
>> error: dereferencing freed memory 'set'
>>
>> drivers/net/ethernet/mellanox/mlxsw/core_acl_flex_actions.c
>> 355 int mlxsw_afa_block_commit(struct mlxsw_afa_block *block)
>> 356 {
>> 357 struct mlxsw_afa_set *set = block->cur_set;
>> 358 struct mlxsw_afa_set *prev_set;
>> 359 int err;
>> 360
>> 361 block->cur_set = NULL;
>> 362
>> 363 /* Go over all linked sets starting from last
>> 364 * and try to find existing set in the hash table.
>> 365 * In case it is not there, assign a KVD linear index
>> 366 * and insert it.
>> 367 */
>> 368 do {
>> 369 prev_set = set->prev;
>> 370 set = mlxsw_afa_set_get(block->afa, set);
>> 371 if (IS_ERR(set)) {
>> 372 err = PTR_ERR(set);
>
>Oh... Also it can be an error pointer on this path.
>
>> 373 goto rollback;
>> 374 }
>> 375 if (prev_set) {
>> 376 prev_set->next = set;
>> 377 mlxsw_afa_set_next_set(prev_set, set->kvdl_index);
>> 378 set = prev_set;
>> 379 }
>> 380 } while (prev_set);
>> 381
>> 382 block->first_set = set;
>> 383 block->finished = true;
>> 384 return 0;
>> 385
>> 386 rollback:
>> 387 while ((set = set->next))
> ^^^^^^^^^^^^^^^
>
>That means this dereference will oops.
Will fix this. Thanks.
>
>regards,
>dan carpenter
>
>
>> 388 mlxsw_afa_set_put(block->afa, set);
>>
>> "set" is refcounted. The heuristic I'm using assumes that if it's
>> refcounted with an atomic type then ignore the possibility that
>> mlxsw_afa_set_put() will free "set". This works very well generally
>> and this is only the second time I've seen it fail because we're using
>> regular types for refcounting.
>>
>> It's possible that we know that we're holding multiple references to
>> "set" so it will never be freed, but I normally don't feel bad sending
>> false positive warnings if the code is very recent so here we are. :)
>>
>> 389 return err;
>> 390 }
>>
>> regards,
>> dan carpenter
--
To unsubscribe from this list: send the line "unsubscribe kernel-janitors" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
