On Thu, Jun 13, 2013 at 03:01:22AM +0100, Al Viro wrote:
> On Fri, Jun 07, 2013 at 05:14:52PM +0100, Al Viro wrote:
> > On Fri, Jun 07, 2013 at 11:09:05AM -0500, Dave Chiluk wrote:
> > > Can't you just use the patch from my original e-mail?  Anyhow I attached
> > > it an already signed-off patch.
> > > 
> > > Al Viro Can you integrate it now?
> > 
> > Applied...  FWIW, patch directly in mail body is more convenient to deal 
> > with.
> 
> Actually, looking at that stuff...  Why are we bothering with -EBUSY for
> removal of busy directories on ncpfs, anyway?  It's not just rmdir(), it's
> overwriting rename() as well.  IS_DEADDIR checks in fs/namei.c and 
> fs/readdir.c
> mean that the only method of ncpfs directories that might get called after
> successful removal is ->setattr() and it would be trivial to add the check
> in ncp_notify_change() that would make it fail for dead directories without
> bothering the server at all...
> 
> Related question: what happens if you open / unlink / fchmod on ncpfs?

Speaking of crap used only by ncpfs: I think we can use ->d_iput() to get rid
of d_validate() for good.  The only remaining user is ncpfs; what happens there
is that we use the page cache of directory to cache the references to dentries
made by readdir.  We could do the following trick:
        * have ->d_fsdata for these dentries a pointer into the cache page where
the reference back to dentry is stored
        * ->freepage() for those pages consisting of
                grab global spinlock
                go through all dentries still pointed to by pointers in that
page, zeroing ->d_fsdata
                drop the spinlock
        * ->d_iput() for those dentries consisting of
                grab the same spinlock
                if ->d_fsdata is non-zero, store NULL at the address pointed
to by it
                drop the spinlock
        * ncp_dget_fpos() would
                grab that spinlock
                check if the reference to dentry in the position we are
interested in is non-NULL
                        grab ->d_lock
                        if DCACHE_DENTRY_KILLED is not set
                                bump ->d_count
                                drop ->d_lock
                                drop the spinlock
                                return dentry
                        // dentry is doomed
                        clear the reference
                        drop ->d_lock
                drop the spinlock
                return NULL
        * ncp_fill_cache() would insert the sucker into cache and set
->d_fsdata under the same spinlock.

IOW, instead of wanking with untrusted pointers to dentries, we simply make
sure we clean the pointer when dentry is going away and clean the reference
from dentry to the location of that pointer when the page is going away.

Objections?  I can do a patch along those lines, but I've nothing to test it
on.  Had that been cifs, I could at least use samba to test the fucker, but
I've no idea how to do that with ncpfs and I'm not too fond of checking how
much bitrot has mars_nwe suffered...
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [email protected]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Reply via email to