[illumos-Developer] kstat_delete interface

[Albert Lee]

On Tue, Jun 14, 2011 at 3:33 PM, Steve Gonczi <gonczi at comcast.net> wrote:
> Greetings,
>
> I recently have been looking at a crash in the stmf code, where a couple of
> kstats appear to point to recycled kstat entries.
> (itl_kstat_info and itl_kstat_taskq)
>
> The ks_module is "caps" instead of "stmf", and the ks_name is
> "nprocs_task_xxx" instead of  the expected names. So that leads me to
> believe
> these kstats has been freed/recycled.
>
> This brings me to the question: would it not make sense to set any kstat
> pointers to NULL after they have been freed?
>
> Looking at the kstat_delete() implementation,  it seems like the natural
> place
> to do this.
>
> Arguably, the "right  solution" would be  to change the interface to
> kstat_delete()
> from
>
> -kstat_delete(kstat_t *ksp)
> +kstat_delete(kstat_t **ksp)
>
> so that it can set the *ksp to NULL when it is done freeing the memory
> that it points to.
>
> Unfortunately kstat_delete() is called in 170 places.
>
> This is clearly  a problem.. Any opinions?
>
>
> Steve
>
>
>
>
> /sG/
>
>

I don't feel the same way about the "correctness" of adding this side
effect which is only effective for a particular case of
use-after-free. It's really just an user error if any pointers are
reused and this does not help if there are still other dangling
pointers and also assumes there is an explicit check for NULL values.

I don't see anything wrong with this model which matches the
alloc/free in various resource allocators.

-Albert