[bug report] udf: merge bh free

[Dan Carpenter <dan.carpenter@linaro.org>]

Hello Fabian Frederick,

Commit 02d4ca49fa22 ("udf: merge bh free") from Jan 6, 2017
(linux-next), leads to the following Smatch static checker warning:

	fs/udf/namei.c:442 udf_mkdir()
	warn: passing positive error code '(-117),(-28),(-22),(-12),(-5),(-1),1' to 'ERR_PTR'

fs/udf/namei.c
    422 static struct dentry *udf_mkdir(struct mnt_idmap *idmap, struct inode *dir,
    423                                 struct dentry *dentry, umode_t mode)
    424 {
    425         struct inode *inode;
    426         struct udf_fileident_iter iter;
    427         int err;
    428         struct udf_inode_info *dinfo = UDF_I(dir);
    429         struct udf_inode_info *iinfo;
    430 
    431         inode = udf_new_inode(dir, S_IFDIR | mode);
    432         if (IS_ERR(inode))
    433                 return ERR_CAST(inode);
    434 
    435         iinfo = UDF_I(inode);
    436         inode->i_op = &udf_dir_inode_operations;
    437         inode->i_fop = &udf_dir_operations;
    438         err = udf_fiiter_add_entry(inode, NULL, &iter);
    439         if (err) {
    440                 clear_nlink(inode);
    441                 discard_new_inode(inode);
--> 442                 return ERR_PTR(err);

Returning ERR_PTR(1) will lead to an Oops in the caller.

    443         }

The issue is this code from inode_getblk():

fs/udf/inode.c
   787          /*
   788           * Move prev_epos and cur_epos into indirect extent if we are at
   789           * the pointer to it
   790           */
   791          ret = udf_next_aext(inode, &prev_epos, &tmpeloc, &tmpelen, &tmpetype, 0);
   792          if (ret < 0)
   793                  goto out_free;
   794          ret = udf_next_aext(inode, &cur_epos, &tmpeloc, &tmpelen, &tmpetype, 0);
                ^^^^^^^^^^^^^^^^^^^
ret is set here.  It can be a negative error code, zero for EOF or one
on success.

   795          if (ret < 0)
   796                  goto out_free;
   797  
   798          /* if the extent is allocated and recorded, return the block
   799             if the extent is not a multiple of the blocksize, round up */
   800  
   801          if (!isBeyondEOF && etype == (EXT_RECORDED_ALLOCATED >> 30)) {
   802                  if (elen & (inode->i_sb->s_blocksize - 1)) {
   803                          elen = EXT_RECORDED_ALLOCATED |
   804                                  ((elen + inode->i_sb->s_blocksize - 1) &
   805                                   ~(inode->i_sb->s_blocksize - 1));
   806                          iinfo->i_lenExtents =
   807                                  ALIGN(iinfo->i_lenExtents,
   808                                        inode->i_sb->s_blocksize);
   809                          udf_write_aext(inode, &cur_epos, &eloc, elen, 1);
   810                  }
   811                  map->oflags = UDF_BLK_MAPPED;
   812                  map->pblk = udf_get_lb_pblock(inode->i_sb, &eloc, offset);
   813                  goto out_free;

Smatch is concerned that the ret = 1 from this goto out_free gets
propagated back to the caller.

   814          }
   815  


This seems intentional.  The caller has similar code earlier earlier but
it won't be triggered on this path because UDF_MAP_CREATE is set.

   405  static int udf_map_block(struct inode *inode, struct udf_map_rq *map)
   406  {
   407          int ret;
   408          struct udf_inode_info *iinfo = UDF_I(inode);
   409  
   410          if (WARN_ON_ONCE(iinfo->i_alloc_type == ICBTAG_FLAG_AD_IN_ICB))
   411                  return -EFSCORRUPTED;
   412  
   413          map->oflags = 0;
   414          if (!(map->iflags & UDF_MAP_CREATE)) {
   415                  struct kernel_lb_addr eloc;
   416                  uint32_t elen;
   417                  sector_t offset;
   418                  struct extent_position epos = {};
   419                  int8_t etype;
   420  
   421                  down_read(&iinfo->i_data_sem);
   422                  ret = inode_bmap(inode, map->lblk, &epos, &eloc, &elen, &offset,
   423                                   &etype);
   424                  if (ret < 0)
   425                          goto out_read;
   426                  if (ret > 0 && etype == (EXT_RECORDED_ALLOCATED >> 30)) {
   427                          map->pblk = udf_get_lb_pblock(inode->i_sb, &eloc,
   428                                                          offset);
   429                          map->oflags |= UDF_BLK_MAPPED;
   430                          ret = 0;
   431                  }
   432  out_read:
   433                  up_read(&iinfo->i_data_sem);
   434                  brelse(epos.bh);
   435  
   436                  return ret;

ret could be either zero or one here.

   437          }

It's unclear what to do...

regards,
dan carpenter