[lkml]   [2007]   [Feb]   [5]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
Messages in this thread
    SubjectRe: remove_proc_entry and read_proc
    On Fri, Feb 02, 2007 at 08:31:57AM +0100, Duncan Sands wrote:
    > > 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.

    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. There will be
    new patch soon.

    ->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);

    > 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.

    > [1] proc_get_inode does a try_module_get, so it is possible that module
    > unloading is not a problem - not sure.

    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;

    To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
    the body of a message to
    More majordomo info at
    Please read the FAQ at

     \ /
      Last update: 2007-02-05 12:35    [W:0.023 / U:51.920 seconds]
    ©2003-2016 Jasper Spaans. hosted at Digital OceanAdvertise on this site