On 2021/5/18 0:56, Jason Gunthorpe wrote:
> On Sat, May 15, 2021 at 03:07:58AM +0000, liweihang wrote:
>> On 2021/5/14 20:34, Jason Gunthorpe wrote:
>>> On Fri, May 14, 2021 at 10:11:34AM +0800, Weihang Li wrote:
>>>> The refcount_t API will WARN on underflow and overflow of a reference
>>>> counter, and avoid use-after-free risks. Increase refcount_t from 0 to 1 is
>>>> regarded as there is a risk about use-after-free. So it should be set to 1
>>>> directly during initialization.
>>>
>>> What does this comment about 0 to 1 mean?
>>>
>>
>> Hi Jason,
>>
>> I first thought refcount_inc() and atomic_inc() are exactly the same, but I got
>> a warning about refcount_t on iwpm_init() after the replacement:
>>
>> [ 16.882939] refcount_t: addition on 0; use-after-free.
>> [ 16.888065] WARNING: CPU: 2 PID: 1 at lib/refcount.c:25
>> refcount_warn_saturate+0xa0/0x144
>> ...
>> [ 17.014698] Call trace:
>> [ 17.017135] refcount_warn_saturate+0xa0/0x144
>> [ 17.021559] iwpm_init+0x104/0x12c
>> [ 17.024948] iw_cm_init+0x24/0xd0
>> [ 17.028248] do_one_initcall+0x54/0x2d0
>> [ 17.032068] kernel_init_freeable+0x224/0x294
>> [ 17.036407] kernel_init+0x20/0x12c
>> [ 17.039880] ret_from_fork+0x10/0x18
>>
>> Then I noticed that the comment of refcount_inc() says:
>>
>> * Will WARN if the refcount is 0, as this represents a possible use-after-free
>> * condition.
>>
>> so I made changes:
>>
>> @@ -77,8 +77,12 @@ int iwpm_init(u8 nl_client)
>> ret = -ENOMEM;
>> goto init_exit;
>> }
>> +
>> + refcount_set(&iwpm_admin.refcount, 1);
>> + } else {
>> + refcount_inc(&iwpm_admin.refcount);
>> }
>> - refcount_inc(&iwpm_admin.refcount);
>> +
>>
>> I wrote the comments because I thought someone might be confused by the above
>> changes :)
>
> Stuff like this needs to be split into a single patch for iwpm_admin
>
> Jason
>
Sure, I will split all of the changes into separate patches.
Weihang
