Re: remove_proc_entry and read_proc

[Duncan Sands <duncan.sands@math.u-psud.fr>]

Hi Alexey,

> I believe, barriers not needed, not now.
> 
> This scheme relies on the fact that remove_proc_entry() will be the only
> place that will clear ->proc_fops and, once cleared, ->proc_fops will
> never be resurrected. Clearing of ->proc_fops will eventually propagate
> to CPU doing first check, thus preveting refcount bumps from this CPU.
> What can be missed is some "rogue" readers or writers¹. Big deal.

I don't understand you.  Without memory barriers, remove_proc_entry will
most of the time, but not all of the time, wait for all readers and writers
to finish before exiting.  Since the whole point of your patch was to ensure
that all readers and writers finish before remove_proc_entry exits, I don't
understand why you don't just put the memory barriers in and make it correct.

Also, I do consider it a big deal:

> ¹ Sigh, modules should do removals of proc entries first. And I should
> check for that.

Modules should of course call remove_proc_entry before exiting.  However
right now, even with your patch, a read or write method can still be
running when remove_proc_entry returns [1], so could still be running when
the module is removed (if they sleep; I guess this applies mostly to
write methods).  This is very bad - why not put in memory barriers and
fix it?  Also, plenty of proc read and write methods access private data
that is allocated before calling create_proc_entry and freed after calling
remove_proc_entry.  If a read or write method is still running after
remove_proc_entry returns, then it can access freed memory - very bad.

Ciao,

Duncan.

[1] proc_get_inode does a try_module_get, so it is possible that module
unloading is not a problem - not sure.
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/