New Defects reported by Coverity Scan for RTEMS

[Gedare Bloom]

On Mon, Mar 20, 2023 at 6:56 AM Joel Sherrill <joel at 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-admin at coverity.com>
> Date: Sun, Mar 19, 2023, 10:59 PM
> Subject: New Defects reported by Coverity Scan for RTEMS
> To: <build at 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
> build at rtems.org
> http://lists.rtems.org/mailman/listinfo/build
> _______________________________________________
> devel mailing list
> devel at rtems.org
> http://lists.rtems.org/mailman/listinfo/devel