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
