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, Song241 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. Do you want a follow-up, or a series respin?
