Hi Andrew,
I've run all 4 of these patches (for the second one, I'm using my latest
version; I'll resend it formally) through a little bit of testing here.
Hi Wang,
Thanks for the patch.
On Wed, Feb 12, 2014 at 12:44:54PM -0800, Andrew Morton wrote:
> From: Wang Guoli <andy.wangguoli@xxxxxxxxxx>
> Subject: jffs2: unlock f->sem on error in jffs2_new_inode()
>
> If jffs2_new_inode() succeeds, it returns with f->sem held, and the caller
> is responsible for releasing the lock. If it fails, it still returns with
> the lock held, but the caller won't release the lock, which will lead to
> deadlock.
Have you actually seen a deadlock for this? AFAICT, the error cases for
jffs2_new_inode() all occur before anyone else actually has a reference
to the inode, so I don't expect that we should see the deadlock.
> Fix it by releasing the lock in jffs2_new_inode() on error.
>
> Signed-off-by: Wang Guoli <andy.wangguoli@xxxxxxxxxx>
> Signed-off-by: Wang Nan <wangnan0@xxxxxxxxxx>
> Cc: Artem Bityutskiy <artem.bityutskiy@xxxxxxxxxxxxxxx>
> Cc: David Woodhouse <dwmw2@xxxxxxxxxxxxx>
> Cc: Wang Guoli <andy.wangguoli@xxxxxxxxxx>
> Cc: Brian Norris <computersforpeace@xxxxxxxxx>
> Cc: <stable@xxxxxxxxxxxxxxx> # 2.6.34+
> Signed-off-by: Andrew Morton <akpm@xxxxxxxxxxxxxxxxxxxx>
> ---
>
> fs/jffs2/fs.c | 9 ++++++---
> 1 file changed, 6 insertions(+), 3 deletions(-)
>
> diff -puN fs/jffs2/fs.c~jffs2-unlock-f-sem-on-error-in-jffs2_new_inode fs/jffs2/fs.c
> --- a/fs/jffs2/fs.c~jffs2-unlock-f-sem-on-error-in-jffs2_new_inode
> +++ a/fs/jffs2/fs.c
> @@ -457,12 +457,14 @@ struct inode *jffs2_new_inode (struct in
> The umask is only applied if there's no default ACL */
> ret = jffs2_init_acl_pre(dir_i, inode, &mode);
> if (ret) {
> - make_bad_inode(inode);
> - iput(inode);
> - return ERR_PTR(ret);
> + mutex_unlock(&f->sem);
> + make_bad_inode(inode);
> + iput(inode);
> + return ERR_PTR(ret);
> }
> ret = jffs2_do_new_inode (c, f, mode, ri);
> if (ret) {
> + mutex_unlock(&f->sem);
> make_bad_inode(inode);
> iput(inode);
> return ERR_PTR(ret);
> @@ -479,6 +481,7 @@ struct inode *jffs2_new_inode (struct in
> inode->i_size = 0;
>
> if (insert_inode_locked(inode) < 0) {
> + mutex_unlock(&f->sem);
> make_bad_inode(inode);
> iput(inode);
> return ERR_PTR(-EINVAL);
Brian
--
To unsubscribe from this list: send the line "unsubscribe stable" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html