[lustre-devel] [bug report] staging: lustre: lov: Ensure correct operation for large object sizes

[Dan Carpenter]

On Mon, Aug 14, 2017 at 04:09:34PM +0100, James Simmons wrote:
> 
> > Hello Nathaniel Clark,
> > 
> > The patch 476f575cf070: "staging: lustre: lov: Ensure correct
> > operation for large object sizes" from Jul 26, 2017, leads to the
> > following static checker warning:
> > 
> > 	drivers/staging/lustre/lustre/lov/lov_ea.c:207 lsm_unpackmd_common()
> > 	warn: signed overflow undefined. 'min_stripe_maxbytes * stripe_count < min_stripe_maxbytes'
> > 
> > drivers/staging/lustre/lustre/lov/lov_ea.c
> >    148  static int lsm_unpackmd_common(struct lov_obd *lov,
> >    149                                 struct lov_stripe_md *lsm,
> >    150                                 struct lov_mds_md *lmm,
> >    151                                 struct lov_ost_data_v1 *objects)
> >    152  {
> >    153          loff_t min_stripe_maxbytes = 0;
> >                 ^^^^^^
> > loff_t is long long.
> > 
> >    154          unsigned int stripe_count;
> >    155          struct lov_oinfo *loi;
> >    156          loff_t lov_bytes;
> >    157          unsigned int i;
> >    158  
> >    159          /*
> >    160           * This supposes lov_mds_md_v1/v3 first fields are
> >    161           * are the same
> >    162           */
> >    163          lmm_oi_le_to_cpu(&lsm->lsm_oi, &lmm->lmm_oi);
> >    164          lsm->lsm_stripe_size = le32_to_cpu(lmm->lmm_stripe_size);
> >    165          lsm->lsm_pattern = le32_to_cpu(lmm->lmm_pattern);
> >    166          lsm->lsm_layout_gen = le16_to_cpu(lmm->lmm_layout_gen);
> >    167          lsm->lsm_pool_name[0] = '\0';
> >    168  
> >    169          stripe_count = lsm_is_released(lsm) ? 0 : lsm->lsm_stripe_count;
> >    170  
> >    171          for (i = 0; i < stripe_count; i++) {
> >    172                  loi = lsm->lsm_oinfo[i];
> >    173                  ostid_le_to_cpu(&objects[i].l_ost_oi, &loi->loi_oi);
> >    174                  loi->loi_ost_idx = le32_to_cpu(objects[i].l_ost_idx);
> >    175                  loi->loi_ost_gen = le32_to_cpu(objects[i].l_ost_gen);
> >    176                  if (lov_oinfo_is_dummy(loi))
> >    177                          continue;
> >    178  
> >    179                  if (loi->loi_ost_idx >= lov->desc.ld_tgt_count &&
> >    180                      !lov2obd(lov)->obd_process_conf) {
> >    181                          CERROR("%s: OST index %d more than OST count %d\n",
> >    182                                 (char *)lov->desc.ld_uuid.uuid,
> >    183                                 loi->loi_ost_idx, lov->desc.ld_tgt_count);
> >    184                          lov_dump_lmm_v1(D_WARNING, lmm);
> >    185                          return -EINVAL;
> >    186                  }
> >    187  
> >    188                  if (!lov->lov_tgts[loi->loi_ost_idx]) {
> >    189                          CERROR("%s: OST index %d missing\n",
> >    190                                 (char *)lov->desc.ld_uuid.uuid,
> >    191                                 loi->loi_ost_idx);
> >    192                          lov_dump_lmm_v1(D_WARNING, lmm);
> >    193                          continue;
> >    194                  }
> >    195  
> >    196                  lov_bytes = lov_tgt_maxbytes(lov->lov_tgts[loi->loi_ost_idx]);
> >    197                  if (min_stripe_maxbytes == 0 || lov_bytes < min_stripe_maxbytes)
> >    198                          min_stripe_maxbytes = lov_bytes;
> >    199          }
> >    200  
> >    201          if (min_stripe_maxbytes == 0)
> >    202                  min_stripe_maxbytes = LUSTRE_EXT3_STRIPE_MAXBYTES;
> >    203  
> >    204          stripe_count = lsm->lsm_stripe_count ?: lov->desc.ld_tgt_count;
> >    205          lov_bytes = min_stripe_maxbytes * stripe_count;
> >                             ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
> > This is undefined in C.
> > 
> >    206  
> >    207          if (lov_bytes < min_stripe_maxbytes) /* handle overflow */
> >                     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
> > So this might be wrong.
> > 
> >    208                  lsm->lsm_maxbytes = MAX_LFS_FILESIZE;
> >    209          else
> >    210                  lsm->lsm_maxbytes = lov_bytes;
> >    211  
> >    212          return 0;
> >    213  }
> 
> Dan what exact command did you use to find this bug? We do use smatch to 
> find these kinds of issues before patches land but some how we are missing
> this class from time to time.
> 
> Just to let you know the bug is being tracked under 
> 
> https://jira.hpdd.intel.com/browse/LU-9862
> 
> We do have a patch as well under going testing and review.

It's something I hadn't pushed.  I'll push that check right now.  But
it has few warnings and I'm not actually sure it matters with the
kernel.

regards,
dan carpenter