| Message ID | 20170629182015.3000-1-dsterba@suse.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
Thanks a lot. I'm sorry for that I hadn't noticed those emails during the vacation. On 06/30/2017 02:20 AM, David Sterba wrote: > The XATTR_ITEM is a type of a directory item so we use the common > validator helper. Unlike other dir items, it can have data. The way the > name len validation is currently implemented does not reflect that. We'd > have to adjust by the data_len when comparing the read and item limits. > > However, this will not work for multi-item xattr dir items. > > Example from tree dump of generic/337: > > item 7 key (257 XATTR_ITEM 751495445) itemoff 15667 itemsize 147 > location key (0 UNKNOWN.0 0) type XATTR > transid 8 data_len 3 name_len 11 > name: user.foobar > data 123 > location key (0 UNKNOWN.0 0) type XATTR > transid 8 data_len 6 name_len 13 > name: user.WvG1c1Td > data qwerty > location key (0 UNKNOWN.0 0) type XATTR > transid 8 data_len 5 name_len 19 > name: user.J3__T_Km3dVsW_ > data hello > > At the point of btrfs_is_name_len_valid call we don't have access to the > data_len value of the 2nd and 3rd sub-item. So simple btrfs_dir_data_len(leaf, > di) would always return 3, although we'd need to get 6 and 5 respectively to > get the claculations right. (read_end + name_len + data_len vs item_end) > > We'd have to also pass data_len externally, which is not point of the > name validation. The last check is supposed to test if there's at least > one dir item space after the one we're processing. I don't think this is > particularly useful, validation of the next item would catch that too. > So the check is removed and we don't weaken the validation. Now tests > btrfs/048, btrfs/053, generic/273 and generic/337 pass. > > Signed-off-by: David Sterba <dsterba@suse.com> > --- > > fs/btrfs/dir-item.c | 8 -------- > 1 file changed, 8 deletions(-) > > diff --git a/fs/btrfs/dir-item.c b/fs/btrfs/dir-item.c > index 2b00dd746118..41cb9196eaa8 100644 > --- a/fs/btrfs/dir-item.c > +++ b/fs/btrfs/dir-item.c > @@ -550,14 +550,6 @@ bool btrfs_is_name_len_valid(struct extent_buffer *leaf, int slot, > goto out; > } > > - /* > - * This may be the last item in the slot > - * Or same type item(s) is left between read_end and item_end > - */ > - if (item_end != read_end && item_end - read_end < size) { > - ret = false; > - goto out; > - } > out: > if (!ret) > btrfs_crit(fs_info, "invalid dir item name len: %u", > -- To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/fs/btrfs/dir-item.c b/fs/btrfs/dir-item.c index 2b00dd746118..41cb9196eaa8 100644 --- a/fs/btrfs/dir-item.c +++ b/fs/btrfs/dir-item.c @@ -550,14 +550,6 @@ bool btrfs_is_name_len_valid(struct extent_buffer *leaf, int slot, goto out; } - /* - * This may be the last item in the slot - * Or same type item(s) is left between read_end and item_end - */ - if (item_end != read_end && item_end - read_end < size) { - ret = false; - goto out; - } out: if (!ret) btrfs_crit(fs_info, "invalid dir item name len: %u",
The XATTR_ITEM is a type of a directory item so we use the common validator helper. Unlike other dir items, it can have data. The way the name len validation is currently implemented does not reflect that. We'd have to adjust by the data_len when comparing the read and item limits. However, this will not work for multi-item xattr dir items. Example from tree dump of generic/337: item 7 key (257 XATTR_ITEM 751495445) itemoff 15667 itemsize 147 location key (0 UNKNOWN.0 0) type XATTR transid 8 data_len 3 name_len 11 name: user.foobar data 123 location key (0 UNKNOWN.0 0) type XATTR transid 8 data_len 6 name_len 13 name: user.WvG1c1Td data qwerty location key (0 UNKNOWN.0 0) type XATTR transid 8 data_len 5 name_len 19 name: user.J3__T_Km3dVsW_ data hello At the point of btrfs_is_name_len_valid call we don't have access to the data_len value of the 2nd and 3rd sub-item. So simple btrfs_dir_data_len(leaf, di) would always return 3, although we'd need to get 6 and 5 respectively to get the claculations right. (read_end + name_len + data_len vs item_end) We'd have to also pass data_len externally, which is not point of the name validation. The last check is supposed to test if there's at least one dir item space after the one we're processing. I don't think this is particularly useful, validation of the next item would catch that too. So the check is removed and we don't weaken the validation. Now tests btrfs/048, btrfs/053, generic/273 and generic/337 pass. Signed-off-by: David Sterba <dsterba@suse.com> --- fs/btrfs/dir-item.c | 8 -------- 1 file changed, 8 deletions(-)