From: Brian Norris <computersforpe...@gmail.com>
Li Zefan reported an unbalanced locking issue, found by his
internal debugging feature on runtime. The particular case he was
looking at doesn't lead to a deadlock, as the structure that this lock
is embedded in is freed on error. But we should straighten out the error
handling.
Because several callers of jffs2_do_read_inode_internal() /
jffs2_do_read_inode() already handle the locking/unlocking and inode
clearing at their own level, let's just push any unlocks/clearing down
to the caller. This consistency is much easier to verify.
Reported-by: Li Zefan <lize...@huawei.com>
Cc: David Woodhouse <dw...@infradead.org>
Cc: Artem Bityutskiy <artem.bityuts...@linux.intel.com>
Cc: Andrew Morton <a...@linux-foundation.org>
Signed-off-by: Brian Norris <computersforpe...@gmail.com>
4 23 cpukit/libfs/src/jffs2/src/readinode.c
diff --git a/cpukit/libfs/src/jffs2/src/readinode.c
b/cpukit/libfs/src/jffs2/src/readinode.c
index 3314a61885..69ab7709f8 100644
--- a/cpukit/libfs/src/jffs2/src/readinode.c
+++ b/cpukit/libfs/src/jffs2/src/readinode.c
@@ -1212,17 +1212,13 @@ static int jffs2_do_read_inode_internal(struct
jffs2_sb_info *c,
JFFS2_ERROR("failed to read from flash: error %d, %zd of %zd
bytes read\n",
ret, retlen, sizeof(*latest_node));
/* FIXME: If this fails, there seems to be a memory leak. Find
it. */
- mutex_unlock(&f->sem);
- jffs2_do_clear_inode(c, f);
- return ret?ret:-EIO;
+ return ret ? ret : -EIO;
}
crc = crc32(0, latest_node, sizeof(*latest_node)-8);
if (crc != je32_to_cpu(latest_node->node_crc)) {
JFFS2_ERROR("CRC failed for read_inode of inode %u at physical
location 0x%x\n",
f->inocache->ino, ref_offset(rii.latest_ref));
- mutex_unlock(&f->sem);
- jffs2_do_clear_inode(c, f);
return -EIO;
}
@@ -1259,16 +1255,11 @@ static int jffs2_do_read_inode_internal(struct
jffs2_sb_info *c,
* keep in RAM to facilitate quick follow symlink
* operation. */
uint32_t csize = je32_to_cpu(latest_node->csize);
- if (csize > JFFS2_MAX_NAME_LEN) {
- mutex_unlock(&f->sem);
- jffs2_do_clear_inode(c, f);
+ if (csize > JFFS2_MAX_NAME_LEN)
return -ENAMETOOLONG;
- }
f->target = kmalloc(csize + 1, GFP_KERNEL);
if (!f->target) {
JFFS2_ERROR("can't allocate %u bytes of memory
for the symlink target path cache\n", csize);
- mutex_unlock(&f->sem);
- jffs2_do_clear_inode(c, f);
return -ENOMEM;
}
@@ -1280,8 +1271,6 @@ static int jffs2_do_read_inode_internal(struct
jffs2_sb_info *c,
ret = -EIO;
kfree(f->target);
f->target = NULL;
- mutex_unlock(&f->sem);
- jffs2_do_clear_inode(c, f);
return ret;
}
@@ -1298,15 +1287,11 @@ static int jffs2_do_read_inode_internal(struct
jffs2_sb_info *c,
if (f->metadata) {
JFFS2_ERROR("Argh. Special inode #%u with mode 0%o had
metadata node\n",
f->inocache->ino,
jemode_to_cpu(latest_node->mode));
- mutex_unlock(&f->sem);
- jffs2_do_clear_inode(c, f);
return -EIO;
}
if (!frag_first(&f->fragtree)) {
JFFS2_ERROR("Argh. Special inode #%u with mode 0%o has
no fragments\n",
f->inocache->ino,
jemode_to_cpu(latest_node->mode));
- mutex_unlock(&f->sem);
- jffs2_do_clear_inode(c, f);
return -EIO;
}
/* ASSERT: f->fraglist != NULL */
@@ -1314,8 +1299,6 @@ static int jffs2_do_read_inode_internal(struct
jffs2_sb_info *c,
JFFS2_ERROR("Argh. Special inode #%u with mode 0x%x had
more than one node\n",
f->inocache->ino,
jemode_to_cpu(latest_node->mode));
/* FIXME: Deal with it - check crc32, check for
duplicate node, check times and discard the older one */
- mutex_unlock(&f->sem);
- jffs2_do_clear_inode(c, f);
return -EIO;
}
/* OK. We're happy */
@@ -1409,10 +1392,8 @@ int jffs2_do_crccheck_inode(struct jffs2_sb_info *c,
struct jffs2_inode_cache *i
f->inocache = ic;
ret = jffs2_do_read_inode_internal(c, f, &n);
- if (!ret) {
- mutex_unlock(&f->sem);
- jffs2_do_clear_inode(c, f);
- }
+ mutex_unlock(&f->sem);
+ jffs2_do_clear_inode(c, f);
jffs2_xattr_do_crccheck_inode(c, ic);
kfree (f);
return ret;
--
2.13.7
_______________________________________________
devel mailing list
devel@rtems.org
http://lists.rtems.org/mailman/listinfo/devel
