BTRFS_SEARCH_ARGS_BUFSIZE too small

BTRFS_SEARCH_ARGS_BUFSIZE too small

[Gerhard Heift]

Hello,

I'm playing around with the BTRFS_IOC_SEARCH_TREE to extract the csums
of the physical blocks. During the tests some item_header had len = 0,
which indicates the buffer was to small to hold the item. I added a
printk into the kernel to get the original size of the item and it was
around 6600 bytes. Is there another way to get the item? Otherwise I
would suggest to create an ioctl, which is a little bit more flexible,
something like

struct btrfs_ioctl_search_args2 {
        struct btrfs_ioctl_search_key key;
        __u64 buf_len
        char buf[0];
};

Gerhard

Re: BTRFS_SEARCH_ARGS_BUFSIZE too small

[David Sterba]

On Fri, Jan 10, 2014 at 12:44:17AM +0100, Gerhard Heift wrote:
> I'm playing around with the BTRFS_IOC_SEARCH_TREE to extract the csums
> of the physical blocks. During the tests some item_header had len = 0,
> which indicates the buffer was to small to hold the item. I added a
> printk into the kernel to get the original size of the item and it was
> around 6600 bytes.

This can happen with the metadata blocks bigger than 4k, the search
ioctl does not have buffer large enough as you've found. The EXTENT_CSUM
items seem to cap at 16k, no matter what's the node size (tried with
64k).

> Is there another way to get the item? Otherwise I would suggest to
> create an ioctl, which is a little bit more flexible, something like
> 
> struct btrfs_ioctl_search_args2 {
>         struct btrfs_ioctl_search_key key;
>         __u64 buf_len
>         char buf[0];
> };

Yeah, a V2 ioctl has to be introduced, the structure could look like
this. Kernel would have uo directly copy_to_user to the buffer without
the intermediate memcpy to the in-kernel copy of the ioctl structure.


david

Re: BTRFS_SEARCH_ARGS_BUFSIZE too small

[Gerhard Heift]

I have already patches for such an ioctl. Which form do you prefer?

struct btrfs_ioctl_search_args_inline {
        struct btrfs_ioctl_search_key key;
        __u64 buf_len
        char buf[0];
};

or

struct btrfs_ioctl_search_args_extern {
        struct btrfs_ioctl_search_key key;
        __u64 buf_len
        char __user *buf;
};

And against which commit should I rebase them?

2014/1/15 David Sterba <dsterba@suse.cz>:
> On Fri, Jan 10, 2014 at 12:44:17AM +0100, Gerhard Heift wrote:
>> I'm playing around with the BTRFS_IOC_SEARCH_TREE to extract the csums
>> of the physical blocks. During the tests some item_header had len = 0,
>> which indicates the buffer was to small to hold the item. I added a
>> printk into the kernel to get the original size of the item and it was
>> around 6600 bytes.
>
> This can happen with the metadata blocks bigger than 4k, the search
> ioctl does not have buffer large enough as you've found. The EXTENT_CSUM
> items seem to cap at 16k, no matter what's the node size (tried with
> 64k).
>
>> Is there another way to get the item? Otherwise I would suggest to
>> create an ioctl, which is a little bit more flexible, something like
>>
>> struct btrfs_ioctl_search_args2 {
>>         struct btrfs_ioctl_search_key key;
>>         __u64 buf_len
>>         char buf[0];
>> };
>
> Yeah, a V2 ioctl has to be introduced, the structure could look like
> this. Kernel would have uo directly copy_to_user to the buffer without
> the intermediate memcpy to the in-kernel copy of the ioctl structure.
>
>
> david

Re: BTRFS_SEARCH_ARGS_BUFSIZE too small

[David Sterba]

On Wed, Jan 15, 2014 at 08:29:22PM +0100, Gerhard Heift wrote:
> I have already patches for such an ioctl. Which form do you prefer?

Great!

> struct btrfs_ioctl_search_args_inline {
>         struct btrfs_ioctl_search_key key;
>         __u64 buf_len
>         char buf[0];
> };

This one.

>         char __user *buf;

We want to avoid dereferencing any user pointers in kernel, the whole
structure is one blob.

> And against which commit should I rebase them?

The kernel patches against btrfs-next, progs patches against any recent
integration branch.


david