Re: [PATCH] btrfs: fix extent buffer read/write range checks

[Nikolay Borisov <nborisov@suse.com>]

<snip>

>>
>> But then, we can even say "start > eb->len" is valid if len == 0?
> 
> Tried the "start >= eb->len" check in the centralized check_eb_range(),
> and unfortunately it triggers a lot of warnings.
> 
> Some callers in fact pass start == eb->len and len == 0:

Isn't this a noop?

> memmove_extent_buffer() in btrfs_del_items()
> copy_extent_buffer() in __push_leaf_*()
> 
> Since the check of "start > eb->len || len > eb->len || start + len >
> eb->len)" has already ensured we won't access anything beyond the eb
> data, I'd prefer to let the start == eb->len && len == 0 case to pass.

In an ideal world shouldn't callers detect their parameters are going to
be a NOOP and never execute the code in the first place? E.g. is it
posible that the math in btrfs_del_item is broken for some edge
condition hence calling those functions with such parameters?

> 
> Doing the extra len == 0 check in those callers seems a little
> over-reacted IMHO.
> 
> Thanks,
> Qu
>>
>>> Or should we also warn such bad practice?
>>
>> Maybe...
>>
>> Or how about let the callers bailing out by e.g. "if (!len) return 1;"
>> in the check function?
>>
>> Regards,
>> Naohiro
>