Re: [PATCH] bugfix: two read_inode() calls without clear_inode() call between

Re: [PATCH] bugfix: two read_inode() calls without clear_inode() call between

[Vasily Averin]

Hello,

Could you please explain me, why we need to wake up somebody right 
before freeing an inode? It seems for me, if somebody really wait on 
this inode, then they have a good chance to access already freed memory.

Thank you,
	Vasily Averin


diff --git a/fs/inode.c b/fs/inode.c
--- a/fs/inode.c
+++ b/fs/inode.c
@@ -282,6 +282,13 @@ static void dispose_list(struct list_hea
  		if (inode->i_data.nrpages)
  			truncate_inode_pages(&inode->i_data, 0);
  		clear_inode(inode);
+
+		spin_lock(&inode_lock);
+		hlist_del_init(&inode->i_hash);
+		list_del_init(&inode->i_sb_list);
+		spin_unlock(&inode_lock);
+
+		wake_up_inode(inode);
                 ^^^^^^^^^^^^^^^^^^^^
  		destroy_inode(inode);
  		nr_disposed++;
  	}

tree b4e0b69dbf3d2da69aa49423227a1da6036e9566
parent 168a9fd6a1bf91041adf9909f6c72cf747f0ca8c
author Artem B. Bityuckiy <dedekind@infradead.org>
committer Linus Torvalds <torvalds@g5.osdl.org>

Acked-by: Christoph Hellwig <hch@lst.de>
Signed-off-by: Andrew Morton <akpm@osdl.org>
Signed-off-by: Linus Torvalds <torvalds@osdl.org>

Re: [PATCH] bugfix: two read_inode() calls without clear_inode() call between

[Miklos Szeredi]

> Could you please explain me, why we need to wake up somebody right 
> before freeing an inode? It seems for me, if somebody really wait on 
> this inode, then they have a good chance to access already freed memory.

find_inode() needs to be woken up (__wait_on_freeing_inode) when an
inode being freed is actually taken off the hash list .  And it's
careful not to touch it after being woken up.

Miklos

> diff --git a/fs/inode.c b/fs/inode.c
> --- a/fs/inode.c
> +++ b/fs/inode.c
> @@ -282,6 +282,13 @@ static void dispose_list(struct list_hea
>   		if (inode->i_data.nrpages)
>   			truncate_inode_pages(&inode->i_data, 0);
>   		clear_inode(inode);
> +
> +		spin_lock(&inode_lock);
> +		hlist_del_init(&inode->i_hash);
> +		list_del_init(&inode->i_sb_list);
> +		spin_unlock(&inode_lock);
> +
> +		wake_up_inode(inode);
>                  ^^^^^^^^^^^^^^^^^^^^
>   		destroy_inode(inode);
>   		nr_disposed++;
>   	}