Messages in this thread | | | From | Duncan Sands <> | Subject | Re: remove_proc_entry and read_proc | Date | Wed, 31 Jan 2007 20:26:14 +0100 |
| |
On Wednesday 31 January 2007 19:42:51 Alexey Dobriyan wrote: > On Wed, Jan 31, 2007 at 11:54:35AM +0100, Duncan Sands wrote: > > Can read_proc still be executing when remove_proc_entry returns? > > > > In my driver [*] I allocate some data and create a proc entry using > > create_proc_entry. My read method reads from my allocated data. When > > shutting down, I call remove_proc_entry and immediately free the data. > > If some call to read_proc is still executing at this point then it will > > be accessing freed memory. Can this happen? I've been rummaging around > > in fs/proc to see what prevents it, but didn't find anything yet. > > This should be fixed by the following patch (in -mm currently): > http://kernel.org/pub/linux/kernel/people/akpm/patches/2.6/2.6.20-rc6/2.6.20-rc6-mm3/broken-out/fix-rmmod-read-write-races-in-proc-entries.patch > > Tell me if you're unsure it will.
Excellent! But tell me,
+ atomic_inc(&dp->pde_users); + if (!dp->proc_fops)
don't you need a memory barrier between these two? Also a corresponding one where proc_fops is set to NULL.
+ /* + * Stop accepting new readers/writers. If you're dynamically + * allocating ->proc_fops, save a pointer somewhere. + */ + de->proc_fops = NULL; + /* Wait until all readers/writers are done. */ + if (atomic_read(&de->pde_users) > 0) { + spin_unlock(&proc_subdir_lock); + msleep(1); + goto again; + }
I don't understand how this is supposed to work. Consider
CPU1 CPU2
atomic_inc(&dp->pde_users); if (dp->proc_fops) de->proc_fops = NULL; use_proc_fops <= BOOM if (atomic_read(&de->pde_users) > 0) {
what prevents dereference of a NULL proc_fops value?
Best wishes,
Duncan. - 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/
| |