Re: remove_proc_entry and read_proc

From: Duncan Sands
Date: Mon Feb 05 2007 - 07:05:40 EST


> Gee, thanks. I sat and wrote code side-by-side, and it looks like, even barriers
> won't fix anything, because they don't affect other CPUs.

?! The whole point of memory barriers is that they affect other CPUs.
Maybe you are thinking of compiler barriers?

> ->proc_fops is valid ->proc_fops is valid
> ->pde_users is 0 ->pde_users is 0
> ------------------------------------------------------------
>
>
> if (!pde->proc_fops)
> goto out;
>
> ->proc_fops = NULL;
> if (atomic_read(->pde_users) > 0)
> goto again;
>
> |
> | atomic_inc(->pde_users);
> |
> |
> |
> V

The proc_fops check *before* the atomic_inc is indeed pointless (notice
how I removed it in the patch I sent?). It's the one after the atomic_inc
that prevents this race, but only if there is a memory barrier between the
atomic_inc and the check... because otherwise they could be reordered (i.e.
seen in reverse order by another CPU) giving the race.

> Modules forget to set ->owner sometimes. Also, it's still racy, because
> of the typical
>
> pde = create_proc_entry();
> /*
> *
> * ->owner is NULL here, effectively, PDE without ->owner.
> *
> */
> if (pde)
> pde->owner = THIS_MODULE;

As long as the module calls remove_proc_entry before being unloaded,
this should be ok.

Best wishes,

Duncan.
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/