> >
> > So, I chose this one. (read /proc/swaps)
>
> Sure, I think your change is good. I just think perhaps mkfs should also try
> to open O_EXCL after all those other tests, as a last safety check.
I think mkfs should first try an O_EXCL open. If that works it doesn't
need to do any of this work to try and predict if the open will fail.
After it fails it can poke around a bit to try and give nice context for
why it failed. But it might not be able to because /proc/swaps is
fundamentally unreliable.
> >>> file = av[optind++];
> >>> + ret = is_swap_device(file);
The input string is a CWD-realtive path. You'd at least want to use
realpath() to get it to a canonical name. So it's not full of junk like
"./" and "../../.." which won't be present in /proc/swaps.
> >>> + char buf[1024];
Use PATH_MAX so it doesn't fail on absurd but allowed file names.
(Where on earth does 1024 come from?)
> >>> + if ((f = fopen("/proc/swaps", "r")) == NULL)
> >>> + return -errno;
As was pointed out, there's no reason this failure should stop mkfs.
/proc/swaps might not be available or allowed and I should still be able
to do an unpriviledged "./mkfs ./myfile; ./btrfs-debug-tree ./myfile".
> >>> + if (strcmp(file, buf) == 0) {
> >>> + ret = 1;
> >>> + break;
> >>> + }
The command line path that lead to the inode might not be the path for
the inode that is in /proc/swaps. Bind mounts, chroot, name spaces,
hard links, etc, all make it possible -- though unlikely -- that mkfs
simply won't be able to tell if the path names are related.
Also, /proc/swaps escapes whitespace in file names. So you could be
comparing "swap file" with "swap\040file".
> >>> + if ((stat(buf, &st_buf) == 0) && S_ISBLK(st_buf.st_mode) &&
> >>> + rdev == st_buf.st_rdev) {
> >>> + ret = 1;
> >>> + break;
> >>> + }
One possible alternative is to then try and open every swap file path to
see if it points to the same inode as the path mkfs was given. But you
might not have access to the paths and we're back to the unpriviledged
failure case.
- z
--
To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
