UBIFS: Fix code issues detected by Fortify
Subsystems (2)
PATCH 1/1 UBIFS: Fix code issues detected by Fortify
Hi Artem,
With the help of Fortify, I found some problems in source code. I would like to fix them in this patch.
Please help me review them!
There is no need to add a return value checking in journal.c, just in order to avoid a "value never read" warning. Remove statements in other files for the same reason.
Thanks!
----------------------------------------------------------
I would like to add these changes to UBIFS.
They are detected by Fortify.
journal.c
* Add return value checking
lpt.c
* Remove useless statements
* Add missing break
lpt_commit.c
* Remove useless statements
* Add error handling
orphan.c
* Remove duplicate code
sb.c
* Add error handling
* @default_compr should never less than zero
super.c
* @compr_type should never less than zero
tnc.c & tnc_commit.c
* Remove useless statments
--- fs/ubifs/journal.c | 3 +++ fs/ubifs/lpt.c | 5 ++--- fs/ubifs/lpt_commit.c | 7 +++++-- fs/ubifs/orphan.c | 1 - fs/ubifs/sb.c | 4 +++- fs/ubifs/super.c | 2 +- fs/ubifs/tnc.c | 1 - fs/ubifs/tnc_commit.c | 1 - 8 files changed, 14 insertions(+), 10 deletions(-)
fs/ubifs/journal.c +3 -0
@@ -389,6 +389,9 @@ out:ubifs_dump_budg (c, &c->bi);ubifs_dump_lprops (c); cmt_retries =dbg_check_lprops (c); +if (cmt_retries) +ubifs_err ("fail to check lprops, error %d" , + cmt_retries);up_write (&c->commit_sem); }return err;
fs/ubifs/lpt.c +2 -3
@@ -1464,7 +1464,6 @@ struct ubifs_lprops *ubifs_lpt_lookup(struct ubifs_info *c, int lnum)return ERR_CAST (nnode); } iip = ((i >> shft) & (UBIFS_LPT_FANOUT -1 )); - shft -=UBIFS_LPT_FANOUT_SHIFT ; pnode =ubifs_get_pnode (c, nnode, iip);if (IS_ERR (pnode))return ERR_CAST (pnode);
@@ -1604,7 +1603,6 @@ struct ubifs_lprops *ubifs_lpt_lookup_dirty(struct ubifs_info *c, int lnum)return ERR_CAST (nnode); } iip = ((i >> shft) & (UBIFS_LPT_FANOUT -1 )); - shft -=UBIFS_LPT_FANOUT_SHIFT ; pnode =ubifs_get_pnode (c, nnode, iip);if (IS_ERR (pnode))return ERR_CAST (pnode);
@@ -1964,7 +1962,6 @@ again: } } iip = ((i >> shft) & (UBIFS_LPT_FANOUT -1 )); - shft -=UBIFS_LPT_FANOUT_SHIFT ; pnode =scan_get_pnode (c, path + h, nnode, iip);if (IS_ERR (pnode)) { err =PTR_ERR (pnode);
@@ -2198,6 +2195,7 @@ static int dbg_chk_pnode(struct ubifs_info *c, struct ubifs_pnode *pnode, lprops->dirty);return -EINVAL ; } +break ;case LPROPS_FREEABLE :case LPROPS_FRDI_IDX :if (lprops->free + lprops->dirty != c->leb_size) {
@@ -2206,6 +2204,7 @@ static int dbg_chk_pnode(struct ubifs_info *c, struct ubifs_pnode *pnode, lprops->dirty);return -EINVAL ; } +break ; } }return 0 ;
fs/ubifs/lpt_commit.c +5 -2
@@ -304,7 +304,6 @@ static int layout_cnodes(struct ubifs_info *c)ubifs_assert (lnum >= c->lpt_first && lnum <= c->lpt_last); } - done_ltab =1 ; c->ltab_lnum = lnum; c->ltab_offs = offs; offs += c->ltab_sz;
@@ -514,7 +513,6 @@ static int write_cnodes(struct ubifs_info *c)if (err)return err; } - done_ltab =1 ;ubifs_pack_ltab (c, buf + offs, c->ltab_cmt); offs += c->ltab_sz;dbg_chk_lpt_sz (c,1 , c->ltab_sz);
@@ -1941,6 +1939,11 @@ static void dump_lpt_leb(const struct ubifs_info *c, int lnum)pr_err ("LEB %d:%d, nnode, " , lnum, offs); err =ubifs_unpack_nnode (c, p, &nnode); +if (err) { +pr_err ("fail to unpack_nnode, error %d\n" , + err); +break ; + }for (i =0 ; i <UBIFS_LPT_FANOUT ; i++) {pr_cont ("%d:%d" , nnode.nbranch[i].lnum, nnode.nbranch[i].offs);
fs/ubifs/orphan.c +0 -1
@@ -346,7 +346,6 @@ static int write_orph_nodes(struct ubifs_info *c, int atomic)int lnum;/* Unmap any unused LEBs after consolidation */ - lnum = c->ohead_lnum +1 ;for (lnum = c->ohead_lnum +1 ; lnum <= c->orph_last; lnum++) { err =ubifs_leb_unmap (c, lnum);if (err)
fs/ubifs/sb.c +3 -1
@@ -332,6 +332,8 @@ static int create_default_filesystem(struct ubifs_info *c) cs->ch.node_type =UBIFS_CS_NODE ; err =ubifs_write_node (c, cs,UBIFS_CS_NODE_SZ ,UBIFS_LOG_LNUM ,0 );kfree (cs); +if (err) +return err;ubifs_msg ("default file-system created" );return 0 ;
@@ -447,7 +449,7 @@ static int validate_sb(struct ubifs_info *c, struct ubifs_sb_node *sup) goto failed; } -if (c->default_compr <0 || c->default_compr >=UBIFS_COMPR_TYPES_CNT ) { +if (c->default_compr >=UBIFS_COMPR_TYPES_CNT ) { err =13 ; goto failed; }
fs/ubifs/super.c +1 -1
@@ -75,7 +75,7 @@ static int validate_inode(struct ubifs_info *c, const struct inode *inode)return 1 ; } -if (ui->compr_type <0 || ui->compr_type >=UBIFS_COMPR_TYPES_CNT ) { +if (ui->compr_type >=UBIFS_COMPR_TYPES_CNT ) {ubifs_err ("unknown compression type %d" , ui->compr_type);return 2 ; }
fs/ubifs/tnc.c +0 -1
@@ -3293,7 +3293,6 @@ int dbg_check_inode_size(struct ubifs_info *c, const struct inode *inode, goto out_unlock;if (err) { - err = -EINVAL ; key = &from_key; goto out_dump; }
fs/ubifs/tnc_commit.c +0 -1
@@ -389,7 +389,6 @@ static int layout_in_gaps(struct ubifs_info *c, int cnt)ubifs_dump_lprops (c); }/* Try to commit anyway */ - err =0 ;break ; } p++;
Signature · 1 line
1.8.1.4
Quoted 10 lines · hujianyang
Hi Artem,
With the help of Fortify, I found some problems in source code. I would like to fix them in this patch.
Please help me review them!
There is no need to add a return value checking in
journal.c, just in order to avoid a "value never read" warning. Remove statements in other files for the same reason.
Would you please just split this patch on smaller ones and submit the series, I'll pick it up.
Signature · 2 lines
Best Regards, Artem Bityutskiy
Quoted 5 lines
Would you please just split this patch on smaller ones and submit the series, I'll pick it up.
Sure, I will do that~!
Thanks~!