On Fri, Mar 18, 2016 at 10:03:42AM -0400, Austin S. Hemmelgarn wrote: > Currently, open_ctree_fs_info will open whatever path you pass it and try > to interpret it as a BTRFS filesystem. While this is not nessecarily > dangerous (except possibly if done on a character device), it does > result in some rather cryptic and non-sensical error messages when > trying to run certain commands in ways they weren't intended to be run. > Add a check using stat(2) to verify that the path we've been passed is > in fact a regular file or a block device, or a symlink pointing to a > regular file or block device. > > This causes the following commands to provide a helpful error message > when run on a FIFO, directory, character device, or socket: > * btrfs check > * btrfs restore > * btrfs-image > * btrfs-find-root > * btrfs inspect-internal dump-tree > > stat(2) is used instead of lstat(2), as stat(2) follows symlinks just > like open(2) does, which means we check the same inode that open(2) > opens, and thus don't need special handling for symlinks. > > Signed-off-by: Austin S. Hemmelgarn <ahferroin7@xxxxxxxxx> Applied, thanks. > This has been both build and runtime tested on an x86-64 system with > glibc. It has been build but not runtime tested with uClibc on x86-64 > and ARMv7. It has not been tested on Android or with musl, although it > should work there also. I would not expect any difference among the other arches and libc, it's using a common interface. > There are other tools that have similarly poor error behavior when > called incorrectly (btrfs rescue immediately comes to mind), but they > don't use open_ctree_fs_info, so this doesn't affect them. I may do > followup patches to fix those too if I have the time. Yeah, it would be good to fix all. > Whitelisting is used instead of blacklisting because I feel it provides > a more concise and more easily readable conditional, and because I > think it's a lot less likely that a new file type will be added that > can contain a filesystem image than it is that one will be added which > can't contain a filesystem image (it's more likely that Solaris doors > get ported to Linux than that we get some new file type). Agreed. -- 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
