[bug report] bcachefs: Kill struct bucket_mark

[bug report] bcachefs: Kill struct bucket_mark

[Dan Carpenter]

Hello Kent Overstreet,

The patch 43ae95661a99: "bcachefs: Kill struct bucket_mark" from Feb
14, 2022 (linux-next), leads to the following Smatch static checker
warning:

	fs/bcachefs/buckets.c:755 check_bucket_ref()
	warn: impossible condition '((bucket_sectors + sectors) > (~0)) => (0-u32max > u32max)'

fs/bcachefs/buckets.c
    740         if (!data_type_is_empty(bucket_data_type) &&
    741             ptr_data_type &&
    742             bucket_data_type != ptr_data_type) {
    743                 bch2_fsck_err(c, FSCK_CAN_IGNORE|FSCK_NEED_FSCK,
    744                         "bucket %u:%zu gen %u different types of data in same bucket: %s, %s\n"
    745                         "while marking %s",
    746                         ptr->dev, bucket_nr, b_gen,
    747                         bch2_data_types[bucket_data_type],
    748                         bch2_data_types[ptr_data_type],
    749                         (printbuf_reset(&buf),
    750                          bch2_bkey_val_to_text(&buf, c, k), buf.buf));
    751                 ret = -EIO;
    752                 goto err;
    753         }
    754 
--> 755         if ((unsigned) (bucket_sectors + sectors) > U32_MAX) {

Originally this was U16_MAX instead of U32_MAX so that made sense.  Also
it's a style violation to use (unsigned) instead of (unsigned int).

    756                 bch2_fsck_err(c, FSCK_CAN_IGNORE|FSCK_NEED_FSCK,
    757                         "bucket %u:%zu gen %u data type %s sector count overflow: %u + %lli > U16_MAX\n"
    758                         "while marking %s",
    759                         ptr->dev, bucket_nr, b_gen,
    760                         bch2_data_types[bucket_data_type ?: ptr_data_type],
    761                         bucket_sectors, sectors,
    762                         (printbuf_reset(&buf),
    763                          bch2_bkey_val_to_text(&buf, c, k), buf.buf));
    764                 ret = -EIO;
    765                 goto err;
    766         }
    767 out:
    768         printbuf_exit(&buf);
    769         return ret;
    770 err:
    771         bch2_dump_trans_updates(trans);
    772         goto out;
    773 }

regards,
dan carpenter

Re: [bug report] bcachefs: Kill struct bucket_mark

[Kent Overstreet]

On Thu, Sep 14, 2023 at 04:40:03PM +0300, Dan Carpenter wrote:
> Hello Kent Overstreet,
> 
> The patch 43ae95661a99: "bcachefs: Kill struct bucket_mark" from Feb
> 14, 2022 (linux-next), leads to the following Smatch static checker
> warning:
> 
> 	fs/bcachefs/buckets.c:755 check_bucket_ref()
> 	warn: impossible condition '((bucket_sectors + sectors) > (~0)) => (0-u32max > u32max)'
> 
> fs/bcachefs/buckets.c
>     740         if (!data_type_is_empty(bucket_data_type) &&
>     741             ptr_data_type &&
>     742             bucket_data_type != ptr_data_type) {
>     743                 bch2_fsck_err(c, FSCK_CAN_IGNORE|FSCK_NEED_FSCK,
>     744                         "bucket %u:%zu gen %u different types of data in same bucket: %s, %s\n"
>     745                         "while marking %s",
>     746                         ptr->dev, bucket_nr, b_gen,
>     747                         bch2_data_types[bucket_data_type],
>     748                         bch2_data_types[ptr_data_type],
>     749                         (printbuf_reset(&buf),
>     750                          bch2_bkey_val_to_text(&buf, c, k), buf.buf));
>     751                 ret = -EIO;
>     752                 goto err;
>     753         }
>     754 
> --> 755         if ((unsigned) (bucket_sectors + sectors) > U32_MAX) {
> 
> Originally this was U16_MAX instead of U32_MAX so that made sense. 

thanks, fixed - we were also storing bucket_sectors in a u16 instead of
a u32 above.