| Message ID | e5c99e50cd4f95af1cd8c6c7f9e756c7f9256e58.1695089675.git.wqu@suse.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | btrfs: set the destination memory to zero if read_extent_buffer() got invalid ranges | expand |
On Tue, Sep 19, 2023 at 11:44:42AM +0930, Qu Wenruo wrote: > Commit f98b6215d7d1 ("btrfs: extent_io: do extra check for extent buffer > read write functions") changed how we handle invalid extent buffer range > for read_extent_buffer(). > > Previously if the range is invalid we just set the destination to zero, > but after the patch we do nothing but erroring out. > > This can lead to smatch static checker errors like: > > fs/btrfs/print-tree.c:186 print_uuid_item() error: uninitialized symbol 'subvol_id'. > fs/btrfs/tests/extent-io-tests.c:338 check_eb_bitmap() error: uninitialized symbol 'has'. > fs/btrfs/tests/extent-io-tests.c:353 check_eb_bitmap() error: uninitialized symbol 'has'. > fs/btrfs/uuid-tree.c:203 btrfs_uuid_tree_remove() error: uninitialized symbol 'read_subid'. > fs/btrfs/uuid-tree.c:353 btrfs_uuid_tree_iterate() error: uninitialized symbol 'subid_le'. > fs/btrfs/uuid-tree.c:72 btrfs_uuid_tree_lookup() error: uninitialized symbol 'data'. > fs/btrfs/volumes.c:7415 btrfs_dev_stats_value() error: uninitialized symbol 'val'. > > Fix those warnings by reverting back to the old memset() behavior. > By this we keep the static checker happy and would still make a lot of > noise when such invalid ranges are passed in. > > Reported-by: Dan Carpenter <dan.carpenter@linaro.org> > Fixes: f98b6215d7d1 ("btrfs: extent_io: do extra check for extent buffer read write functions") > Signed-off-by: Qu Wenruo <wqu@suse.com> This looks useful to be fixed independ of the contig eb pages patchset so I'll apply it now. The warnings reported by linux-next shoud be fixed by that too. Thanks.
diff --git a/fs/btrfs/extent_io.c b/fs/btrfs/extent_io.c index c333b7e1017f..1abd3b0a4253 100644 --- a/fs/btrfs/extent_io.c +++ b/fs/btrfs/extent_io.c @@ -3994,8 +3994,14 @@ void read_extent_buffer(const struct extent_buffer *eb, void *dstv, char *dst = (char *)dstv; unsigned long i = get_eb_page_index(start); - if (check_eb_range(eb, start, len)) + if (check_eb_range(eb, start, len)) { + /* + * Invalid range hit, reset the memory to 0, so callers won't + * get some random garbage for their uninitialzed memory. + */ + memset(dstv, 0, len); return; + } offset = get_eb_offset_in_page(eb, start);
Commit f98b6215d7d1 ("btrfs: extent_io: do extra check for extent buffer read write functions") changed how we handle invalid extent buffer range for read_extent_buffer(). Previously if the range is invalid we just set the destination to zero, but after the patch we do nothing but erroring out. This can lead to smatch static checker errors like: fs/btrfs/print-tree.c:186 print_uuid_item() error: uninitialized symbol 'subvol_id'. fs/btrfs/tests/extent-io-tests.c:338 check_eb_bitmap() error: uninitialized symbol 'has'. fs/btrfs/tests/extent-io-tests.c:353 check_eb_bitmap() error: uninitialized symbol 'has'. fs/btrfs/uuid-tree.c:203 btrfs_uuid_tree_remove() error: uninitialized symbol 'read_subid'. fs/btrfs/uuid-tree.c:353 btrfs_uuid_tree_iterate() error: uninitialized symbol 'subid_le'. fs/btrfs/uuid-tree.c:72 btrfs_uuid_tree_lookup() error: uninitialized symbol 'data'. fs/btrfs/volumes.c:7415 btrfs_dev_stats_value() error: uninitialized symbol 'val'. Fix those warnings by reverting back to the old memset() behavior. By this we keep the static checker happy and would still make a lot of noise when such invalid ranges are passed in. Reported-by: Dan Carpenter <dan.carpenter@linaro.org> Fixes: f98b6215d7d1 ("btrfs: extent_io: do extra check for extent buffer read write functions") Signed-off-by: Qu Wenruo <wqu@suse.com> --- fs/btrfs/extent_io.c | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-)