Re: [PATCH 3/3] VFS: close race between getcwd() and d_move()

On Tue, Nov 21 2017, Al Viro wrote: > On Fri, Nov 10, 2017 at 08:53:28PM +, Al Viro wrote: >> On Fri, Nov 10, 2017 at 03:45:41PM +1100, NeilBrown wrote: >> > -void __d_drop(struct dentry *dentry) >> > +static void ___d_drop(struct dentry *dentry) >> > { >> >if (!d_unhashed(dentry)) { >>

Re: [PATCH 3/3] VFS: close race between getcwd() and d_move()

On Tue, Nov 21 2017, Al Viro wrote: > On Fri, Nov 10, 2017 at 08:53:28PM +, Al Viro wrote: >> On Fri, Nov 10, 2017 at 03:45:41PM +1100, NeilBrown wrote: >> > -void __d_drop(struct dentry *dentry) >> > +static void ___d_drop(struct dentry *dentry) >> > { >> >if (!d_unhashed(dentry)) { >>

Re: [PATCH 3/3] VFS: close race between getcwd() and d_move()

On Fri, Nov 10, 2017 at 08:53:28PM +, Al Viro wrote: > On Fri, Nov 10, 2017 at 03:45:41PM +1100, NeilBrown wrote: > > -void __d_drop(struct dentry *dentry) > > +static void ___d_drop(struct dentry *dentry) > > { > > if (!d_unhashed(dentry)) { > > struct hlist_bl_head *b; > >

Re: [PATCH 3/3] VFS: close race between getcwd() and d_move()

On Fri, Nov 10, 2017 at 08:53:28PM +, Al Viro wrote: > On Fri, Nov 10, 2017 at 03:45:41PM +1100, NeilBrown wrote: > > -void __d_drop(struct dentry *dentry) > > +static void ___d_drop(struct dentry *dentry) > > { > > if (!d_unhashed(dentry)) { > > struct hlist_bl_head *b; > >

Re: [PATCH 3/3] VFS: close race between getcwd() and d_move()

On Fri, Nov 10, 2017 at 03:45:41PM +1100, NeilBrown wrote: > -void __d_drop(struct dentry *dentry) > +static void ___d_drop(struct dentry *dentry) > { > if (!d_unhashed(dentry)) { > struct hlist_bl_head *b; > @@ -486,12 +488,15 @@ void __d_drop(struct dentry *dentry) > >

Re: [PATCH 3/3] VFS: close race between getcwd() and d_move()

On Fri, Nov 10, 2017 at 03:45:41PM +1100, NeilBrown wrote: > -void __d_drop(struct dentry *dentry) > +static void ___d_drop(struct dentry *dentry) > { > if (!d_unhashed(dentry)) { > struct hlist_bl_head *b; > @@ -486,12 +488,15 @@ void __d_drop(struct dentry *dentry) > >

Re: [PATCH 3/3] VFS: close race between getcwd() and d_move()

On Thu, Nov 9, 2017 at 8:45 PM, NeilBrown wrote: > > However your description of what it was that you didn't like gave me an > idea - I can take the same approach as my original, but not pass flags > around. > I quite like how this turned out. > Dropping the BUG_ON() in d_rehash()

Re: [PATCH 3/3] VFS: close race between getcwd() and d_move()

On Thu, Nov 9, 2017 at 8:45 PM, NeilBrown wrote: > > However your description of what it was that you didn't like gave me an > idea - I can take the same approach as my original, but not pass flags > around. > I quite like how this turned out. > Dropping the BUG_ON() in d_rehash() isn't ideal,

Re: [PATCH 3/3] VFS: close race between getcwd() and d_move()

On Thu, Nov 09 2017, Linus Torvalds wrote: > On Thu, Nov 9, 2017 at 2:14 PM, NeilBrown wrote: >> On Thu, Nov 09 2017, Linus Torvalds wrote: >>> >>> How nasty would it be to just expand the calls to __d_drop/__d_rehash >>> into __d_move itself, and take both has list locks at the

Re: [PATCH 3/3] VFS: close race between getcwd() and d_move()

On Thu, Nov 09 2017, Linus Torvalds wrote: > On Thu, Nov 9, 2017 at 2:14 PM, NeilBrown wrote: >> On Thu, Nov 09 2017, Linus Torvalds wrote: >>> >>> How nasty would it be to just expand the calls to __d_drop/__d_rehash >>> into __d_move itself, and take both has list locks at the same time >>>

Re: [PATCH 3/3] VFS: close race between getcwd() and d_move()

On Thu, Nov 9, 2017 at 2:14 PM, NeilBrown wrote: > On Thu, Nov 09 2017, Linus Torvalds wrote: >> >> How nasty would it be to just expand the calls to __d_drop/__d_rehash >> into __d_move itself, and take both has list locks at the same time >> (with the usual ordering and checking

Re: [PATCH 3/3] VFS: close race between getcwd() and d_move()

On Thu, Nov 9, 2017 at 2:14 PM, NeilBrown wrote: > On Thu, Nov 09 2017, Linus Torvalds wrote: >> >> How nasty would it be to just expand the calls to __d_drop/__d_rehash >> into __d_move itself, and take both has list locks at the same time >> (with the usual ordering and checking if it's the

Re: [PATCH 3/3] VFS: close race between getcwd() and d_move()

On Thu, Nov 09 2017, Linus Torvalds wrote: > On Wed, Nov 8, 2017 at 7:22 PM, NeilBrown wrote: >> d_move() will call __d_drop() and then __d_rehash() >> on the dentry being moved. This creates a small window >> when the dentry appears to be unhashed. Many tests >> of

Re: [PATCH 3/3] VFS: close race between getcwd() and d_move()

On Thu, Nov 09 2017, Linus Torvalds wrote: > On Wed, Nov 8, 2017 at 7:22 PM, NeilBrown wrote: >> d_move() will call __d_drop() and then __d_rehash() >> on the dentry being moved. This creates a small window >> when the dentry appears to be unhashed. Many tests >> of d_unhashed() are made under

Re: [PATCH 3/3] VFS: close race between getcwd() and d_move()

On Wed, Nov 8, 2017 at 7:22 PM, NeilBrown wrote: > d_move() will call __d_drop() and then __d_rehash() > on the dentry being moved. This creates a small window > when the dentry appears to be unhashed. Many tests > of d_unhashed() are made under ->d_lock and so are safe > from

Re: [PATCH 3/3] VFS: close race between getcwd() and d_move()

On Wed, Nov 8, 2017 at 7:22 PM, NeilBrown wrote: > d_move() will call __d_drop() and then __d_rehash() > on the dentry being moved. This creates a small window > when the dentry appears to be unhashed. Many tests > of d_unhashed() are made under ->d_lock and so are safe > from racing with this

Re: [PATCH 3/3] VFS: close race between getcwd() and d_move()

On 9.11.2017 15:08, Matthew Wilcox wrote: > On Thu, Nov 09, 2017 at 01:41:24PM +0200, Nikolay Borisov wrote: >> On 9.11.2017 05:22, NeilBrown wrote: >>> @@ -493,12 +496,18 @@ void __d_drop(struct dentry *dentry) >>> } else >>> hlist_bl_lock(b); >>>

Re: [PATCH 3/3] VFS: close race between getcwd() and d_move()

On 9.11.2017 15:08, Matthew Wilcox wrote: > On Thu, Nov 09, 2017 at 01:41:24PM +0200, Nikolay Borisov wrote: >> On 9.11.2017 05:22, NeilBrown wrote: >>> @@ -493,12 +496,18 @@ void __d_drop(struct dentry *dentry) >>> } else >>> hlist_bl_lock(b); >>>

Re: [PATCH 3/3] VFS: close race between getcwd() and d_move()

On Thu, Nov 09, 2017 at 01:41:24PM +0200, Nikolay Borisov wrote: > On 9.11.2017 05:22, NeilBrown wrote: > > @@ -493,12 +496,18 @@ void __d_drop(struct dentry *dentry) > > } else > > hlist_bl_lock(b); > > __hlist_bl_del(>d_hash); > > -

Re: [PATCH 3/3] VFS: close race between getcwd() and d_move()

On Thu, Nov 09, 2017 at 01:41:24PM +0200, Nikolay Borisov wrote: > On 9.11.2017 05:22, NeilBrown wrote: > > @@ -493,12 +496,18 @@ void __d_drop(struct dentry *dentry) > > } else > > hlist_bl_lock(b); > > __hlist_bl_del(>d_hash); > > -

Re: [PATCH 3/3] VFS: close race between getcwd() and d_move()

On 9.11.2017 05:22, NeilBrown wrote: > d_move() will call __d_drop() and then __d_rehash() > on the dentry being moved. This creates a small window > when the dentry appears to be unhashed. Many tests > of d_unhashed() are made under ->d_lock and so are safe > from racing with this window,

Re: [PATCH 3/3] VFS: close race between getcwd() and d_move()

On 9.11.2017 05:22, NeilBrown wrote: > d_move() will call __d_drop() and then __d_rehash() > on the dentry being moved. This creates a small window > when the dentry appears to be unhashed. Many tests > of d_unhashed() are made under ->d_lock and so are safe > from racing with this window,

[PATCH 3/3] VFS: close race between getcwd() and d_move()

d_move() will call __d_drop() and then __d_rehash() on the dentry being moved. This creates a small window when the dentry appears to be unhashed. Many tests of d_unhashed() are made under ->d_lock and so are safe from racing with this window, but some aren't. In particular, getcwd() calls

[PATCH 3/3] VFS: close race between getcwd() and d_move()

d_move() will call __d_drop() and then __d_rehash() on the dentry being moved. This creates a small window when the dentry appears to be unhashed. Many tests of d_unhashed() are made under ->d_lock and so are safe from racing with this window, but some aren't. In particular, getcwd() calls