(sorry for the delay, various external issues)
I have sent out the new patch set. Thanks for the comments.
more inline.
On 10/30/13 05:33 AM, Zach Brown wrote:
This adds ioctl BTRFS_IOC_GET_FSIDS which reads the fs
info through the btrfs-control
Why not use sysfs?
various sysfs interface for btrfs is still being a RFC
ioctl would much simpler to get the bug fixed.
+ sz_fslist_arg = sizeof(*fslist_arg);
+ fslist_arg = memdup_user(arg, sz_fslist_arg);
Doesn't check allocation failure.
fixed it.
+
+ sz_fslist = sizeof(*fslist) * fslist_arg->count;
+ kfree(fslist_arg);
That allocation and copy and free gets a single u64. Use
copy_from_user() for the u64.
oh yes. thanks.
+ fslist_arg = memdup_user(arg, sz_fslist_arg + sz_fslist);
Allocates an arbitrarily huge size that depends only on user input.
Doesn't check failure again. And I bet you can scribble on kernel
memory if you wrap the size.
fixed it. now it finds the number of fsid and then allocates mem.
+ if (copy_to_user(arg, fslist_arg, sz_fslist_arg + sz_fslist))
+ ret = -EFAULT;
And there's no reason to buffer all this in the kernel to begin with.
Just copy_to_user() as you iterate over each fs_devices.
Ok in the v2 patch I have narrowed the allocation and copy to
just what is present. but I still feel one-shot copy is better.
Now I have also used uuid_mutex its bit less granular for the
purpose here but taking into consideration that thread is from
btrfs-control (and so no root pointer is readily available) for
which it should be fine IMO. any comments. thanks.
+ fslist = (struct btrfs_ioctl_fslist *) fslist +
+ sizeof(*fslist);
AKA fslist++.
fixed it.
- z
Posted V2.
Thanks, Anand
--
To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
