On 2019/7/10 下午6:42, Nikolay Borisov wrote:
>
>
[...]
>>
>> +/*
>> + * Check if the [start, start + len) range is valid before reading/writing
>> + * the eb.
>> + *
>> + * Caller should not touch the dst/src memory if this function returns
>> error.
>> + */
>> +static int check_eb_range(const struct extent_buffer *eb, unsigned long
>> start,
>> + unsigned long len)
>> +{
>> + unsigned long end;
>> +
>> + /* start, start + len should not go beyond eb->len nor overflow */
>> + if (unlikely(start > eb->len || start + len > eb->len ||
>
> I think your check here is wrong, it should be start + len > start +
> eb->len. start is the logical address hence it can be a lot bigger than
> the size of the eb which is 16k by default.
Definitely NO.
[start, start + len) must be in the range of [0, nodesize).
So think again.
>
>> + check_add_overflow(start, len, &end))) {
>> + WARN(IS_ENABLED(CONFIG_BTRFS_DEBUG), KERN_ERR
>> +"btrfs: bad eb rw request, eb bytenr=%llu len=%lu rw start=%lu len=%lu\n",
>> + eb->start, eb->len, start, len);
>> + btrfs_warn(eb->fs_info,
>> +"btrfs: bad eb rw request, eb bytenr=%llu len=%lu rw start=%lu len=%lu\n",
>> + eb->start, eb->len, start, len);
>
> If CONFIG_BTRFS_DEBUG is enabled then we will print the warning text
> twice. Simply make it:
>
> WARN_ON(IS_ENABLED()) and leave the btrfs_Warn to always print the text.
WARN_ON() doesn't contain any text to indicate the reason of the stack dump.
Thus I still prefer to show exact the reason other than takes developer
several seconds to combine the stack with the following btrfs_warn()
message.
Anyway, it's not something you'll see even in months, thus I don't
really think it would cause any difference.
Thanks,
Qu
