On Mon, Mar 20, 2023 at 6:56 AM Joel Sherrill <j...@rtems.org> wrote: > > New issue from Coveriry. > > I vaguely recall that JFFS2 uses dynamic checks for things that static would > work and Coveriry spots the dead code > > ---------- Forwarded message --------- > From: <scan-ad...@coverity.com> > Date: Sun, Mar 19, 2023, 10:59 PM > Subject: New Defects reported by Coverity Scan for RTEMS > To: <bu...@rtems.org> > > > Hi, > > Please find the latest report on new defect(s) introduced to RTEMS found with > Coverity Scan. > > 4 new defect(s) introduced to RTEMS found with Coverity Scan. > > > New defect(s) Reported-by: Coverity Scan > Showing 4 of 4 defect(s) > > > ** CID 1523448: Control flow issues (DEADCODE) > /cpukit/libfs/src/jffs2/src/gc.c: 137 in jffs2_garbage_collect_pass() > > > ________________________________________________________________________________________________________ > *** CID 1523448: Control flow issues (DEADCODE) > /cpukit/libfs/src/jffs2/src/gc.c: 137 in jffs2_garbage_collect_pass() > 131 struct jffs2_raw_node_ref *raw; > 132 uint32_t gcblock_dirty; > 133 int ret = 0, inum, nlink; > 134 int xattr = 0; > 135 > 136 if (mutex_lock_interruptible(&c->alloc_sem)) > >>> CID 1523448: Control flow issues (DEADCODE) > >>> Execution cannot reach this statement: "return -4;". > 137 return -EINTR; > 138 > 139 > 140 for (;;) { > 141 /* We can't start doing GC until we've finished > checking > 142 the node CRCs etc. */ >
This is a bug. It can be fixed by removing the error handling, because mutex_lock_interruptible has no return value. > ** CID 1523447: Null pointer dereferences (FORWARD_NULL) > /cpukit/libfs/src/jffs2/src/fs-rtems.c: 1370 in rtems_jffs2_initialize() > > > ________________________________________________________________________________________________________ > *** CID 1523447: Null pointer dereferences (FORWARD_NULL) > /cpukit/libfs/src/jffs2/src/fs-rtems.c: 1370 in rtems_jffs2_initialize() > 1364 } else { > 1365 err = -ENOMEM; > 1366 } > 1367 > 1368 sb = &fs_info->sb; > 1369 c = JFFS2_SB_INFO(sb); > >>> CID 1523447: Null pointer dereferences (FORWARD_NULL) > >>> Dereferencing null pointer "c". > 1370 c->mtd = NULL; > 1371 > 1372 if (err == 0) { > 1373 rtems_recursive_mutex_init(&sb->s_mutex, > RTEMS_FILESYSTEM_TYPE_JFFS2); > 1374 } > 1375 > This is a bug, because c == &sb->jffs2_info == &fs_info->sb == fs_info. Protect this dereference with `if (err == 0)`. I think the sequence of code starting with `sb = ...` can be moved into the `if` block at line 1372. > ** CID 1330065: Incorrect expression (PW.ASSIGN_WHERE_COMPARE_MEANT) > /cpukit/libfs/src/jffs2/src/wbuf.c: 651 in () > > > ________________________________________________________________________________________________________ > *** CID 1330065: Incorrect expression (PW.ASSIGN_WHERE_COMPARE_MEANT) > /cpukit/libfs/src/jffs2/src/wbuf.c: 651 in () > 645 goto wfail; > 646 } else if (retlen != c->wbuf_pagesize) { > 647 pr_warn("jffs2_flush_wbuf(): Write was short: %zd > instead of %d\n", > 648 retlen, c->wbuf_pagesize); > 649 ret = -EIO; > 650 goto wfail; > >>> CID 1330065: Incorrect expression (PW.ASSIGN_WHERE_COMPARE_MEANT) > >>> use of "=" where "==" may have been intended > 651 } else if ((ret = jffs2_verify_write(c, c->wbuf, > c->wbuf_ofs))) { > 652 wfail: > 653 jffs2_wbuf_recover(c); > 654 > 655 return ret; > 656 } > looks like a false positive. > ** CID 100676: Resource leaks (RESOURCE_LEAK) > /cpukit/libfs/src/jffs2/src/scan.c: 290 in jffs2_scan_medium() > > > ________________________________________________________________________________________________________ > *** CID 100676: Resource leaks (RESOURCE_LEAK) > /cpukit/libfs/src/jffs2/src/scan.c: 290 in jffs2_scan_medium() > 284 if (buf_size) > 285 kfree(flashbuf); > 286 #ifndef __ECOS > 287 else > 288 mtd_unpoint(c->mtd, 0, c->mtd->size); > 289 #endif > >>> CID 100676: Resource leaks (RESOURCE_LEAK) > >>> Variable "flashbuf" going out of scope leaks the storage it points to. > 290 return ret; > 291 } > 292 > 293 static int jffs2_fill_scan_buf(struct jffs2_sb_info *c, void *buf, > 294 uint32_t ofs, uint32_t len) > 295 { > I think this is a false positive. I don't see how the flashbuf can be allocated, while buf_size be zero. > > ________________________________________________________________________________________________________ > To view the defects in Coverity Scan visit, > https://u15810271.ct.sendgrid.net/ls/click?upn=HRESupC-2F2Czv4BOaCWWCy7my0P0qcxCbhZ31OYv50ypUUzi-2FdSNmuyRB7BEFT8xQ4-2B8hpujh0hTgQljRGId4Dg-3D-3D4ud5_EU3W9teASMK00lBXX9WT4lsogDrkCcNZLvg-2FVxwAXMpnugfe7VtmjlezAUiUAqURm-2BYADlhnMIumDCuBe7mGT9aB8TE9cxvKFYrBHVJ7mgUGDbfjluxq0RzsXGFS9SkOWyeEfvDu1fFzYW3HwPYe1Rb-2FBCAMDNLGqgmVE-2Bb4qg5R5mDe6-2FFbWF4SiJvT-2FGVjdNh1OVArH-2Ff8IlRB6Y43MQ-3D-3D > > _______________________________________________ > build mailing list > bu...@rtems.org > http://lists.rtems.org/mailman/listinfo/build > _______________________________________________ > devel mailing list > devel@rtems.org > http://lists.rtems.org/mailman/listinfo/devel _______________________________________________ devel mailing list devel@rtems.org http://lists.rtems.org/mailman/listinfo/devel