iput() ends up calling ->evict() on new inode, which is not yet initialized
by owning fs. So use destroy_inode() instead.
Add to sb->s_inodes list only if inode is not in I_CREATING state (meaning
that it wasn't allocated with new_inode(), which already does the
insertion).
Reported-by: Al Viro <[email protected]>
Signed-off-by: Miklos Szeredi <[email protected]>
Fixes: 80ea09a002bf ("vfs: factor out inode_insert5()")
---
fs/inode.c | 8 ++++++--
1 file changed, 6 insertions(+), 2 deletions(-)
diff --git a/fs/inode.c b/fs/inode.c
index 04dd7e0d5142..0aa5b29b6f87 100644
--- a/fs/inode.c
+++ b/fs/inode.c
@@ -1050,6 +1050,7 @@ struct inode *inode_insert5(struct inode *inode, unsigned long hashval,
{
struct hlist_head *head = inode_hashtable + hash(inode->i_sb, hashval);
struct inode *old;
+ bool creating = inode->i_state & I_CREATING;
again:
spin_lock(&inode_hash_lock);
@@ -1083,6 +1084,8 @@ struct inode *inode_insert5(struct inode *inode, unsigned long hashval,
inode->i_state |= I_NEW;
hlist_add_head(&inode->i_hash, head);
spin_unlock(&inode->i_lock);
+ if (!creating)
+ inode_sb_list_add(inode);
unlock:
spin_unlock(&inode_hash_lock);
@@ -1117,12 +1120,13 @@ struct inode *iget5_locked(struct super_block *sb, unsigned long hashval,
struct inode *inode = ilookup5(sb, hashval, test, data);
if (!inode) {
- struct inode *new = new_inode(sb);
+ struct inode *new = alloc_inode(sb);
if (new) {
+ new->i_state = 0;
inode = inode_insert5(new, hashval, test, set, data);
if (unlikely(inode != new))
- iput(new);
+ destroy_inode(new);
}
}
return inode;
--
2.14.3
On Tue, Jul 24, 2018 at 10:01 AM, Miklos Szeredi <[email protected]> wrote:
> iput() ends up calling ->evict() on new inode, which is not yet initialized
> by owning fs. So use destroy_inode() instead.
>
> Add to sb->s_inodes list only if inode is not in I_CREATING state (meaning
> that it wasn't allocated with new_inode(), which already does the
> insertion).
>
> Reported-by: Al Viro <[email protected]>
> Signed-off-by: Miklos Szeredi <[email protected]>
> Fixes: 80ea09a002bf ("vfs: factor out inode_insert5()")
> ---
> fs/inode.c | 8 ++++++--
> 1 file changed, 6 insertions(+), 2 deletions(-)
>
> diff --git a/fs/inode.c b/fs/inode.c
> index 04dd7e0d5142..0aa5b29b6f87 100644
> --- a/fs/inode.c
> +++ b/fs/inode.c
> @@ -1050,6 +1050,7 @@ struct inode *inode_insert5(struct inode *inode, unsigned long hashval,
> {
> struct hlist_head *head = inode_hashtable + hash(inode->i_sb, hashval);
> struct inode *old;
> + bool creating = inode->i_state & I_CREATING;
>
> again:
> spin_lock(&inode_hash_lock);
> @@ -1083,6 +1084,8 @@ struct inode *inode_insert5(struct inode *inode, unsigned long hashval,
> inode->i_state |= I_NEW;
> hlist_add_head(&inode->i_hash, head);
> spin_unlock(&inode->i_lock);
> + if (!creating)
> + inode_sb_list_add(inode);
> unlock:
> spin_unlock(&inode_hash_lock);
>
> @@ -1117,12 +1120,13 @@ struct inode *iget5_locked(struct super_block *sb, unsigned long hashval,
> struct inode *inode = ilookup5(sb, hashval, test, data);
>
> if (!inode) {
> - struct inode *new = new_inode(sb);
> + struct inode *new = alloc_inode(sb);
>
> if (new) {
> + new->i_state = 0;
> inode = inode_insert5(new, hashval, test, set, data);
> if (unlikely(inode != new))
> - iput(new);
> + destroy_inode(new);
> }
> }
> return inode;
> --
> 2.14.3
>
FYI with this patch (now merged) I'm seeing warnings whenever an
object is created in an overlayfs mount:
[ 842.152673] list_add double add: new=ffff88017efe03d8,
prev=ffff88015c07ad88, next=ffff88017efe03d8.
[ 842.152687] WARNING: CPU: 4 PID: 7592 at lib/list_debug.c:31
__list_add_valid+0x6e/0x80
The call stack looks like this, for the file creation case:
[ 842.152746] inode_sb_list_add+0x4e/0x90
[ 842.152753] ? ovl_inode_test+0x20/0x20 [overlay]
[ 842.152757] ? ovl_get_redirect_xattr+0x140/0x140 [overlay]
[ 842.152759] inode_insert5+0x13e/0x1f0
[ 842.152764] ovl_get_inode.cold.16+0x38/0x44 [overlay]
[ 842.152768] ovl_instantiate+0x75/0x130 [overlay]
[ 842.152773] ovl_create_or_link+0x1c9/0x7d0 [overlay]
[ 842.152776] ? ovl_alloc_inode+0x1b/0x80 [overlay]
[ 842.152778] ? inode_sb_list_add+0x4e/0x90
[ 842.152782] ? ovl_fill_inode+0xd8/0x150 [overlay]
[ 842.152787] ovl_create_object+0xa1/0xd0 [overlay]
[ 842.152791] ovl_create+0x23/0x30 [overlay]
.. where the inode passed to inode_insert5 originally comes from
new_inode (ovl_create_object -> ovl_new_inode -> new_inode), was
already added to the sb list, and doesn't have I_CREATING set.
Originally seen from docker's use of overlayfs, but easily
reproducible by creating a simple overlay of 2 ext4 directories and
creating a new file or directory in the overlay.
Marc
On Wed, Aug 22, 2018 at 4:37 PM, Marc Dionne <[email protected]> wrote:
>
> FYI with this patch (now merged) I'm seeing warnings whenever an
> object is created in an overlayfs mount:
>
> [ 842.152673] list_add double add: new=ffff88017efe03d8,
> prev=ffff88015c07ad88, next=ffff88017efe03d8.
> [ 842.152687] WARNING: CPU: 4 PID: 7592 at lib/list_debug.c:31
> __list_add_valid+0x6e/0x80
>
And I see that's now fixed by 6faf05c2b2b4 ("ovl: set I_CREATING on
inode being created").
Marc