| Message ID | 20180613212344.11608-2-richard@nod.at |
|---|---|
| State | Superseded |
| Delegated to: | Richard Weinberger |
| Headers | show |
| Series | ubi: Fastmap updates | expand |
Hi Richard,
I love your patch! Perhaps something to improve:
[auto build test WARNING on mtd/master]
[also build test WARNING on v4.17 next-20180613]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]
url: https://github.com/0day-ci/linux/commits/Richard-Weinberger/ubi-Fastmap-updates/20180614-052830
base: git://git.infradead.org/linux-mtd.git master
reproduce:
# apt-get install sparse
make ARCH=x86_64 allmodconfig
make C=1 CF=-D__CHECK_ENDIAN__
sparse warnings: (new ones prefixed by >>)
>> drivers/mtd/ubi/fastmap.c:110:14: sparse: incorrect type in assignment (different base types) @@ expected restricted __be32 [usertype] pnum @@ got [usertype] pnum @@
drivers/mtd/ubi/fastmap.c:110:14: expected restricted __be32 [usertype] pnum
drivers/mtd/ubi/fastmap.c:110:14: got unsigned int
>> drivers/mtd/ubi/fastmap.c:111:13: sparse: restricted __be32 degrades to integer
>> drivers/mtd/ubi/fastmap.c:112:27: sparse: incorrect type in assignment (different base types) @@ expected int [signed] <noident> @@ got restricted __be3int [signed] <noident> @@
drivers/mtd/ubi/fastmap.c:112:27: expected int [signed] <noident>
drivers/mtd/ubi/fastmap.c:112:27: got restricted __be32 [usertype] pnum
drivers/mtd/ubi/fastmap.c:116:13: sparse: restricted __be32 degrades to integer
drivers/mtd/ubi/fastmap.c:116:25: sparse: restricted __be32 degrades to integer
drivers/mtd/ubi/fastmap.c:121:27: sparse: incorrect type in assignment (different base types) @@ expected int [signed] <noident> @@ got restricted __be3int [signed] <noident> @@
drivers/mtd/ubi/fastmap.c:121:27: expected int [signed] <noident>
drivers/mtd/ubi/fastmap.c:121:27: got restricted __be32 [usertype] pnum
drivers/mtd/ubi/fastmap.c:604:23: sparse: incorrect type in assignment (different base types) @@ expected unsigned long long [unsigned] max_sqnum @@ got nsigned long long [unsigned] max_sqnum @@
drivers/mtd/ubi/fastmap.c:604:23: expected unsigned long long [unsigned] max_sqnum
drivers/mtd/ubi/fastmap.c:604:23: got restricted __be64 [usertype] sqnum
drivers/mtd/ubi/fastmap.c:1075:17: sparse: incorrect type in assignment (different base types) @@ expected restricted __be32 [usertype] tmp_crc @@ got [usertype] tmp_crc @@
drivers/mtd/ubi/fastmap.c:1075:17: expected restricted __be32 [usertype] tmp_crc
drivers/mtd/ubi/fastmap.c:1075:17: got unsigned int
drivers/mtd/ubi/fastmap.c:1077:13: sparse: incorrect type in assignment (different base types) @@ expected restricted __be32 [usertype] crc @@ got [usertype] crc @@
drivers/mtd/ubi/fastmap.c:1077:13: expected restricted __be32 [usertype] crc
drivers/mtd/ubi/fastmap.c:1077:13: got unsigned int
drivers/mtd/ubi/fastmap.c:1086:22: sparse: incorrect type in assignment (different base types) @@ expected restricted __be64 [usertype] sqnum @@ got unsigned lonrestricted __be64 [usertype] sqnum @@
drivers/mtd/ubi/fastmap.c:1086:22: expected restricted __be64 [usertype] sqnum
drivers/mtd/ubi/fastmap.c:1086:22: got unsigned long long [unsigned] [assigned] sqnum
vim +110 drivers/mtd/ubi/fastmap.c
103
104 static bool read_pnum(struct ubi_device *ubi, struct ubi_attach_info *ai,
105 __be32 pnum, int *out_pnum)
106 {
107 int ret = true;
108 int max_pnum = ubi->peb_count;
109
> 110 pnum = be32_to_cpu(pnum);
> 111 if (pnum == UBI_UNKNOWN) {
> 112 *out_pnum = pnum;
113 goto out;
114 }
115
116 if (pnum < 0 || pnum >= max_pnum) {
117 ubi_err(ubi, "fastmap references PEB out of range: %i", pnum);
118 ret = false;
119 goto out;
120 } else {
121 *out_pnum = pnum;
122 }
123
124 out:
125 return ret;
126 }
127
---
0-DAY kernel test infrastructure Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all Intel Corporation
Am Donnerstag, 14. Juni 2018, 03:04:40 CEST schrieb kbuild test robot: > Hi Richard, > > I love your patch! Perhaps something to improve: > > [auto build test WARNING on mtd/master] > [also build test WARNING on v4.17 next-20180613] > [if your patch is applied to the wrong git tree, please drop us a note to help improve the system] > > url: https://github.com/0day-ci/linux/commits/Richard-Weinberger/ubi-Fastmap-updates/20180614-052830 > base: git://git.infradead.org/linux-mtd.git master > reproduce: > # apt-get install sparse > make ARCH=x86_64 allmodconfig > make C=1 CF=-D__CHECK_ENDIAN__ > > > sparse warnings: (new ones prefixed by >>) > > >> drivers/mtd/ubi/fastmap.c:110:14: sparse: incorrect type in assignment (different base types) @@ expected restricted __be32 [usertype] pnum @@ got [usertype] pnum @@ > drivers/mtd/ubi/fastmap.c:110:14: expected restricted __be32 [usertype] pnum > drivers/mtd/ubi/fastmap.c:110:14: got unsigned int > >> drivers/mtd/ubi/fastmap.c:111:13: sparse: restricted __be32 degrades to integer > >> drivers/mtd/ubi/fastmap.c:112:27: sparse: incorrect type in assignment (different base types) @@ expected int [signed] <noident> @@ got restricted __be3int [signed] <noident> @@ > drivers/mtd/ubi/fastmap.c:112:27: expected int [signed] <noident> > drivers/mtd/ubi/fastmap.c:112:27: got restricted __be32 [usertype] pnum > drivers/mtd/ubi/fastmap.c:116:13: sparse: restricted __be32 degrades to integer > drivers/mtd/ubi/fastmap.c:116:25: sparse: restricted __be32 degrades to integer > drivers/mtd/ubi/fastmap.c:121:27: sparse: incorrect type in assignment (different base types) @@ expected int [signed] <noident> @@ got restricted __be3int [signed] <noident> @@ > drivers/mtd/ubi/fastmap.c:121:27: expected int [signed] <noident> > drivers/mtd/ubi/fastmap.c:121:27: got restricted __be32 [usertype] pnum > drivers/mtd/ubi/fastmap.c:604:23: sparse: incorrect type in assignment (different base types) @@ expected unsigned long long [unsigned] max_sqnum @@ got nsigned long long [unsigned] max_sqnum @@ > drivers/mtd/ubi/fastmap.c:604:23: expected unsigned long long [unsigned] max_sqnum > drivers/mtd/ubi/fastmap.c:604:23: got restricted __be64 [usertype] sqnum > drivers/mtd/ubi/fastmap.c:1075:17: sparse: incorrect type in assignment (different base types) @@ expected restricted __be32 [usertype] tmp_crc @@ got [usertype] tmp_crc @@ > drivers/mtd/ubi/fastmap.c:1075:17: expected restricted __be32 [usertype] tmp_crc > drivers/mtd/ubi/fastmap.c:1075:17: got unsigned int > drivers/mtd/ubi/fastmap.c:1077:13: sparse: incorrect type in assignment (different base types) @@ expected restricted __be32 [usertype] crc @@ got [usertype] crc @@ > drivers/mtd/ubi/fastmap.c:1077:13: expected restricted __be32 [usertype] crc > drivers/mtd/ubi/fastmap.c:1077:13: got unsigned int > drivers/mtd/ubi/fastmap.c:1086:22: sparse: incorrect type in assignment (different base types) @@ expected restricted __be64 [usertype] sqnum @@ got unsigned lonrestricted __be64 [usertype] sqnum @@ > drivers/mtd/ubi/fastmap.c:1086:22: expected restricted __be64 [usertype] sqnum > drivers/mtd/ubi/fastmap.c:1086:22: got unsigned long long [unsigned] [assigned] sqnum > > vim +110 drivers/mtd/ubi/fastmap.c > > 103 > 104 static bool read_pnum(struct ubi_device *ubi, struct ubi_attach_info *ai, > 105 __be32 pnum, int *out_pnum) > 106 { > 107 int ret = true; > 108 int max_pnum = ubi->peb_count; > 109 > > 110 pnum = be32_to_cpu(pnum); > > 111 if (pnum == UBI_UNKNOWN) { > > 112 *out_pnum = pnum; > 113 goto out; > 114 } Okay, let's use a new variable of type int instead of reusing pnum. Thanks, //richard
On Wed, 13 Jun 2018 23:23:31 +0200 Richard Weinberger <richard@nod.at> wrote: > PEB numbers can be used as indices, make sure that they > are within bounds. > > Signed-off-by: Richard Weinberger <richard@nod.at> > --- > drivers/mtd/ubi/fastmap.c | 71 ++++++++++++++++++++++++++++++++++++++--------- > 1 file changed, 58 insertions(+), 13 deletions(-) > > diff --git a/drivers/mtd/ubi/fastmap.c b/drivers/mtd/ubi/fastmap.c > index 462526a10537..768fa8a76867 100644 > --- a/drivers/mtd/ubi/fastmap.c > +++ b/drivers/mtd/ubi/fastmap.c > @@ -101,6 +101,29 @@ size_t ubi_calc_fm_size(struct ubi_device *ubi) > return roundup(size, ubi->leb_size); > } > > +static bool read_pnum(struct ubi_device *ubi, struct ubi_attach_info *ai, > + __be32 pnum, int *out_pnum) Why not returning an int here, with negative values meaning that pnum is invalid and positive values encoding the actual PEB number? Also, I think check_pnum(), validate_pnum() or decode_pnum() would be better names than read_pnum(). > +{ > + int ret = true; > + int max_pnum = ubi->peb_count; > + > + pnum = be32_to_cpu(pnum); > + if (pnum == UBI_UNKNOWN) { > + *out_pnum = pnum; > + goto out; > + } > + > + if (pnum < 0 || pnum >= max_pnum) { > + ubi_err(ubi, "fastmap references PEB out of range: %i", pnum); > + ret = false; > + goto out; > + } else { > + *out_pnum = pnum; > + } > + > +out: > + return ret; > +} > > /** > * new_fm_vhdr - allocate a new volume header for fastmap usage. > @@ -438,7 +461,10 @@ static int scan_pool(struct ubi_device *ubi, struct ubi_attach_info *ai, > int scrub = 0; > int image_seq; > > - pnum = be32_to_cpu(pebs[i]); > + if (!read_pnum(ubi, ai, pebs[i], &pnum)) { > + ret = UBI_BAD_FASTMAP; > + goto out; > + } > > if (ubi_io_is_bad(ubi, pnum)) { > ubi_err(ubi, "bad PEB in fastmap pool!"); > @@ -565,7 +591,7 @@ static int ubi_attach_fastmap(struct ubi_device *ubi, > struct ubi_fm_ec *fmec; > struct ubi_fm_volhdr *fmvhdr; > struct ubi_fm_eba *fm_eba; > - int ret, i, j, pool_size, wl_pool_size; > + int ret, i, j, pool_size, wl_pool_size, pnum; > size_t fm_pos = 0, fm_size = ubi->fm_size; > unsigned long long max_sqnum = 0; > void *fm_raw = ubi->fm_buf; > @@ -647,8 +673,10 @@ static int ubi_attach_fastmap(struct ubi_device *ubi, > if (fm_pos >= fm_size) > goto fail_bad; > > - add_aeb(ai, &ai->free, be32_to_cpu(fmec->pnum), > - be32_to_cpu(fmec->ec), 0); > + if (!read_pnum(ubi, ai, fmec->pnum, &pnum)) > + goto fail_bad; > + > + add_aeb(ai, &ai->free, pnum, be32_to_cpu(fmec->ec), 0); > } > > /* read EC values from used list */ > @@ -658,8 +686,10 @@ static int ubi_attach_fastmap(struct ubi_device *ubi, > if (fm_pos >= fm_size) > goto fail_bad; > > - add_aeb(ai, &used, be32_to_cpu(fmec->pnum), > - be32_to_cpu(fmec->ec), 0); > + if (!read_pnum(ubi, ai, fmec->pnum, &pnum)) > + goto fail_bad; > + > + add_aeb(ai, &used, pnum, be32_to_cpu(fmec->ec), 0); > } > > /* read EC values from scrub list */ > @@ -669,8 +699,10 @@ static int ubi_attach_fastmap(struct ubi_device *ubi, > if (fm_pos >= fm_size) > goto fail_bad; > > - add_aeb(ai, &used, be32_to_cpu(fmec->pnum), > - be32_to_cpu(fmec->ec), 1); > + if (!read_pnum(ubi, ai, fmec->pnum, &pnum)) > + goto fail_bad; > + > + add_aeb(ai, &used, pnum, be32_to_cpu(fmec->ec), 1); > } > > /* read EC values from erase list */ > @@ -680,8 +712,10 @@ static int ubi_attach_fastmap(struct ubi_device *ubi, > if (fm_pos >= fm_size) > goto fail_bad; > > - add_aeb(ai, &ai->erase, be32_to_cpu(fmec->pnum), > - be32_to_cpu(fmec->ec), 1); > + if (!read_pnum(ubi, ai, fmec->pnum, &pnum)) > + goto fail_bad; > + > + add_aeb(ai, &ai->erase, pnum, be32_to_cpu(fmec->ec), 1); > } > > ai->mean_ec = div_u64(ai->ec_sum, ai->ec_count); > @@ -731,7 +765,8 @@ static int ubi_attach_fastmap(struct ubi_device *ubi, > } > > for (j = 0; j < be32_to_cpu(fm_eba->reserved_pebs); j++) { > - int pnum = be32_to_cpu(fm_eba->pnum[j]); > + if (!read_pnum(ubi, ai, fm_eba->pnum[j], &pnum)) > + goto fail_bad; > > if (pnum < 0) > continue; > @@ -954,7 +989,10 @@ int ubi_scan_fastmap(struct ubi_device *ubi, struct ubi_attach_info *ai, > for (i = 0; i < used_blocks; i++) { > int image_seq; > > - pnum = be32_to_cpu(fmsb->block_loc[i]); > + if (!read_pnum(ubi, ai, fmsb->block_loc[i], &pnum)) { > + ret = UBI_BAD_FASTMAP; > + goto free_hdr; > + } > > if (ubi_io_is_bad(ubi, pnum)) { > ret = UBI_BAD_FASTMAP; > @@ -1068,7 +1106,14 @@ int ubi_scan_fastmap(struct ubi_device *ubi, struct ubi_attach_info *ai, > goto free_hdr; > } > > - e->pnum = be32_to_cpu(fmsb2->block_loc[i]); > + if (!read_pnum(ubi, ai, fmsb2->block_loc[i], &e->pnum)) { > + while (i--) > + kmem_cache_free(ubi_wl_entry_slab, fm->e[i]); > + > + ret = -ENOMEM; > + goto free_hdr; > + } > + > e->ec = be32_to_cpu(fmsb2->block_ec[i]); > fm->e[i] = e; > }
diff --git a/drivers/mtd/ubi/fastmap.c b/drivers/mtd/ubi/fastmap.c index 462526a10537..768fa8a76867 100644 --- a/drivers/mtd/ubi/fastmap.c +++ b/drivers/mtd/ubi/fastmap.c @@ -101,6 +101,29 @@ size_t ubi_calc_fm_size(struct ubi_device *ubi) return roundup(size, ubi->leb_size); } +static bool read_pnum(struct ubi_device *ubi, struct ubi_attach_info *ai, + __be32 pnum, int *out_pnum) +{ + int ret = true; + int max_pnum = ubi->peb_count; + + pnum = be32_to_cpu(pnum); + if (pnum == UBI_UNKNOWN) { + *out_pnum = pnum; + goto out; + } + + if (pnum < 0 || pnum >= max_pnum) { + ubi_err(ubi, "fastmap references PEB out of range: %i", pnum); + ret = false; + goto out; + } else { + *out_pnum = pnum; + } + +out: + return ret; +} /** * new_fm_vhdr - allocate a new volume header for fastmap usage. @@ -438,7 +461,10 @@ static int scan_pool(struct ubi_device *ubi, struct ubi_attach_info *ai, int scrub = 0; int image_seq; - pnum = be32_to_cpu(pebs[i]); + if (!read_pnum(ubi, ai, pebs[i], &pnum)) { + ret = UBI_BAD_FASTMAP; + goto out; + } if (ubi_io_is_bad(ubi, pnum)) { ubi_err(ubi, "bad PEB in fastmap pool!"); @@ -565,7 +591,7 @@ static int ubi_attach_fastmap(struct ubi_device *ubi, struct ubi_fm_ec *fmec; struct ubi_fm_volhdr *fmvhdr; struct ubi_fm_eba *fm_eba; - int ret, i, j, pool_size, wl_pool_size; + int ret, i, j, pool_size, wl_pool_size, pnum; size_t fm_pos = 0, fm_size = ubi->fm_size; unsigned long long max_sqnum = 0; void *fm_raw = ubi->fm_buf; @@ -647,8 +673,10 @@ static int ubi_attach_fastmap(struct ubi_device *ubi, if (fm_pos >= fm_size) goto fail_bad; - add_aeb(ai, &ai->free, be32_to_cpu(fmec->pnum), - be32_to_cpu(fmec->ec), 0); + if (!read_pnum(ubi, ai, fmec->pnum, &pnum)) + goto fail_bad; + + add_aeb(ai, &ai->free, pnum, be32_to_cpu(fmec->ec), 0); } /* read EC values from used list */ @@ -658,8 +686,10 @@ static int ubi_attach_fastmap(struct ubi_device *ubi, if (fm_pos >= fm_size) goto fail_bad; - add_aeb(ai, &used, be32_to_cpu(fmec->pnum), - be32_to_cpu(fmec->ec), 0); + if (!read_pnum(ubi, ai, fmec->pnum, &pnum)) + goto fail_bad; + + add_aeb(ai, &used, pnum, be32_to_cpu(fmec->ec), 0); } /* read EC values from scrub list */ @@ -669,8 +699,10 @@ static int ubi_attach_fastmap(struct ubi_device *ubi, if (fm_pos >= fm_size) goto fail_bad; - add_aeb(ai, &used, be32_to_cpu(fmec->pnum), - be32_to_cpu(fmec->ec), 1); + if (!read_pnum(ubi, ai, fmec->pnum, &pnum)) + goto fail_bad; + + add_aeb(ai, &used, pnum, be32_to_cpu(fmec->ec), 1); } /* read EC values from erase list */ @@ -680,8 +712,10 @@ static int ubi_attach_fastmap(struct ubi_device *ubi, if (fm_pos >= fm_size) goto fail_bad; - add_aeb(ai, &ai->erase, be32_to_cpu(fmec->pnum), - be32_to_cpu(fmec->ec), 1); + if (!read_pnum(ubi, ai, fmec->pnum, &pnum)) + goto fail_bad; + + add_aeb(ai, &ai->erase, pnum, be32_to_cpu(fmec->ec), 1); } ai->mean_ec = div_u64(ai->ec_sum, ai->ec_count); @@ -731,7 +765,8 @@ static int ubi_attach_fastmap(struct ubi_device *ubi, } for (j = 0; j < be32_to_cpu(fm_eba->reserved_pebs); j++) { - int pnum = be32_to_cpu(fm_eba->pnum[j]); + if (!read_pnum(ubi, ai, fm_eba->pnum[j], &pnum)) + goto fail_bad; if (pnum < 0) continue; @@ -954,7 +989,10 @@ int ubi_scan_fastmap(struct ubi_device *ubi, struct ubi_attach_info *ai, for (i = 0; i < used_blocks; i++) { int image_seq; - pnum = be32_to_cpu(fmsb->block_loc[i]); + if (!read_pnum(ubi, ai, fmsb->block_loc[i], &pnum)) { + ret = UBI_BAD_FASTMAP; + goto free_hdr; + } if (ubi_io_is_bad(ubi, pnum)) { ret = UBI_BAD_FASTMAP; @@ -1068,7 +1106,14 @@ int ubi_scan_fastmap(struct ubi_device *ubi, struct ubi_attach_info *ai, goto free_hdr; } - e->pnum = be32_to_cpu(fmsb2->block_loc[i]); + if (!read_pnum(ubi, ai, fmsb2->block_loc[i], &e->pnum)) { + while (i--) + kmem_cache_free(ubi_wl_entry_slab, fm->e[i]); + + ret = -ENOMEM; + goto free_hdr; + } + e->ec = be32_to_cpu(fmsb2->block_ec[i]); fm->e[i] = e; }
PEB numbers can be used as indices, make sure that they are within bounds. Signed-off-by: Richard Weinberger <richard@nod.at> --- drivers/mtd/ubi/fastmap.c | 71 ++++++++++++++++++++++++++++++++++++++--------- 1 file changed, 58 insertions(+), 13 deletions(-)