Linux kernel mirror (for testing) git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git
kernel os linux

jffs2: fix unbalanced locking

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 <lizefan@huawei.com>
Cc: David Woodhouse <dwmw2@infradead.org>
Cc: Artem Bityutskiy <artem.bityutskiy@linux.intel.com>
Cc: Andrew Morton <akpm@linux-foundation.org>
Signed-off-by: Brian Norris <computersforpeace@gmail.com>

+6 -28
+2 -5
fs/jffs2/fs.c
··· 272 272 mutex_lock(&f->sem); 273 273 274 274 ret = jffs2_do_read_inode(c, f, inode->i_ino, &latest_node); 275 + if (ret) 276 + goto error; 275 277 276 - if (ret) { 277 - mutex_unlock(&f->sem); 278 - iget_failed(inode); 279 - return ERR_PTR(ret); 280 - } 281 278 inode->i_mode = jemode_to_cpu(latest_node.mode); 282 279 i_uid_write(inode, je16_to_cpu(latest_node.uid)); 283 280 i_gid_write(inode, je16_to_cpu(latest_node.gid));
+4 -23
fs/jffs2/readinode.c
··· 1203 1203 JFFS2_ERROR("failed to read from flash: error %d, %zd of %zd bytes read\n", 1204 1204 ret, retlen, sizeof(*latest_node)); 1205 1205 /* FIXME: If this fails, there seems to be a memory leak. Find it. */ 1206 - mutex_unlock(&f->sem); 1207 - jffs2_do_clear_inode(c, f); 1208 - return ret?ret:-EIO; 1206 + return ret ? ret : -EIO; 1209 1207 } 1210 1208 1211 1209 crc = crc32(0, latest_node, sizeof(*latest_node)-8); 1212 1210 if (crc != je32_to_cpu(latest_node->node_crc)) { 1213 1211 JFFS2_ERROR("CRC failed for read_inode of inode %u at physical location 0x%x\n", 1214 1212 f->inocache->ino, ref_offset(rii.latest_ref)); 1215 - mutex_unlock(&f->sem); 1216 - jffs2_do_clear_inode(c, f); 1217 1213 return -EIO; 1218 1214 } 1219 1215 ··· 1246 1250 * keep in RAM to facilitate quick follow symlink 1247 1251 * operation. */ 1248 1252 uint32_t csize = je32_to_cpu(latest_node->csize); 1249 - if (csize > JFFS2_MAX_NAME_LEN) { 1250 - mutex_unlock(&f->sem); 1251 - jffs2_do_clear_inode(c, f); 1253 + if (csize > JFFS2_MAX_NAME_LEN) 1252 1254 return -ENAMETOOLONG; 1253 - } 1254 1255 f->target = kmalloc(csize + 1, GFP_KERNEL); 1255 1256 if (!f->target) { 1256 1257 JFFS2_ERROR("can't allocate %u bytes of memory for the symlink target path cache\n", csize); 1257 - mutex_unlock(&f->sem); 1258 - jffs2_do_clear_inode(c, f); 1259 1258 return -ENOMEM; 1260 1259 } 1261 1260 ··· 1262 1271 ret = -EIO; 1263 1272 kfree(f->target); 1264 1273 f->target = NULL; 1265 - mutex_unlock(&f->sem); 1266 - jffs2_do_clear_inode(c, f); 1267 1274 return ret; 1268 1275 } 1269 1276 ··· 1278 1289 if (f->metadata) { 1279 1290 JFFS2_ERROR("Argh. Special inode #%u with mode 0%o had metadata node\n", 1280 1291 f->inocache->ino, jemode_to_cpu(latest_node->mode)); 1281 - mutex_unlock(&f->sem); 1282 - jffs2_do_clear_inode(c, f); 1283 1292 return -EIO; 1284 1293 } 1285 1294 if (!frag_first(&f->fragtree)) { 1286 1295 JFFS2_ERROR("Argh. Special inode #%u with mode 0%o has no fragments\n", 1287 1296 f->inocache->ino, jemode_to_cpu(latest_node->mode)); 1288 - mutex_unlock(&f->sem); 1289 - jffs2_do_clear_inode(c, f); 1290 1297 return -EIO; 1291 1298 } 1292 1299 /* ASSERT: f->fraglist != NULL */ ··· 1290 1305 JFFS2_ERROR("Argh. Special inode #%u with mode 0x%x had more than one node\n", 1291 1306 f->inocache->ino, jemode_to_cpu(latest_node->mode)); 1292 1307 /* FIXME: Deal with it - check crc32, check for duplicate node, check times and discard the older one */ 1293 - mutex_unlock(&f->sem); 1294 - jffs2_do_clear_inode(c, f); 1295 1308 return -EIO; 1296 1309 } 1297 1310 /* OK. We're happy */ ··· 1383 1400 f->inocache = ic; 1384 1401 1385 1402 ret = jffs2_do_read_inode_internal(c, f, &n); 1386 - if (!ret) { 1387 - mutex_unlock(&f->sem); 1388 - jffs2_do_clear_inode(c, f); 1389 - } 1403 + mutex_unlock(&f->sem); 1404 + jffs2_do_clear_inode(c, f); 1390 1405 jffs2_xattr_do_crccheck_inode(c, ic); 1391 1406 kfree (f); 1392 1407 return ret;