On 2014/2/26 10:03, Brian Norris wrote: > 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 <[email protected]> >> 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. >
We found this bug by reading code. There's no deadlock have been actually seen. Thank you. >> Fix it by releasing the lock in jffs2_new_inode() on error. >> >> Signed-off-by: Wang Guoli <[email protected]> >> Signed-off-by: Wang Nan <[email protected]> >> Cc: Artem Bityutskiy <[email protected]> >> Cc: David Woodhouse <[email protected]> >> Cc: Wang Guoli <[email protected]> >> Cc: Brian Norris <[email protected]> >> Cc: <[email protected]> # 2.6.34+ >> Signed-off-by: Andrew Morton <[email protected]> >> --- >> >> 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 [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html
