On Mon, Apr 24, 2023 at 1:13 PM Jonathan Derrick
<jonathan.derrick@xxxxxxxxx> wrote:
>
>
>
> On 4/24/23 12:00 PM, Song Liu wrote:
> > Hi Dan,
> >
> > Thanks for the report!.
> >
> > On Fri, Apr 21, 2023 at 3:45 AM Dan Carpenter <dan.carpenter@xxxxxxxxxx> wrote:
> >>
> >> Hello Jon Derrick,
> >>
> >> The patch 10172f200b67: "md: Fix types in sb writer" from Feb 24,
> >> 2023, leads to the following Smatch static checker warning:
> >>
> >> drivers/md/md-bitmap.c:265 __write_sb_page()
> >> warn: unsigned 'offset' is never less than zero.
> >>
> >> drivers/md/md-bitmap.c
> >> 234 static int __write_sb_page(struct md_rdev *rdev, struct bitmap *bitmap,
> >> 235 struct page *page)
> >> 236 {
> >> 237 struct block_device *bdev;
> >> 238 struct mddev *mddev = bitmap->mddev;
> >> 239 struct bitmap_storage *store = &bitmap->storage;
> >> 240 sector_t offset = mddev->bitmap_info.offset;
> >> ^^^^^^^^
> >> offset used to be llof_t which is s64.
> >
> > Hi Jon,
> >
> > Please look into this soon.
> >
> > Thanks,
> > Song
> >
> >>
> >> 241 sector_t ps, sboff, doff;
> >> 242 unsigned int size = PAGE_SIZE;
> >> 243 unsigned int opt_size = PAGE_SIZE;
> >> 244
> >> 245 bdev = (rdev->meta_bdev) ? rdev->meta_bdev : rdev->bdev;
> >> 246 if (page->index == store->file_pages - 1) {
> >> 247 unsigned int last_page_size = store->bytes & (PAGE_SIZE - 1);
> >> 248
> >> 249 if (last_page_size == 0)
> >> 250 last_page_size = PAGE_SIZE;
> >> 251 size = roundup(last_page_size, bdev_logical_block_size(bdev));
> >> 252 opt_size = optimal_io_size(bdev, last_page_size, size);
> >> 253 }
> >> 254
> >> 255 ps = page->index * PAGE_SIZE / SECTOR_SIZE;
> >> 256 sboff = rdev->sb_start + offset;
> >> 257 doff = rdev->data_offset;
> >> 258
> >> 259 /* Just make sure we aren't corrupting data or metadata */
> >> 260 if (mddev->external) {
> >> 261 /* Bitmap could be anywhere. */
> >> 262 if (sboff + ps > doff &&
> >> 263 sboff < (doff + mddev->dev_sectors + PAGE_SIZE / SECTOR_SIZE))
> >> 264 return -EINVAL;
> >> --> 265 } else if (offset < 0) {
> >> ^^^^^^^^^^
> >> Now that it's a sector_t this is impossible.
> >>
> >> 266 /* DATA BITMAP METADATA */
> >> 267 size = bitmap_io_size(size, opt_size, offset + ps, 0);
> >> 268 if (size == 0)
> >> 269 /* bitmap runs in to metadata */
> >> 270 return -EINVAL;
> >> 271
> >> 272 if (doff + mddev->dev_sectors > sboff)
> >> 273 /* data runs in to bitmap */
> >> 274 return -EINVAL;
> >> 275 } else if (rdev->sb_start < rdev->data_offset) {
> >> 276 /* METADATA BITMAP DATA */
> >> 277 size = bitmap_io_size(size, opt_size, sboff + ps, doff);
> >> 278 if (size == 0)
> >> 279 /* bitmap runs in to data */
> >> 280 return -EINVAL;
> >> 281 } else {
> >> 282 /* DATA METADATA BITMAP - no problems */
> >> 283 }
> >> 284
> >> 285 md_super_write(mddev, rdev, sboff + ps, (int) size, page);
> >> 286 return 0;
> >> 287 }
> >>
> >> regards,
> >> dan carpenter
>
> Seems like just changing it (and sboff) back to loff_t would be acceptable.
Yes, that should work.
>
> Do you want a follow-up, or a series respin?
Please send a follow-up patch.
Thanks,
Song
