On 2021/07/09 0:00, David Sterba wrote:
> On Thu, Jul 08, 2021 at 10:10:55PM +0900, Naohiro Aota wrote:
>> From: Chaitanya Kulkarni <chaitanya.kulkarni@xxxxxxx>
>>
>> The function bio_trim has offset and size arguments that are declared
>> as int.
>>
>> The callers of this function uses sector_t type when passing the offset
>> and size e,g. drivers/md/raid1.c:narrow_write_error() and
>> drivers/md/raid1.c:narrow_write_error().
>>
>> Change offset & size arguments to sector_t type for bio_trim().
>>
>> Tested-by: Naohiro Aota <naohiro.aota@xxxxxxx>
>> Signed-off-by: Chaitanya Kulkarni <chaitanya.kulkarni@xxxxxxx>
>> ---
>> block/bio.c | 2 +-
>> include/linux/bio.h | 2 +-
>> 2 files changed, 2 insertions(+), 2 deletions(-)
>>
>> diff --git a/block/bio.c b/block/bio.c
>> index 44205dfb6b60..d342ce84f6cf 100644
>> --- a/block/bio.c
>> +++ b/block/bio.c
>> @@ -1465,7 +1465,7 @@ EXPORT_SYMBOL(bio_split);
>> * @offset: number of sectors to trim from the front of @bio
>> * @size: size we want to trim @bio to, in sectors
>> */
>> -void bio_trim(struct bio *bio, int offset, int size)
>> +void bio_trim(struct bio *bio, sector_t offset, sector_t size)
>
> sectort_t seems to be the right one, there are << 9 in the function so
> that could lead to some bugs if the offset and size are at the boundary.
Need to add an overflow check:
size <<= 9;
...
bio->bi_iter.bi_size = size;
bi_size is "unsigned int" so if "size << 9" is larger than UINT_MAX, things will
break in ugly ways. And since trim is a hint to the device, in case of overflow,
the BIO size should probably simply be set to 0, with a WARN_ON signaling it.
Note that the potential overflow already exists with the current code as the BIO
size can be less than requested or 0 if size <<9 overflows the int type...
>
>> {
>> /* 'bio' is a cloned bio which we need to trim to match
>> * the given offset and size.
>> diff --git a/include/linux/bio.h b/include/linux/bio.h
>> index a0b4cfdf62a4..fb663152521e 100644
>> --- a/include/linux/bio.h
>> +++ b/include/linux/bio.h
>> @@ -379,7 +379,7 @@ static inline void bip_set_seed(struct bio_integrity_payload *bip,
>>
>> #endif /* CONFIG_BLK_DEV_INTEGRITY */
>>
>> -extern void bio_trim(struct bio *bio, int offset, int size);
>> +void bio_trim(struct bio *bio, sector_t offset, sector_t size);
>
> You may want to keep the extern for consistency in that file, though
> it's not necessary for the prototype.
>
> The patch is simple I can take it through the btrfs tree with the other
> fixes unless there are objections.
>
--
Damien Le Moal
Western Digital Research
