Sam Price commented: https://gitlab.rtems.org/rtems/rtos/rtems/-/merge_requests/1440#note_157676 The two diagnostics this change answers, from `gcc -fanalyzer` at 15.2.0, built for `riscv/mbv`: ``` ../../../cpukit/libfs/src/jffs2/src/fs-rtems.c:1093:22: warning: use after 'free' of 'root_i' [CWE-416] [-Wanalyzer-use-after-free] ../../../cpukit/libfs/src/jffs2/src/fs-rtems.c:1098:9: warning: double-'free' of 'root_i' [CWE-415] [-Wanalyzer-double-free] ``` Both are false positives. The analyser assumes `jffs2_iput()` frees its argument, which it only does when `i_count` reaches zero *and* `i_nlink` is zero. The assert above the call fixes `i_count` at 1, so it turns on the root inode's link count, which is not visible at that point. The test settles it by measurement: nine mount and unmount cycles, `malloc_walk()` after each, and a resource snapshot compared across them. The full analyser output, with the event paths it reports, is below. <details> <summary>gcc -fanalyzer, both diagnostics with event paths (247 lines)</summary> ``` ../../../cpukit/libfs/src/jffs2/src/fs-rtems.c:1093:22: warning: use after 'free' of 'root_i' [CWE-416] [-Wanalyzer-use-after-free] 1093 | assert(root_i->i_cache_next == NULL); | ~~~~~~^~~~~~~~~~~~~~ 'rtems_jffs2_fsunmount': events 1-2 │ │ 1073 | static void rtems_jffs2_fsunmount(rtems_filesystem_mount_table_entry_t *mt_entry) │ | ^~~~~~~~~~~~~~~~~~~~~ │ | | │ | (1) entry to 'rtems_jffs2_fsunmount' │...... │ 1092 | icache_evict(root_i, NULL); │ | ~~~~~~~~~~~~~~~~~~~~~~~~~~ │ | | │ | (2) calling 'icache_evict' from 'rtems_jffs2_fsunmount' │ └──> 'icache_evict': events 3-8 │ │ 71 | static void icache_evict(struct _inode *root_i, struct _inode *i) │ | ^~~~~~~~~~~~ │ | | │ | (3) entry to 'icache_evict' │...... │ 80 | while (this) { │ | ~~~~ │ | | │ | (4) following 'true' branch (when 'this' is non-NULL)... ─>─┐ │ | │ │ | │ │ |┌───────────────────────────────────────────────────────────────────────────┘ │ 81 |│ next = this->i_cache_next; │ |│ ~~~~~~~~~~~~~~~~~~~~~~~~~ │ |│ | │ |└────────────────────>(5) ...to here │ 82 | if (this != i && this->i_count == 0) { │ | ~ │ | | │ | (6) following 'true' branch... ─>─┐ │ | │ │ | │ │ |┌─────────────────────────────────────────────────────┘ │ 83 |│ struct _inode *parent = this->i_parent; │ |│ ~~~~~~ │ |│ | │ |└──────────────────────────────────────>(7) ...to here │...... │ 90 | free(this); │ | ~~~~~~~~~~ │ | | │ | (8) freed here │ <──────┘ │ 'rtems_jffs2_fsunmount': event 9 │ │ 1092 | icache_evict(root_i, NULL); │ | ^~~~~~~~~~~~~~~~~~~~~~~~~~ │ | | │ | (9) returning to 'rtems_jffs2_fsunmount' from 'icache_evict' │ 'rtems_jffs2_fsunmount': event 10 │ │ 1093 | assert(root_i->i_cache_next == NULL); │ | ~~~~~~^~~~~~~~~~~~~~ │ | | │ | (10) ⚠️ use after 'free' of 'root_i'; freed at (8) │ ../../../cpukit/libfs/src/jffs2/src/fs-rtems.c:1098:9: warning: double-'free' of 'root_i' [CWE-415] [-Wanalyzer-double-free] 1098 | free(root_i); | ^~~~~~~~~~~~ 'rtems_jffs2_fsunmount': events 1-2 │ │ 1073 | static void rtems_jffs2_fsunmount(rtems_filesystem_mount_table_entry_t *mt_entry) │ | ^~~~~~~~~~~~~~~~~~~~~ │ | | │ | (1) entry to 'rtems_jffs2_fsunmount' │...... │ 1092 | icache_evict(root_i, NULL); │ | ~~~~~~~~~~~~~~~~~~~~~~~~~~ │ | | │ | (2) calling 'icache_evict' from 'rtems_jffs2_fsunmount' │ └──> 'icache_evict': events 3-4 │ │ 71 | static void icache_evict(struct _inode *root_i, struct _inode *i) │ | ^~~~~~~~~~~~ │ | | │ | (3) entry to 'icache_evict' │...... │ 82 | if (this != i && this->i_count == 0) { │ | ~ │ | | │ | (4) following 'true' branch (when 'this != i')... ─>─┐ │ | │ │ 'icache_evict': event 5 │ │ | │ │ |┌────────────────────────────────────────────────────────────────────────┘ │ 82 |│ if (this != i && this->i_count == 0) { │ |│ ~~~~^~~~~~~~~ │ |│ | │ |└────────────────────────────────────>(5) ...to here │ 'icache_evict': event 6 │ │ 82 | if (this != i && this->i_count == 0) { │ | ~~~~~~~~~~^~~~~~~~~~~~~~~~~~~~~ │ | | │ | (6) following 'false' branch... ─>─┐ │ | │ │ 'icache_evict': event 7 │ │cc1: │ (7): ...to here │ <──────┘ │ 'rtems_jffs2_fsunmount': event 8 │ │ 1092 | icache_evict(root_i, NULL); │ | ^~~~~~~~~~~~~~~~~~~~~~~~~~ │ | | │ | (8) returning to 'rtems_jffs2_fsunmount' from 'icache_evict' │ 'rtems_jffs2_fsunmount': event 9 │ │ 1093 | assert(root_i->i_cache_next == NULL); │ | ~~~~~~^~~~~~~~~~~~~~ │ | | │ | (9) state of 'INIT_VAL((*INIT_VAL((*INIT_VAL(mt_entry_9(D))).mt_fs_root)).location.node_access)': 'start' -> 'assumed-non-null' (NULL origin) │ 'rtems_jffs2_fsunmount': event 10 │ │ 1093 | assert(root_i->i_cache_next == NULL); │ | ^~~~~~ │ | | │ | (10) following 'false' branch... ─>─┐ │ | │ │ 'rtems_jffs2_fsunmount': event 11 │ │ | │ │ |┌────────────────────────────────────────────┘ │ 1094 |│ assert(root_i->i_count == 1); │ |│ ~~~~~~^~~~~~~~~ │ |│ | │ |└────────────────────>(11) ...to here │ 'rtems_jffs2_fsunmount': event 12 │ │ 1094 | assert(root_i->i_count == 1); │ | ^~~~~~ │ | | │ | (12) following 'false' branch... ─>─┐ │ | │ │ 'rtems_jffs2_fsunmount': events 13-14 │ │ | │ │ |┌────────────────────────────────────────────┘ │ 1095 |│ jffs2_iput(root_i); │ |│ ^~~~~~~~~~~~~~~~~~ │ |│ | │ |└───────>(13) ...to here │ | (14) calling 'jffs2_iput' from 'rtems_jffs2_fsunmount' │ └──> 'jffs2_iput': event 15 │ │ 1617 | void jffs2_iput(struct _inode *i) │ | ^~~~~~~~~~ │ | | │ | (15) entry to 'jffs2_iput' │ 'jffs2_iput': event 16 │ │ 1624 | assert(i != NULL); │ | ^~~~~~ │ | | │ | (16) following 'false' branch (when 'i' is non-NULL)... ─>─┐ │ | │ │ 'jffs2_iput': events 17-21 │ │ | │ │ |┌───────────────────────────────────────────────────────────────────┘ │ 1626 |│ i->i_count--; │ |│ ~^~~~~~~~~ │ |│ | │ |└────────>(17) ...to here │ 1627 | │ 1628 | if (i->i_count < 0) │ | ~ │ | | │ | (18) following 'false' branch... ─>─┐ │ | │ │...... │ | │ │ |┌───────────────────────────────────────────────┘ │ 1631 |│ if (i->i_count) │ |│ ~ │ |│ | │ |└──────────>(19) ...to here │ | (20) following 'false' branch... ─>─┐ │ | │ │...... │ | │ │ |┌───────────────────────────────────────────────┘ │ 1634 |│ if (!i->i_nlink) { │ |│ ~~~~~~~~~~ │ |│ | │ |└─────────────>(21) ...to here │ 'jffs2_iput': events 22-24 │ │ 1634 | if (!i->i_nlink) { │ | ^ │ | | │ | (22) following 'true' branch... ─>─┐ │ | │ │...... │ | │ │ |┌──────────────────────────────────────────────┘ │ 1638 |│ if (i->i_cache_prev) │ |│ ~~~~~~~~~~~~~~~ │ |│ | │ |└────────────────────>(23) ...to here │...... │ 1646 | free(i); │ | ~~~~~~~ │ | | │ | (24) first 'free' here │ <──────┘ │ 'rtems_jffs2_fsunmount': events 25-26 │ │ 1095 | jffs2_iput(root_i); │ | ^~~~~~~~~~~~~~~~~~ │ | | │ | (25) returning to 'rtems_jffs2_fsunmount' from 'jffs2_iput' │...... │ 1098 | free(root_i); │ | ~~~~~~~~~~~~ │ | | │ | (26) ⚠️ second 'free' here; first 'free' was at (24) │ ``` </details> -- View it on GitLab: https://gitlab.rtems.org/rtems/rtos/rtems/-/merge_requests/1440#note_157676 You're receiving this email because of your account on gitlab.rtems.org. Unsubscribe from this thread: https://gitlab.rtems.org/-/sent_notifications/5-2635qsv5cljo0p9tugmdtjd22-1d/unsubscribe | Manage all notifications: https://gitlab.rtems.org/-/profile/notifications | Help: https://gitlab.rtems.org/help
_______________________________________________ bugs mailing list [email protected] http://lists.rtems.org/mailman/listinfo/bugs
