| Message ID | 1359113550-23962-1-git-send-email-gene@czarc.net (mailing list archive) |
|---|---|
| State | Under Review, archived |
| Headers | show |
On Fri, 25 Jan 2013 06:32:30 -0500, Gene Czarcinski wrote: > This patch hits a lot of files but adds little code. It > could be considered a bugfix, Currently, when one of the > btrfs user-space programs is executed by a regular user, > the result if oftem a number of strange error messages > which do not indicate the real problem. This patch changes > that situation. > > A test is performed as to whether the program is running > as root. If it is not, issue an error message and exit. > Signed-off-by: Gene Czarcinski <gene@czarc.net> > --- > btrfs-corrupt-block.c | 5 +++++ > btrfs-image.c | 5 +++++ > btrfs-map-logical.c | 5 +++++ > btrfs-select-super.c | 5 +++++ > btrfs-show-super.c | 5 +++++ > btrfs-show.c | 5 +++++ > btrfs-vol.c | 5 +++++ > btrfs-zero-log.c | 5 +++++ > btrfs.c | 6 ++++++ > btrfsck.c | 5 +++++ > btrfsctl.c | 5 +++++ > btrfstune.c | 5 +++++ > calc-size.c | 5 +++++ > convert.c | 6 ++++++ > debug-tree.c | 5 +++++ > dir-test.c | 5 +++++ > find-root.c | 5 +++++ > ioctl-test.c | 6 ++++++ > mkfs.c | 5 +++++ > quick-test.c | 6 ++++++ > restore.c | 5 +++++ > 21 files changed, 109 insertions(+) > > diff --git a/btrfs-corrupt-block.c b/btrfs-corrupt-block.c > index b57e757..083fd50 100644 > --- a/btrfs-corrupt-block.c > +++ b/btrfs-corrupt-block.c > @@ -296,6 +296,11 @@ int main(int ac, char **av) > > srand(128); > > + if (geteuid() != 0) { > + fprintf(stderr,"Error: %s must run as root\n", argv[0]); > + exit(1); > + } > + > while(1) { > int c; > c = getopt_long(ac, av, "l:c:b:eEk", long_options, > diff --git a/btrfs-image.c b/btrfs-image.c > index 7dc131d..fd9b28a 100644 > --- a/btrfs-image.c > +++ b/btrfs-image.c > @@ -831,6 +831,11 @@ int main(int argc, char *argv[]) > int ret; > FILE *out; > > + if (geteuid() != 0) { > + fprintf(stderr,"Error: %s must run as root\n", argv[0]); > + exit(1); > + } > + > while (1) { > int c = getopt(argc, argv, "rc:t:"); > if (c < 0) > diff --git a/btrfs-map-logical.c b/btrfs-map-logical.c > index fa4fb3f..59f2f0e 100644 > --- a/btrfs-map-logical.c > +++ b/btrfs-map-logical.c > @@ -116,6 +116,11 @@ int main(int ac, char **av) > int out_fd = 0; > int err; > > + if (geteuid() != 0) { > + fprintf(stderr,"Error: %s must run as root\n", av[0]); > + exit(1); > + } > + > while(1) { > int c; > c = getopt_long(ac, av, "l:c:o:b:", long_options, > diff --git a/btrfs-select-super.c b/btrfs-select-super.c > index 0c4f5c0..049379d 100644 > --- a/btrfs-select-super.c > +++ b/btrfs-select-super.c > @@ -46,6 +46,11 @@ int main(int ac, char **av) > int num; > u64 bytenr = 0; > > + if (geteuid() != 0) { > + fprintf(stderr,"Error: %s must run as root\n", argv[0]); > + exit(1); > + } > + > while(1) { > int c; > c = getopt(ac, av, "s:"); > diff --git a/btrfs-show-super.c b/btrfs-show-super.c > index a9e2524..2fa4776 100644 > --- a/btrfs-show-super.c > +++ b/btrfs-show-super.c > @@ -63,6 +63,11 @@ int main(int argc, char **argv) > int arg, i; > u64 sb_bytenr = btrfs_sb_offset(0); > > + if (geteuid() != 0) { > + fprintf(stderr,"Error: %s must run as root\n", argv[0]); > + exit(1); > + } > + > while ((opt = getopt(argc, argv, "ai:")) != -1) { > switch (opt) { > case 'i': > diff --git a/btrfs-show.c b/btrfs-show.c > index 8210fd2..6b3b91a 100644 > --- a/btrfs-show.c > +++ b/btrfs-show.c > @@ -122,6 +122,11 @@ int main(int ac, char **av) > "** Please consider to switch to the btrfs utility\n" > "**\n"); > > + if (geteuid() != 0) { > + fprintf(stderr,"Error: %s must run as root\n", av[0]); > + exit(1); > + } > + > while(1) { > int c; > c = getopt_long(ac, av, "", long_options, > diff --git a/btrfs-vol.c b/btrfs-vol.c > index ad824bd..7e02f72 100644 > --- a/btrfs-vol.c > +++ b/btrfs-vol.c > @@ -83,6 +83,11 @@ int main(int ac, char **av) > "** Please consider to switch to the btrfs utility\n" > "**\n"); > > + if (geteuid() != 0) { > + fprintf(stderr,"Error: %s must run as root\n", av[0]); > + exit(1); > + } > + > while(1) { > int c; > c = getopt_long(ac, av, "a:br:", long_options, > diff --git a/btrfs-zero-log.c b/btrfs-zero-log.c > index 1ea867b..80e4e38 100644 > --- a/btrfs-zero-log.c > +++ b/btrfs-zero-log.c > @@ -45,6 +45,11 @@ int main(int ac, char **av) > struct btrfs_trans_handle *trans; > int ret; > > + if (geteuid() != 0) { > + fprintf(stderr,"Error: %s must run as root\n", av[0]); > + exit(1); > + } > + > if (ac != 2) > print_usage(); > > diff --git a/btrfs.c b/btrfs.c > index 687acec..328966b 100644 > --- a/btrfs.c > +++ b/btrfs.c > @@ -18,6 +18,7 @@ > #include <stdio.h> > #include <stdlib.h> > #include <string.h> > +#include <unistd.h> > > #include "crc32c.h" > #include "commands.h" > @@ -261,6 +262,11 @@ int main(int argc, char **argv) > { > const struct cmd_struct *cmd; > > + if (geteuid() != 0) { > + fprintf(stderr,"Error: %s must run as root\n", argv[0]); > + exit(1); > + } > + > crc32c_optimization_init(); > > argc--; > diff --git a/btrfsck.c b/btrfsck.c > index 6274ff7..bdfdfc5 100644 > --- a/btrfsck.c > +++ b/btrfsck.c > @@ -3501,6 +3501,11 @@ int main(int ac, char **av) > int init_csum_tree = 0; > int rw = 0; > > + if (geteuid() != 0) { > + fprintf(stderr,"Error: %s must run as root\n", av[0]); > + exit(1); > + } > + > while(1) { > int c; > c = getopt_long(ac, av, "as:", long_options, > diff --git a/btrfsctl.c b/btrfsctl.c > index 049a5f3..cbe41e7 100644 > --- a/btrfsctl.c > +++ b/btrfsctl.c > @@ -113,6 +113,11 @@ int main(int ac, char **av) > "** Please consider to switch to the btrfs utility\n" > "**\n"); > > + if (geteuid() != 0) { > + fprintf(stderr,"Error: %s must run as root\n", av[0]); > + exit(1); > + } > + > if (ac == 2 && strcmp(av[1], "-a") == 0) { > fprintf(stderr, "Scanning for Btrfs filesystems\n"); > btrfs_scan_one_dir("/dev", 1); > diff --git a/btrfstune.c b/btrfstune.c > index 6950f74..d4017f1 100644 > --- a/btrfstune.c > +++ b/btrfstune.c > @@ -79,6 +79,11 @@ int main(int argc, char *argv[]) > int seeding_value = 0; > int ret; > > + if (geteuid() != 0) { > + fprintf(stderr,"Error: %s must run as root\n", argv[0]); > + exit(1); > + } > + > while(1) { > int c = getopt(argc, argv, "S:"); > if (c < 0) > diff --git a/calc-size.c b/calc-size.c > index c4adfb0..0d3442c 100644 > --- a/calc-size.c > +++ b/calc-size.c > @@ -194,6 +194,11 @@ int main(int argc, char **argv) > int opt; > int ret = 0; > > + if (geteuid() != 0) { > + fprintf(stderr,"Error: %s must run as root\n", argv[0]); > + exit(1); > + } > + > while ((opt = getopt(argc, argv, "vb")) != -1) { > switch (opt) { > case 'v': > diff --git a/convert.c b/convert.c > index 1de2a44..1b0e27c 100644 > --- a/convert.c > +++ b/convert.c > @@ -2770,6 +2770,12 @@ int main(int argc, char *argv[]) > int datacsum = 1; > int rollback = 0; > char *file; > + > + if (geteuid() != 0) { > + fprintf(stderr,"Error: %s must run as root\n", argv[0]); > + exit(1); > + } > + > while(1) { > int c = getopt(argc, argv, "dinr"); > if (c < 0) > diff --git a/debug-tree.c b/debug-tree.c > index f6bd5d8..5b2f531 100644 > --- a/debug-tree.c > +++ b/debug-tree.c > @@ -129,6 +129,11 @@ int main(int ac, char **av) > u64 block_only = 0; > struct btrfs_root *tree_root_scan; > > + if (geteuid() != 0) { > + fprintf(stderr,"Error: %s must run as root\n", av[0]); > + exit(1); > + } > + > radix_tree_init(); > > while(1) { > diff --git a/dir-test.c b/dir-test.c > index c7644d6..9fa5b06 100644 > --- a/dir-test.c > +++ b/dir-test.c > @@ -433,6 +433,11 @@ int main(int ac, char **av) > int err = 0; > int initial_only = 0; > struct btrfs_trans_handle *trans; > + if (geteuid() != 0) { > + fprintf(stderr,"Error: %s must run as root\n", argv[0]); > + exit(1); > + } > + > radix_tree_init(); > > root = open_ctree(av[ac-1], &super, 0); > diff --git a/find-root.c b/find-root.c > index 83f1592..06465eb 100644 > --- a/find-root.c > +++ b/find-root.c > @@ -414,6 +414,11 @@ int main(int argc, char **argv) > int opt; > int ret; > > + if (geteuid() != 0) { > + fprintf(stderr,"Error: %s must run as root\n", argv[0]); > + exit(1); > + } > + > while ((opt = getopt(argc, argv, "vo:")) != -1) { > switch(opt) { > case 'v': > diff --git a/ioctl-test.c b/ioctl-test.c > index 1c27d61..299d2af 100644 > --- a/ioctl-test.c > +++ b/ioctl-test.c > @@ -1,5 +1,6 @@ > #include <stdio.h> > #include <stdlib.h> > +#include <unistd.h> > #include "kerncompat.h" > #include "ioctl.h" > > @@ -28,6 +29,11 @@ unsigned long ioctls[] = { > int main(int ac, char **av) > { > int i = 0; > + if (geteuid() != 0) { > + fprintf(stderr,"Error: %s must run as root\n", argv[0]); > + exit(1); > + } > + > while(ioctls[i]) { > printf("%lu\n" ,ioctls[i]); > i++; > diff --git a/mkfs.c b/mkfs.c > index a129ec4..501e384 100644 > --- a/mkfs.c > +++ b/mkfs.c > @@ -1274,6 +1274,11 @@ int main(int ac, char **av) > u64 source_dir_size = 0; > char *pretty_buf; > > + if (geteuid() != 0) { > + fprintf(stderr,"Error: %s must run as root\n", av[0]); > + exit(1); > + } > + > while(1) { > int c; > c = getopt_long(ac, av, "A:b:l:n:s:m:d:L:r:VMK", long_options, > diff --git a/quick-test.c b/quick-test.c > index 05d73fd..e2d6f78 100644 > --- a/quick-test.c > +++ b/quick-test.c > @@ -19,6 +19,7 @@ > #include <stdio.h> > #include <stdlib.h> > #include <fcntl.h> > +#include <unistd.h> > #include "kerncompat.h" > #include "radix-tree.h" > #include "ctree.h" > @@ -49,6 +50,11 @@ int main(int ac, char **av) { > buf = malloc(512); > memset(buf, 0, 512); > > + if (geteuid() != 0) { > + fprintf(stderr,"Error: %s must run as root\n", argv[0]); > + exit(1); > + } > + > radix_tree_init(); > > root = open_ctree(av[1], BTRFS_SUPER_INFO_OFFSET, O_RDWR); > diff --git a/restore.c b/restore.c > index 80afb84..4efc8b5 100644 > --- a/restore.c > +++ b/restore.c > @@ -771,6 +771,11 @@ int main(int argc, char **argv) > int super_mirror = 0; > int find_dir = 0; > > + if (geteuid() != 0) { > + fprintf(stderr,"Error: %s must run as root\n", argv[0]); > + exit(1); > + } > + > while ((opt = getopt(argc, argv, "sviot:u:df:")) != -1) { > switch (opt) { > case 's': > 21 times copy & paste, you set a new record :) -- 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
On Fri, 25 Jan 2013 06:32:30 -0500 Gene Czarcinski <gene@czarc.net> wrote: > This patch hits a lot of files but adds little code. It > could be considered a bugfix, Currently, when one of the > btrfs user-space programs is executed by a regular user, > the result if oftem a number of strange error messages > which do not indicate the real problem. This patch changes > that situation. > > A test is performed as to whether the program is running > as root. If it is not, issue an error message and exit. > Signed-off-by: Gene Czarcinski <gene@czarc.net> $ ls -la /dev/sda brw-rw---T 1 root disk 8, 0 Jan 15 12:11 /dev/sda The user does not have to be root, they can be a member of the group "disk" to manage this device. Also some or all of the tools accept not just a block device, but also a regular file as their parameter. Wouldn't it be better to check whether or not the running user has *write access* to the device or file to be operated on, before failing? > --- > btrfs-corrupt-block.c | 5 +++++ > btrfs-image.c | 5 +++++ > btrfs-map-logical.c | 5 +++++ > btrfs-select-super.c | 5 +++++ > btrfs-show-super.c | 5 +++++ > btrfs-show.c | 5 +++++ > btrfs-vol.c | 5 +++++ > btrfs-zero-log.c | 5 +++++ > btrfs.c | 6 ++++++ > btrfsck.c | 5 +++++ > btrfsctl.c | 5 +++++ > btrfstune.c | 5 +++++ > calc-size.c | 5 +++++ > convert.c | 6 ++++++ > debug-tree.c | 5 +++++ > dir-test.c | 5 +++++ > find-root.c | 5 +++++ > ioctl-test.c | 6 ++++++ > mkfs.c | 5 +++++ > quick-test.c | 6 ++++++ > restore.c | 5 +++++ > 21 files changed, 109 insertions(+)
On 01/25/2013 06:41 AM, Stefan Behrens wrote: > On Fri, 25 Jan 2013 06:32:30 -0500, Gene Czarcinski wrote: >> This patch hits a lot of files but adds little code. It >> could be considered a bugfix, Currently, when one of the >> btrfs user-space programs is executed by a regular user, >> the result if oftem a number of strange error messages >> which do not indicate the real problem. This patch changes >> that situation. >> >> A test is performed as to whether the program is running >> as root. If it is not, issue an error message and exit. >> Signed-off-by: Gene Czarcinski <gene@czarc.net> >> --- >> btrfs-corrupt-block.c | 5 +++++ >> btrfs-image.c | 5 +++++ >> btrfs-map-logical.c | 5 +++++ >> btrfs-select-super.c | 5 +++++ >> btrfs-show-super.c | 5 +++++ >> btrfs-show.c | 5 +++++ >> btrfs-vol.c | 5 +++++ >> btrfs-zero-log.c | 5 +++++ >> btrfs.c | 6 ++++++ >> btrfsck.c | 5 +++++ >> btrfsctl.c | 5 +++++ >> btrfstune.c | 5 +++++ >> calc-size.c | 5 +++++ >> convert.c | 6 ++++++ >> debug-tree.c | 5 +++++ >> dir-test.c | 5 +++++ >> find-root.c | 5 +++++ >> ioctl-test.c | 6 ++++++ >> mkfs.c | 5 +++++ >> quick-test.c | 6 ++++++ >> restore.c | 5 +++++ >> 21 files changed, 109 insertions(+) >> >> diff --git a/btrfs-corrupt-block.c b/btrfs-corrupt-block.c >> index b57e757..083fd50 100644 >> --- a/btrfs-corrupt-block.c >> +++ b/btrfs-corrupt-block.c >> @@ -296,6 +296,11 @@ int main(int ac, char **av) >> >> srand(128); >> >> + if (geteuid() != 0) { >> + fprintf(stderr,"Error: %s must run as root\n", argv[0]); >> + exit(1); >> + } >> + >> while(1) { >> int c; >> c = getopt_long(ac, av, "l:c:b:eEk", long_options, >> diff --git a/btrfs-image.c b/btrfs-image.c >> index 7dc131d..fd9b28a 100644 >> --- a/btrfs-image.c >> +++ b/btrfs-image.c >> @@ -831,6 +831,11 @@ int main(int argc, char *argv[]) >> int ret; >> FILE *out; >> >> + if (geteuid() != 0) { >> + fprintf(stderr,"Error: %s must run as root\n", argv[0]); >> + exit(1); >> + } >> + >> while (1) { >> int c = getopt(argc, argv, "rc:t:"); >> if (c < 0) >> diff --git a/btrfs-map-logical.c b/btrfs-map-logical.c >> index fa4fb3f..59f2f0e 100644 >> --- a/btrfs-map-logical.c >> +++ b/btrfs-map-logical.c >> @@ -116,6 +116,11 @@ int main(int ac, char **av) >> int out_fd = 0; >> int err; >> >> + if (geteuid() != 0) { >> + fprintf(stderr,"Error: %s must run as root\n", av[0]); >> + exit(1); >> + } >> + >> while(1) { >> int c; >> c = getopt_long(ac, av, "l:c:o:b:", long_options, >> diff --git a/btrfs-select-super.c b/btrfs-select-super.c >> index 0c4f5c0..049379d 100644 >> --- a/btrfs-select-super.c >> +++ b/btrfs-select-super.c >> @@ -46,6 +46,11 @@ int main(int ac, char **av) >> int num; >> u64 bytenr = 0; >> >> + if (geteuid() != 0) { >> + fprintf(stderr,"Error: %s must run as root\n", argv[0]); >> + exit(1); >> + } >> + >> while(1) { >> int c; >> c = getopt(ac, av, "s:"); >> diff --git a/btrfs-show-super.c b/btrfs-show-super.c >> index a9e2524..2fa4776 100644 >> --- a/btrfs-show-super.c >> +++ b/btrfs-show-super.c >> @@ -63,6 +63,11 @@ int main(int argc, char **argv) >> int arg, i; >> u64 sb_bytenr = btrfs_sb_offset(0); >> >> + if (geteuid() != 0) { >> + fprintf(stderr,"Error: %s must run as root\n", argv[0]); >> + exit(1); >> + } >> + >> while ((opt = getopt(argc, argv, "ai:")) != -1) { >> switch (opt) { >> case 'i': >> diff --git a/btrfs-show.c b/btrfs-show.c >> index 8210fd2..6b3b91a 100644 >> --- a/btrfs-show.c >> +++ b/btrfs-show.c >> @@ -122,6 +122,11 @@ int main(int ac, char **av) >> "** Please consider to switch to the btrfs utility\n" >> "**\n"); >> >> + if (geteuid() != 0) { >> + fprintf(stderr,"Error: %s must run as root\n", av[0]); >> + exit(1); >> + } >> + >> while(1) { >> int c; >> c = getopt_long(ac, av, "", long_options, >> diff --git a/btrfs-vol.c b/btrfs-vol.c >> index ad824bd..7e02f72 100644 >> --- a/btrfs-vol.c >> +++ b/btrfs-vol.c >> @@ -83,6 +83,11 @@ int main(int ac, char **av) >> "** Please consider to switch to the btrfs utility\n" >> "**\n"); >> >> + if (geteuid() != 0) { >> + fprintf(stderr,"Error: %s must run as root\n", av[0]); >> + exit(1); >> + } >> + >> while(1) { >> int c; >> c = getopt_long(ac, av, "a:br:", long_options, >> diff --git a/btrfs-zero-log.c b/btrfs-zero-log.c >> index 1ea867b..80e4e38 100644 >> --- a/btrfs-zero-log.c >> +++ b/btrfs-zero-log.c >> @@ -45,6 +45,11 @@ int main(int ac, char **av) >> struct btrfs_trans_handle *trans; >> int ret; >> >> + if (geteuid() != 0) { >> + fprintf(stderr,"Error: %s must run as root\n", av[0]); >> + exit(1); >> + } >> + >> if (ac != 2) >> print_usage(); >> >> diff --git a/btrfs.c b/btrfs.c >> index 687acec..328966b 100644 >> --- a/btrfs.c >> +++ b/btrfs.c >> @@ -18,6 +18,7 @@ >> #include <stdio.h> >> #include <stdlib.h> >> #include <string.h> >> +#include <unistd.h> >> >> #include "crc32c.h" >> #include "commands.h" >> @@ -261,6 +262,11 @@ int main(int argc, char **argv) >> { >> const struct cmd_struct *cmd; >> >> + if (geteuid() != 0) { >> + fprintf(stderr,"Error: %s must run as root\n", argv[0]); >> + exit(1); >> + } >> + >> crc32c_optimization_init(); >> >> argc--; >> diff --git a/btrfsck.c b/btrfsck.c >> index 6274ff7..bdfdfc5 100644 >> --- a/btrfsck.c >> +++ b/btrfsck.c >> @@ -3501,6 +3501,11 @@ int main(int ac, char **av) >> int init_csum_tree = 0; >> int rw = 0; >> >> + if (geteuid() != 0) { >> + fprintf(stderr,"Error: %s must run as root\n", av[0]); >> + exit(1); >> + } >> + >> while(1) { >> int c; >> c = getopt_long(ac, av, "as:", long_options, >> diff --git a/btrfsctl.c b/btrfsctl.c >> index 049a5f3..cbe41e7 100644 >> --- a/btrfsctl.c >> +++ b/btrfsctl.c >> @@ -113,6 +113,11 @@ int main(int ac, char **av) >> "** Please consider to switch to the btrfs utility\n" >> "**\n"); >> >> + if (geteuid() != 0) { >> + fprintf(stderr,"Error: %s must run as root\n", av[0]); >> + exit(1); >> + } >> + >> if (ac == 2 && strcmp(av[1], "-a") == 0) { >> fprintf(stderr, "Scanning for Btrfs filesystems\n"); >> btrfs_scan_one_dir("/dev", 1); >> diff --git a/btrfstune.c b/btrfstune.c >> index 6950f74..d4017f1 100644 >> --- a/btrfstune.c >> +++ b/btrfstune.c >> @@ -79,6 +79,11 @@ int main(int argc, char *argv[]) >> int seeding_value = 0; >> int ret; >> >> + if (geteuid() != 0) { >> + fprintf(stderr,"Error: %s must run as root\n", argv[0]); >> + exit(1); >> + } >> + >> while(1) { >> int c = getopt(argc, argv, "S:"); >> if (c < 0) >> diff --git a/calc-size.c b/calc-size.c >> index c4adfb0..0d3442c 100644 >> --- a/calc-size.c >> +++ b/calc-size.c >> @@ -194,6 +194,11 @@ int main(int argc, char **argv) >> int opt; >> int ret = 0; >> >> + if (geteuid() != 0) { >> + fprintf(stderr,"Error: %s must run as root\n", argv[0]); >> + exit(1); >> + } >> + >> while ((opt = getopt(argc, argv, "vb")) != -1) { >> switch (opt) { >> case 'v': >> diff --git a/convert.c b/convert.c >> index 1de2a44..1b0e27c 100644 >> --- a/convert.c >> +++ b/convert.c >> @@ -2770,6 +2770,12 @@ int main(int argc, char *argv[]) >> int datacsum = 1; >> int rollback = 0; >> char *file; >> + >> + if (geteuid() != 0) { >> + fprintf(stderr,"Error: %s must run as root\n", argv[0]); >> + exit(1); >> + } >> + >> while(1) { >> int c = getopt(argc, argv, "dinr"); >> if (c < 0) >> diff --git a/debug-tree.c b/debug-tree.c >> index f6bd5d8..5b2f531 100644 >> --- a/debug-tree.c >> +++ b/debug-tree.c >> @@ -129,6 +129,11 @@ int main(int ac, char **av) >> u64 block_only = 0; >> struct btrfs_root *tree_root_scan; >> >> + if (geteuid() != 0) { >> + fprintf(stderr,"Error: %s must run as root\n", av[0]); >> + exit(1); >> + } >> + >> radix_tree_init(); >> >> while(1) { >> diff --git a/dir-test.c b/dir-test.c >> index c7644d6..9fa5b06 100644 >> --- a/dir-test.c >> +++ b/dir-test.c >> @@ -433,6 +433,11 @@ int main(int ac, char **av) >> int err = 0; >> int initial_only = 0; >> struct btrfs_trans_handle *trans; >> + if (geteuid() != 0) { >> + fprintf(stderr,"Error: %s must run as root\n", argv[0]); >> + exit(1); >> + } >> + >> radix_tree_init(); >> >> root = open_ctree(av[ac-1], &super, 0); >> diff --git a/find-root.c b/find-root.c >> index 83f1592..06465eb 100644 >> --- a/find-root.c >> +++ b/find-root.c >> @@ -414,6 +414,11 @@ int main(int argc, char **argv) >> int opt; >> int ret; >> >> + if (geteuid() != 0) { >> + fprintf(stderr,"Error: %s must run as root\n", argv[0]); >> + exit(1); >> + } >> + >> while ((opt = getopt(argc, argv, "vo:")) != -1) { >> switch(opt) { >> case 'v': >> diff --git a/ioctl-test.c b/ioctl-test.c >> index 1c27d61..299d2af 100644 >> --- a/ioctl-test.c >> +++ b/ioctl-test.c >> @@ -1,5 +1,6 @@ >> #include <stdio.h> >> #include <stdlib.h> >> +#include <unistd.h> >> #include "kerncompat.h" >> #include "ioctl.h" >> >> @@ -28,6 +29,11 @@ unsigned long ioctls[] = { >> int main(int ac, char **av) >> { >> int i = 0; >> + if (geteuid() != 0) { >> + fprintf(stderr,"Error: %s must run as root\n", argv[0]); >> + exit(1); >> + } >> + >> while(ioctls[i]) { >> printf("%lu\n" ,ioctls[i]); >> i++; >> diff --git a/mkfs.c b/mkfs.c >> index a129ec4..501e384 100644 >> --- a/mkfs.c >> +++ b/mkfs.c >> @@ -1274,6 +1274,11 @@ int main(int ac, char **av) >> u64 source_dir_size = 0; >> char *pretty_buf; >> >> + if (geteuid() != 0) { >> + fprintf(stderr,"Error: %s must run as root\n", av[0]); >> + exit(1); >> + } >> + >> while(1) { >> int c; >> c = getopt_long(ac, av, "A:b:l:n:s:m:d:L:r:VMK", long_options, >> diff --git a/quick-test.c b/quick-test.c >> index 05d73fd..e2d6f78 100644 >> --- a/quick-test.c >> +++ b/quick-test.c >> @@ -19,6 +19,7 @@ >> #include <stdio.h> >> #include <stdlib.h> >> #include <fcntl.h> >> +#include <unistd.h> >> #include "kerncompat.h" >> #include "radix-tree.h" >> #include "ctree.h" >> @@ -49,6 +50,11 @@ int main(int ac, char **av) { >> buf = malloc(512); >> memset(buf, 0, 512); >> >> + if (geteuid() != 0) { >> + fprintf(stderr,"Error: %s must run as root\n", argv[0]); >> + exit(1); >> + } >> + >> radix_tree_init(); >> >> root = open_ctree(av[1], BTRFS_SUPER_INFO_OFFSET, O_RDWR); >> diff --git a/restore.c b/restore.c >> index 80afb84..4efc8b5 100644 >> --- a/restore.c >> +++ b/restore.c >> @@ -771,6 +771,11 @@ int main(int argc, char **argv) >> int super_mirror = 0; >> int find_dir = 0; >> >> + if (geteuid() != 0) { >> + fprintf(stderr,"Error: %s must run as root\n", argv[0]); >> + exit(1); >> + } >> + >> while ((opt = getopt(argc, argv, "sviot:u:df:")) != -1) { >> switch (opt) { >> case 's': >> > 21 times copy & paste, you set a new record :) > I was very tempted to do a little more. I know that there is no standard that says the two parameters of main() are named argc and argv but it is traditional. I could not believe I got errors because it was named av instead of argv. But, patches like this should stay on topic. Gene -- 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
On Fri, 25 Jan 2013 07:03:19 -0500, Gene Czarcinski wrote: > On 01/25/2013 06:41 AM, Stefan Behrens wrote: >> On Fri, 25 Jan 2013 06:32:30 -0500, Gene Czarcinski wrote: >>> This patch hits a lot of files but adds little code. It >>> could be considered a bugfix, Currently, when one of the >>> btrfs user-space programs is executed by a regular user, >>> the result if oftem a number of strange error messages >>> which do not indicate the real problem. This patch changes >>> that situation. >>> >>> A test is performed as to whether the program is running >>> as root. If it is not, issue an error message and exit. >>> Signed-off-by: Gene Czarcinski <gene@czarc.net> >>> --- >>> btrfs-corrupt-block.c | 5 +++++ >>> btrfs-image.c | 5 +++++ >>> btrfs-map-logical.c | 5 +++++ >>> btrfs-select-super.c | 5 +++++ >>> btrfs-show-super.c | 5 +++++ >>> btrfs-show.c | 5 +++++ >>> btrfs-vol.c | 5 +++++ >>> btrfs-zero-log.c | 5 +++++ >>> btrfs.c | 6 ++++++ >>> btrfsck.c | 5 +++++ >>> btrfsctl.c | 5 +++++ >>> btrfstune.c | 5 +++++ >>> calc-size.c | 5 +++++ >>> convert.c | 6 ++++++ >>> debug-tree.c | 5 +++++ >>> dir-test.c | 5 +++++ >>> find-root.c | 5 +++++ >>> ioctl-test.c | 6 ++++++ >>> mkfs.c | 5 +++++ >>> quick-test.c | 6 ++++++ >>> restore.c | 5 +++++ >>> 21 files changed, 109 insertions(+) >>> >>> diff --git a/btrfs-corrupt-block.c b/btrfs-corrupt-block.c >>> index b57e757..083fd50 100644 >>> --- a/btrfs-corrupt-block.c >>> +++ b/btrfs-corrupt-block.c >>> @@ -296,6 +296,11 @@ int main(int ac, char **av) >>> srand(128); >>> + if (geteuid() != 0) { >>> + fprintf(stderr,"Error: %s must run as root\n", argv[0]); >>> + exit(1); >>> + } >>> + >>> while(1) { >>> int c; >>> c = getopt_long(ac, av, "l:c:b:eEk", long_options, >>> diff --git a/btrfs-image.c b/btrfs-image.c >>> index 7dc131d..fd9b28a 100644 >>> --- a/btrfs-image.c >>> +++ b/btrfs-image.c >>> @@ -831,6 +831,11 @@ int main(int argc, char *argv[]) >>> int ret; >>> FILE *out; >>> + if (geteuid() != 0) { >>> + fprintf(stderr,"Error: %s must run as root\n", argv[0]); >>> + exit(1); >>> + } >>> + >>> while (1) { >>> int c = getopt(argc, argv, "rc:t:"); >>> if (c < 0) >>> diff --git a/btrfs-map-logical.c b/btrfs-map-logical.c >>> index fa4fb3f..59f2f0e 100644 >>> --- a/btrfs-map-logical.c >>> +++ b/btrfs-map-logical.c >>> @@ -116,6 +116,11 @@ int main(int ac, char **av) >>> int out_fd = 0; >>> int err; >>> + if (geteuid() != 0) { >>> + fprintf(stderr,"Error: %s must run as root\n", av[0]); >>> + exit(1); >>> + } >>> + >>> while(1) { >>> int c; >>> c = getopt_long(ac, av, "l:c:o:b:", long_options, >>> diff --git a/btrfs-select-super.c b/btrfs-select-super.c >>> index 0c4f5c0..049379d 100644 >>> --- a/btrfs-select-super.c >>> +++ b/btrfs-select-super.c >>> @@ -46,6 +46,11 @@ int main(int ac, char **av) >>> int num; >>> u64 bytenr = 0; >>> + if (geteuid() != 0) { >>> + fprintf(stderr,"Error: %s must run as root\n", argv[0]); >>> + exit(1); >>> + } >>> + >>> while(1) { >>> int c; >>> c = getopt(ac, av, "s:"); >>> diff --git a/btrfs-show-super.c b/btrfs-show-super.c >>> index a9e2524..2fa4776 100644 >>> --- a/btrfs-show-super.c >>> +++ b/btrfs-show-super.c >>> @@ -63,6 +63,11 @@ int main(int argc, char **argv) >>> int arg, i; >>> u64 sb_bytenr = btrfs_sb_offset(0); >>> + if (geteuid() != 0) { >>> + fprintf(stderr,"Error: %s must run as root\n", argv[0]); >>> + exit(1); >>> + } >>> + >>> while ((opt = getopt(argc, argv, "ai:")) != -1) { >>> switch (opt) { >>> case 'i': >>> diff --git a/btrfs-show.c b/btrfs-show.c >>> index 8210fd2..6b3b91a 100644 >>> --- a/btrfs-show.c >>> +++ b/btrfs-show.c >>> @@ -122,6 +122,11 @@ int main(int ac, char **av) >>> "** Please consider to switch to the btrfs utility\n" >>> "**\n"); >>> + if (geteuid() != 0) { >>> + fprintf(stderr,"Error: %s must run as root\n", av[0]); >>> + exit(1); >>> + } >>> + >>> while(1) { >>> int c; >>> c = getopt_long(ac, av, "", long_options, >>> diff --git a/btrfs-vol.c b/btrfs-vol.c >>> index ad824bd..7e02f72 100644 >>> --- a/btrfs-vol.c >>> +++ b/btrfs-vol.c >>> @@ -83,6 +83,11 @@ int main(int ac, char **av) >>> "** Please consider to switch to the btrfs utility\n" >>> "**\n"); >>> + if (geteuid() != 0) { >>> + fprintf(stderr,"Error: %s must run as root\n", av[0]); >>> + exit(1); >>> + } >>> + >>> while(1) { >>> int c; >>> c = getopt_long(ac, av, "a:br:", long_options, >>> diff --git a/btrfs-zero-log.c b/btrfs-zero-log.c >>> index 1ea867b..80e4e38 100644 >>> --- a/btrfs-zero-log.c >>> +++ b/btrfs-zero-log.c >>> @@ -45,6 +45,11 @@ int main(int ac, char **av) >>> struct btrfs_trans_handle *trans; >>> int ret; >>> + if (geteuid() != 0) { >>> + fprintf(stderr,"Error: %s must run as root\n", av[0]); >>> + exit(1); >>> + } >>> + >>> if (ac != 2) >>> print_usage(); >>> diff --git a/btrfs.c b/btrfs.c >>> index 687acec..328966b 100644 >>> --- a/btrfs.c >>> +++ b/btrfs.c >>> @@ -18,6 +18,7 @@ >>> #include <stdio.h> >>> #include <stdlib.h> >>> #include <string.h> >>> +#include <unistd.h> >>> #include "crc32c.h" >>> #include "commands.h" >>> @@ -261,6 +262,11 @@ int main(int argc, char **argv) >>> { >>> const struct cmd_struct *cmd; >>> + if (geteuid() != 0) { >>> + fprintf(stderr,"Error: %s must run as root\n", argv[0]); >>> + exit(1); >>> + } >>> + >>> crc32c_optimization_init(); >>> argc--; >>> diff --git a/btrfsck.c b/btrfsck.c >>> index 6274ff7..bdfdfc5 100644 >>> --- a/btrfsck.c >>> +++ b/btrfsck.c >>> @@ -3501,6 +3501,11 @@ int main(int ac, char **av) >>> int init_csum_tree = 0; >>> int rw = 0; >>> + if (geteuid() != 0) { >>> + fprintf(stderr,"Error: %s must run as root\n", av[0]); >>> + exit(1); >>> + } >>> + >>> while(1) { >>> int c; >>> c = getopt_long(ac, av, "as:", long_options, >>> diff --git a/btrfsctl.c b/btrfsctl.c >>> index 049a5f3..cbe41e7 100644 >>> --- a/btrfsctl.c >>> +++ b/btrfsctl.c >>> @@ -113,6 +113,11 @@ int main(int ac, char **av) >>> "** Please consider to switch to the btrfs utility\n" >>> "**\n"); >>> >>> + if (geteuid() != 0) { >>> + fprintf(stderr,"Error: %s must run as root\n", av[0]); >>> + exit(1); >>> + } >>> + >>> if (ac == 2 && strcmp(av[1], "-a") == 0) { >>> fprintf(stderr, "Scanning for Btrfs filesystems\n"); >>> btrfs_scan_one_dir("/dev", 1); >>> diff --git a/btrfstune.c b/btrfstune.c >>> index 6950f74..d4017f1 100644 >>> --- a/btrfstune.c >>> +++ b/btrfstune.c >>> @@ -79,6 +79,11 @@ int main(int argc, char *argv[]) >>> int seeding_value = 0; >>> int ret; >>> + if (geteuid() != 0) { >>> + fprintf(stderr,"Error: %s must run as root\n", argv[0]); >>> + exit(1); >>> + } >>> + >>> while(1) { >>> int c = getopt(argc, argv, "S:"); >>> if (c < 0) >>> diff --git a/calc-size.c b/calc-size.c >>> index c4adfb0..0d3442c 100644 >>> --- a/calc-size.c >>> +++ b/calc-size.c >>> @@ -194,6 +194,11 @@ int main(int argc, char **argv) >>> int opt; >>> int ret = 0; >>> + if (geteuid() != 0) { >>> + fprintf(stderr,"Error: %s must run as root\n", argv[0]); >>> + exit(1); >>> + } >>> + >>> while ((opt = getopt(argc, argv, "vb")) != -1) { >>> switch (opt) { >>> case 'v': >>> diff --git a/convert.c b/convert.c >>> index 1de2a44..1b0e27c 100644 >>> --- a/convert.c >>> +++ b/convert.c >>> @@ -2770,6 +2770,12 @@ int main(int argc, char *argv[]) >>> int datacsum = 1; >>> int rollback = 0; >>> char *file; >>> + >>> + if (geteuid() != 0) { >>> + fprintf(stderr,"Error: %s must run as root\n", argv[0]); >>> + exit(1); >>> + } >>> + >>> while(1) { >>> int c = getopt(argc, argv, "dinr"); >>> if (c < 0) >>> diff --git a/debug-tree.c b/debug-tree.c >>> index f6bd5d8..5b2f531 100644 >>> --- a/debug-tree.c >>> +++ b/debug-tree.c >>> @@ -129,6 +129,11 @@ int main(int ac, char **av) >>> u64 block_only = 0; >>> struct btrfs_root *tree_root_scan; >>> + if (geteuid() != 0) { >>> + fprintf(stderr,"Error: %s must run as root\n", av[0]); >>> + exit(1); >>> + } >>> + >>> radix_tree_init(); >>> while(1) { >>> diff --git a/dir-test.c b/dir-test.c >>> index c7644d6..9fa5b06 100644 >>> --- a/dir-test.c >>> +++ b/dir-test.c >>> @@ -433,6 +433,11 @@ int main(int ac, char **av) >>> int err = 0; >>> int initial_only = 0; >>> struct btrfs_trans_handle *trans; >>> + if (geteuid() != 0) { >>> + fprintf(stderr,"Error: %s must run as root\n", argv[0]); >>> + exit(1); >>> + } >>> + >>> radix_tree_init(); >>> root = open_ctree(av[ac-1], &super, 0); >>> diff --git a/find-root.c b/find-root.c >>> index 83f1592..06465eb 100644 >>> --- a/find-root.c >>> +++ b/find-root.c >>> @@ -414,6 +414,11 @@ int main(int argc, char **argv) >>> int opt; >>> int ret; >>> + if (geteuid() != 0) { >>> + fprintf(stderr,"Error: %s must run as root\n", argv[0]); >>> + exit(1); >>> + } >>> + >>> while ((opt = getopt(argc, argv, "vo:")) != -1) { >>> switch(opt) { >>> case 'v': >>> diff --git a/ioctl-test.c b/ioctl-test.c >>> index 1c27d61..299d2af 100644 >>> --- a/ioctl-test.c >>> +++ b/ioctl-test.c >>> @@ -1,5 +1,6 @@ >>> #include <stdio.h> >>> #include <stdlib.h> >>> +#include <unistd.h> >>> #include "kerncompat.h" >>> #include "ioctl.h" >>> @@ -28,6 +29,11 @@ unsigned long ioctls[] = { >>> int main(int ac, char **av) >>> { >>> int i = 0; >>> + if (geteuid() != 0) { >>> + fprintf(stderr,"Error: %s must run as root\n", argv[0]); >>> + exit(1); >>> + } >>> + >>> while(ioctls[i]) { >>> printf("%lu\n" ,ioctls[i]); >>> i++; >>> diff --git a/mkfs.c b/mkfs.c >>> index a129ec4..501e384 100644 >>> --- a/mkfs.c >>> +++ b/mkfs.c >>> @@ -1274,6 +1274,11 @@ int main(int ac, char **av) >>> u64 source_dir_size = 0; >>> char *pretty_buf; >>> + if (geteuid() != 0) { >>> + fprintf(stderr,"Error: %s must run as root\n", av[0]); >>> + exit(1); >>> + } >>> + >>> while(1) { >>> int c; >>> c = getopt_long(ac, av, "A:b:l:n:s:m:d:L:r:VMK", long_options, >>> diff --git a/quick-test.c b/quick-test.c >>> index 05d73fd..e2d6f78 100644 >>> --- a/quick-test.c >>> +++ b/quick-test.c >>> @@ -19,6 +19,7 @@ >>> #include <stdio.h> >>> #include <stdlib.h> >>> #include <fcntl.h> >>> +#include <unistd.h> >>> #include "kerncompat.h" >>> #include "radix-tree.h" >>> #include "ctree.h" >>> @@ -49,6 +50,11 @@ int main(int ac, char **av) { >>> buf = malloc(512); >>> memset(buf, 0, 512); >>> + if (geteuid() != 0) { >>> + fprintf(stderr,"Error: %s must run as root\n", argv[0]); >>> + exit(1); >>> + } >>> + >>> radix_tree_init(); >>> root = open_ctree(av[1], BTRFS_SUPER_INFO_OFFSET, O_RDWR); >>> diff --git a/restore.c b/restore.c >>> index 80afb84..4efc8b5 100644 >>> --- a/restore.c >>> +++ b/restore.c >>> @@ -771,6 +771,11 @@ int main(int argc, char **argv) >>> int super_mirror = 0; >>> int find_dir = 0; >>> + if (geteuid() != 0) { >>> + fprintf(stderr,"Error: %s must run as root\n", argv[0]); >>> + exit(1); >>> + } >>> + >>> while ((opt = getopt(argc, argv, "sviot:u:df:")) != -1) { >>> switch (opt) { >>> case 's': >>> >> 21 times copy & paste, you set a new record :) >> > I was very tempted to do a little more. I know that there is no > standard that says the two parameters of main() are named argc and argv > but it is traditional. I could not believe I got errors because it was > named av instead of argv. But, patches like this should stay on topic. What I wanted to say was, put this duplicated code in a subfunction :) You'll still have to copy & paste 21 times, but just a one-liner. util.c: void exit_if_not_superuser(const char *progname) ... -- 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
On 01/25/2013 06:55 AM, Roman Mamedov wrote: > On Fri, 25 Jan 2013 06:32:30 -0500 > Gene Czarcinski <gene@czarc.net> wrote: > >> This patch hits a lot of files but adds little code. It >> could be considered a bugfix, Currently, when one of the >> btrfs user-space programs is executed by a regular user, >> the result if oftem a number of strange error messages >> which do not indicate the real problem. This patch changes >> that situation. >> >> A test is performed as to whether the program is running >> as root. If it is not, issue an error message and exit. >> Signed-off-by: Gene Czarcinski <gene@czarc.net> > $ ls -la /dev/sda > brw-rw---T 1 root disk 8, 0 Jan 15 12:11 /dev/sda > > The user does not have to be root, they can be a member of the group "disk" to > manage this device. > > Also some or all of the tools accept not just a block device, but also a > regular file as their parameter. > > Wouldn't it be better to check whether or not the running user has > *write access* to the device or file to be operated on, before failing? I knew there would be corner cases where root was not required for execution. After all, I do not need to be root to execute "btrfs --version". Now, is it worth the effort to determine the corner cases and do you have a proposed solution as to determining what privileges are needed when? I can understand when it could be a regular file but is it all that common for users to be part of group disk? If there is a case where it is difficult to figure out if root is needed, then my solution would be to turn it into a warning message and remove the exit for that specific program. However, I believe the real answer is to use sudo. Gene > >> --- >> btrfs-corrupt-block.c | 5 +++++ >> btrfs-image.c | 5 +++++ >> btrfs-map-logical.c | 5 +++++ >> btrfs-select-super.c | 5 +++++ >> btrfs-show-super.c | 5 +++++ >> btrfs-show.c | 5 +++++ >> btrfs-vol.c | 5 +++++ >> btrfs-zero-log.c | 5 +++++ >> btrfs.c | 6 ++++++ >> btrfsck.c | 5 +++++ >> btrfsctl.c | 5 +++++ >> btrfstune.c | 5 +++++ >> calc-size.c | 5 +++++ >> convert.c | 6 ++++++ >> debug-tree.c | 5 +++++ >> dir-test.c | 5 +++++ >> find-root.c | 5 +++++ >> ioctl-test.c | 6 ++++++ >> mkfs.c | 5 +++++ >> quick-test.c | 6 ++++++ >> restore.c | 5 +++++ >> 21 files changed, 109 insertions(+) -- 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
On Fri, Jan 25, 2013 at 07:29:44AM -0500, Gene Czarcinski wrote: > On 01/25/2013 06:55 AM, Roman Mamedov wrote: > >On Fri, 25 Jan 2013 06:32:30 -0500 > >Gene Czarcinski <gene@czarc.net> wrote: > > > >>This patch hits a lot of files but adds little code. It > >>could be considered a bugfix, Currently, when one of the > >>btrfs user-space programs is executed by a regular user, > >>the result if oftem a number of strange error messages > >>which do not indicate the real problem. This patch changes > >>that situation. > >> > >>A test is performed as to whether the program is running > >>as root. If it is not, issue an error message and exit. > >>Signed-off-by: Gene Czarcinski <gene@czarc.net> > >$ ls -la /dev/sda > >brw-rw---T 1 root disk 8, 0 Jan 15 12:11 /dev/sda > > > >The user does not have to be root, they can be a member of the group "disk" to > >manage this device. > > > >Also some or all of the tools accept not just a block device, but also a > >regular file as their parameter. > > > >Wouldn't it be better to check whether or not the running user has > >*write access* to the device or file to be operated on, before failing? > I knew there would be corner cases where root was not required for > execution. After all, I do not need to be root to execute "btrfs > --version". Now, is it worth the effort to determine the corner > cases and do you have a proposed solution as to determining what > privileges are needed when? I can understand when it could be a > regular file but is it all that common for users to be part of group > disk? Don't try to check all the possible success conditions beforehand -- that's what leads to websites that fail to work because your browser is not IE, but work perfectly when you change your user-agent string to "MSIE". This is highly frustrating for users. Instead, try whatever it is you were trying to do (open a file, send an ioctl), and determine, as well as you can, why it failed by looking at the error codes that you get back, and report that. "Permission denied" -> means you don't have permissions -> you need to be root, or have yourself put in the disk group, or get the disk-management-capability. Let the user work out which of those solutions they need, rather than forcing them to use the one you thought of. Hugo. > If there is a case where it is difficult to figure out if root is > needed, then my solution would be to turn it into a warning message > and remove the exit for that specific program. > > However, I believe the real answer is to use sudo. > > Gene > > > >>--- > >> btrfs-corrupt-block.c | 5 +++++ > >> btrfs-image.c | 5 +++++ > >> btrfs-map-logical.c | 5 +++++ > >> btrfs-select-super.c | 5 +++++ > >> btrfs-show-super.c | 5 +++++ > >> btrfs-show.c | 5 +++++ > >> btrfs-vol.c | 5 +++++ > >> btrfs-zero-log.c | 5 +++++ > >> btrfs.c | 6 ++++++ > >> btrfsck.c | 5 +++++ > >> btrfsctl.c | 5 +++++ > >> btrfstune.c | 5 +++++ > >> calc-size.c | 5 +++++ > >> convert.c | 6 ++++++ > >> debug-tree.c | 5 +++++ > >> dir-test.c | 5 +++++ > >> find-root.c | 5 +++++ > >> ioctl-test.c | 6 ++++++ > >> mkfs.c | 5 +++++ > >> quick-test.c | 6 ++++++ > >> restore.c | 5 +++++ > >> 21 files changed, 109 insertions(+) >
On Fri, 25 Jan 2013 07:29:44 -0500
Gene Czarcinski <gene@czarc.net> wrote:
> After all, I do not need to be root to execute "btrfs --version".
Is that all that comes to mind? I just did
$ dd if=/dev/zero of=fs.img bs=1M count=2048
2048+0 records in
2048+0 records out
2147483648 bytes (2.1 GB) copied, 3.76772 s, 570 MB/s
$ /sbin/mkfs.btrfs fs.img
WARNING! - Btrfs Btrfs v0.19 IS EXPERIMENTAL
WARNING! - see http://btrfs.wiki.kernel.org before using
fs created label (null) on fs.img
nodesize 4096 leafsize 4096 sectorsize 4096 size 2.00GB
Btrfs Btrfs v0.19
$ /sbin/btrfsck fs.img
checking extents
checking fs roots
checking root refs
found 28672 bytes used err is 0
total csum bytes: 0
total tree bytes: 28672
total fs tree bytes: 8192
btree space waste bytes: 23875
file data blocks allocated: 0
referenced 0
Btrfs Btrfs v0.19
etc, etc.
And after that I could start a QEMU VM or an UserModeLinux kernel image,
passing this fs.img to it as a block device -- all while still being a regular
user, without needing root privileges.
On 01/25/2013 07:17 AM, Stefan Behrens wrote: > On Fri, 25 Jan 2013 07:03:19 -0500, Gene Czarcinski wrote: >> On 01/25/2013 06:41 AM, Stefan Behrens wrote: >>> On Fri, 25 Jan 2013 06:32:30 -0500, Gene Czarcinski wrote: >>>> This patch hits a lot of files but adds little code. It >>>> could be considered a bugfix, Currently, when one of the >>>> btrfs user-space programs is executed by a regular user, >>>> the result if oftem a number of strange error messages >>>> which do not indicate the real problem. This patch changes >>>> that situation. >>>> >>>> A test is performed as to whether the program is running >>>> as root. If it is not, issue an error message and exit. >>>> Signed-off-by: Gene Czarcinski <gene@czarc.net> >>>> --- >>>> btrfs-corrupt-block.c | 5 +++++ >>>> btrfs-image.c | 5 +++++ >>>> btrfs-map-logical.c | 5 +++++ >>>> btrfs-select-super.c | 5 +++++ >>>> btrfs-show-super.c | 5 +++++ >>>> btrfs-show.c | 5 +++++ >>>> btrfs-vol.c | 5 +++++ >>>> btrfs-zero-log.c | 5 +++++ >>>> btrfs.c | 6 ++++++ >>>> btrfsck.c | 5 +++++ >>>> btrfsctl.c | 5 +++++ >>>> btrfstune.c | 5 +++++ >>>> calc-size.c | 5 +++++ >>>> convert.c | 6 ++++++ >>>> debug-tree.c | 5 +++++ >>>> dir-test.c | 5 +++++ >>>> find-root.c | 5 +++++ >>>> ioctl-test.c | 6 ++++++ >>>> mkfs.c | 5 +++++ >>>> quick-test.c | 6 ++++++ >>>> restore.c | 5 +++++ >>>> 21 files changed, 109 insertions(+) >>>> >>>> diff --git a/btrfs-corrupt-block.c b/btrfs-corrupt-block.c >>>> index b57e757..083fd50 100644 >>>> --- a/btrfs-corrupt-block.c >>>> +++ b/btrfs-corrupt-block.c >>>> @@ -296,6 +296,11 @@ int main(int ac, char **av) >>>> srand(128); >>>> + if (geteuid() != 0) { >>>> + fprintf(stderr,"Error: %s must run as root\n", argv[0]); >>>> + exit(1); >>>> + } >>>> + >>>> while(1) { >>>> int c; >>>> c = getopt_long(ac, av, "l:c:b:eEk", long_options, >>>> diff --git a/btrfs-image.c b/btrfs-image.c >>>> index 7dc131d..fd9b28a 100644 >>>> --- a/btrfs-image.c >>>> +++ b/btrfs-image.c >>>> @@ -831,6 +831,11 @@ int main(int argc, char *argv[]) >>>> int ret; >>>> FILE *out; >>>> + if (geteuid() != 0) { >>>> + fprintf(stderr,"Error: %s must run as root\n", argv[0]); >>>> + exit(1); >>>> + } >>>> + >>>> while (1) { >>>> int c = getopt(argc, argv, "rc:t:"); >>>> if (c < 0) >>>> diff --git a/btrfs-map-logical.c b/btrfs-map-logical.c >>>> index fa4fb3f..59f2f0e 100644 >>>> --- a/btrfs-map-logical.c >>>> +++ b/btrfs-map-logical.c >>>> @@ -116,6 +116,11 @@ int main(int ac, char **av) >>>> int out_fd = 0; >>>> int err; >>>> + if (geteuid() != 0) { >>>> + fprintf(stderr,"Error: %s must run as root\n", av[0]); >>>> + exit(1); >>>> + } >>>> + >>>> while(1) { >>>> int c; >>>> c = getopt_long(ac, av, "l:c:o:b:", long_options, >>>> diff --git a/btrfs-select-super.c b/btrfs-select-super.c >>>> index 0c4f5c0..049379d 100644 >>>> --- a/btrfs-select-super.c >>>> +++ b/btrfs-select-super.c >>>> @@ -46,6 +46,11 @@ int main(int ac, char **av) >>>> int num; >>>> u64 bytenr = 0; >>>> + if (geteuid() != 0) { >>>> + fprintf(stderr,"Error: %s must run as root\n", argv[0]); >>>> + exit(1); >>>> + } >>>> + >>>> while(1) { >>>> int c; >>>> c = getopt(ac, av, "s:"); >>>> diff --git a/btrfs-show-super.c b/btrfs-show-super.c >>>> index a9e2524..2fa4776 100644 >>>> --- a/btrfs-show-super.c >>>> +++ b/btrfs-show-super.c >>>> @@ -63,6 +63,11 @@ int main(int argc, char **argv) >>>> int arg, i; >>>> u64 sb_bytenr = btrfs_sb_offset(0); >>>> + if (geteuid() != 0) { >>>> + fprintf(stderr,"Error: %s must run as root\n", argv[0]); >>>> + exit(1); >>>> + } >>>> + >>>> while ((opt = getopt(argc, argv, "ai:")) != -1) { >>>> switch (opt) { >>>> case 'i': >>>> diff --git a/btrfs-show.c b/btrfs-show.c >>>> index 8210fd2..6b3b91a 100644 >>>> --- a/btrfs-show.c >>>> +++ b/btrfs-show.c >>>> @@ -122,6 +122,11 @@ int main(int ac, char **av) >>>> "** Please consider to switch to the btrfs utility\n" >>>> "**\n"); >>>> + if (geteuid() != 0) { >>>> + fprintf(stderr,"Error: %s must run as root\n", av[0]); >>>> + exit(1); >>>> + } >>>> + >>>> while(1) { >>>> int c; >>>> c = getopt_long(ac, av, "", long_options, >>>> diff --git a/btrfs-vol.c b/btrfs-vol.c >>>> index ad824bd..7e02f72 100644 >>>> --- a/btrfs-vol.c >>>> +++ b/btrfs-vol.c >>>> @@ -83,6 +83,11 @@ int main(int ac, char **av) >>>> "** Please consider to switch to the btrfs utility\n" >>>> "**\n"); >>>> + if (geteuid() != 0) { >>>> + fprintf(stderr,"Error: %s must run as root\n", av[0]); >>>> + exit(1); >>>> + } >>>> + >>>> while(1) { >>>> int c; >>>> c = getopt_long(ac, av, "a:br:", long_options, >>>> diff --git a/btrfs-zero-log.c b/btrfs-zero-log.c >>>> index 1ea867b..80e4e38 100644 >>>> --- a/btrfs-zero-log.c >>>> +++ b/btrfs-zero-log.c >>>> @@ -45,6 +45,11 @@ int main(int ac, char **av) >>>> struct btrfs_trans_handle *trans; >>>> int ret; >>>> + if (geteuid() != 0) { >>>> + fprintf(stderr,"Error: %s must run as root\n", av[0]); >>>> + exit(1); >>>> + } >>>> + >>>> if (ac != 2) >>>> print_usage(); >>>> diff --git a/btrfs.c b/btrfs.c >>>> index 687acec..328966b 100644 >>>> --- a/btrfs.c >>>> +++ b/btrfs.c >>>> @@ -18,6 +18,7 @@ >>>> #include <stdio.h> >>>> #include <stdlib.h> >>>> #include <string.h> >>>> +#include <unistd.h> >>>> #include "crc32c.h" >>>> #include "commands.h" >>>> @@ -261,6 +262,11 @@ int main(int argc, char **argv) >>>> { >>>> const struct cmd_struct *cmd; >>>> + if (geteuid() != 0) { >>>> + fprintf(stderr,"Error: %s must run as root\n", argv[0]); >>>> + exit(1); >>>> + } >>>> + >>>> crc32c_optimization_init(); >>>> argc--; >>>> diff --git a/btrfsck.c b/btrfsck.c >>>> index 6274ff7..bdfdfc5 100644 >>>> --- a/btrfsck.c >>>> +++ b/btrfsck.c >>>> @@ -3501,6 +3501,11 @@ int main(int ac, char **av) >>>> int init_csum_tree = 0; >>>> int rw = 0; >>>> + if (geteuid() != 0) { >>>> + fprintf(stderr,"Error: %s must run as root\n", av[0]); >>>> + exit(1); >>>> + } >>>> + >>>> while(1) { >>>> int c; >>>> c = getopt_long(ac, av, "as:", long_options, >>>> diff --git a/btrfsctl.c b/btrfsctl.c >>>> index 049a5f3..cbe41e7 100644 >>>> --- a/btrfsctl.c >>>> +++ b/btrfsctl.c >>>> @@ -113,6 +113,11 @@ int main(int ac, char **av) >>>> "** Please consider to switch to the btrfs utility\n" >>>> "**\n"); >>>> >>>> + if (geteuid() != 0) { >>>> + fprintf(stderr,"Error: %s must run as root\n", av[0]); >>>> + exit(1); >>>> + } >>>> + >>>> if (ac == 2 && strcmp(av[1], "-a") == 0) { >>>> fprintf(stderr, "Scanning for Btrfs filesystems\n"); >>>> btrfs_scan_one_dir("/dev", 1); >>>> diff --git a/btrfstune.c b/btrfstune.c >>>> index 6950f74..d4017f1 100644 >>>> --- a/btrfstune.c >>>> +++ b/btrfstune.c >>>> @@ -79,6 +79,11 @@ int main(int argc, char *argv[]) >>>> int seeding_value = 0; >>>> int ret; >>>> + if (geteuid() != 0) { >>>> + fprintf(stderr,"Error: %s must run as root\n", argv[0]); >>>> + exit(1); >>>> + } >>>> + >>>> while(1) { >>>> int c = getopt(argc, argv, "S:"); >>>> if (c < 0) >>>> diff --git a/calc-size.c b/calc-size.c >>>> index c4adfb0..0d3442c 100644 >>>> --- a/calc-size.c >>>> +++ b/calc-size.c >>>> @@ -194,6 +194,11 @@ int main(int argc, char **argv) >>>> int opt; >>>> int ret = 0; >>>> + if (geteuid() != 0) { >>>> + fprintf(stderr,"Error: %s must run as root\n", argv[0]); >>>> + exit(1); >>>> + } >>>> + >>>> while ((opt = getopt(argc, argv, "vb")) != -1) { >>>> switch (opt) { >>>> case 'v': >>>> diff --git a/convert.c b/convert.c >>>> index 1de2a44..1b0e27c 100644 >>>> --- a/convert.c >>>> +++ b/convert.c >>>> @@ -2770,6 +2770,12 @@ int main(int argc, char *argv[]) >>>> int datacsum = 1; >>>> int rollback = 0; >>>> char *file; >>>> + >>>> + if (geteuid() != 0) { >>>> + fprintf(stderr,"Error: %s must run as root\n", argv[0]); >>>> + exit(1); >>>> + } >>>> + >>>> while(1) { >>>> int c = getopt(argc, argv, "dinr"); >>>> if (c < 0) >>>> diff --git a/debug-tree.c b/debug-tree.c >>>> index f6bd5d8..5b2f531 100644 >>>> --- a/debug-tree.c >>>> +++ b/debug-tree.c >>>> @@ -129,6 +129,11 @@ int main(int ac, char **av) >>>> u64 block_only = 0; >>>> struct btrfs_root *tree_root_scan; >>>> + if (geteuid() != 0) { >>>> + fprintf(stderr,"Error: %s must run as root\n", av[0]); >>>> + exit(1); >>>> + } >>>> + >>>> radix_tree_init(); >>>> while(1) { >>>> diff --git a/dir-test.c b/dir-test.c >>>> index c7644d6..9fa5b06 100644 >>>> --- a/dir-test.c >>>> +++ b/dir-test.c >>>> @@ -433,6 +433,11 @@ int main(int ac, char **av) >>>> int err = 0; >>>> int initial_only = 0; >>>> struct btrfs_trans_handle *trans; >>>> + if (geteuid() != 0) { >>>> + fprintf(stderr,"Error: %s must run as root\n", argv[0]); >>>> + exit(1); >>>> + } >>>> + >>>> radix_tree_init(); >>>> root = open_ctree(av[ac-1], &super, 0); >>>> diff --git a/find-root.c b/find-root.c >>>> index 83f1592..06465eb 100644 >>>> --- a/find-root.c >>>> +++ b/find-root.c >>>> @@ -414,6 +414,11 @@ int main(int argc, char **argv) >>>> int opt; >>>> int ret; >>>> + if (geteuid() != 0) { >>>> + fprintf(stderr,"Error: %s must run as root\n", argv[0]); >>>> + exit(1); >>>> + } >>>> + >>>> while ((opt = getopt(argc, argv, "vo:")) != -1) { >>>> switch(opt) { >>>> case 'v': >>>> diff --git a/ioctl-test.c b/ioctl-test.c >>>> index 1c27d61..299d2af 100644 >>>> --- a/ioctl-test.c >>>> +++ b/ioctl-test.c >>>> @@ -1,5 +1,6 @@ >>>> #include <stdio.h> >>>> #include <stdlib.h> >>>> +#include <unistd.h> >>>> #include "kerncompat.h" >>>> #include "ioctl.h" >>>> @@ -28,6 +29,11 @@ unsigned long ioctls[] = { >>>> int main(int ac, char **av) >>>> { >>>> int i = 0; >>>> + if (geteuid() != 0) { >>>> + fprintf(stderr,"Error: %s must run as root\n", argv[0]); >>>> + exit(1); >>>> + } >>>> + >>>> while(ioctls[i]) { >>>> printf("%lu\n" ,ioctls[i]); >>>> i++; >>>> diff --git a/mkfs.c b/mkfs.c >>>> index a129ec4..501e384 100644 >>>> --- a/mkfs.c >>>> +++ b/mkfs.c >>>> @@ -1274,6 +1274,11 @@ int main(int ac, char **av) >>>> u64 source_dir_size = 0; >>>> char *pretty_buf; >>>> + if (geteuid() != 0) { >>>> + fprintf(stderr,"Error: %s must run as root\n", av[0]); >>>> + exit(1); >>>> + } >>>> + >>>> while(1) { >>>> int c; >>>> c = getopt_long(ac, av, "A:b:l:n:s:m:d:L:r:VMK", long_options, >>>> diff --git a/quick-test.c b/quick-test.c >>>> index 05d73fd..e2d6f78 100644 >>>> --- a/quick-test.c >>>> +++ b/quick-test.c >>>> @@ -19,6 +19,7 @@ >>>> #include <stdio.h> >>>> #include <stdlib.h> >>>> #include <fcntl.h> >>>> +#include <unistd.h> >>>> #include "kerncompat.h" >>>> #include "radix-tree.h" >>>> #include "ctree.h" >>>> @@ -49,6 +50,11 @@ int main(int ac, char **av) { >>>> buf = malloc(512); >>>> memset(buf, 0, 512); >>>> + if (geteuid() != 0) { >>>> + fprintf(stderr,"Error: %s must run as root\n", argv[0]); >>>> + exit(1); >>>> + } >>>> + >>>> radix_tree_init(); >>>> root = open_ctree(av[1], BTRFS_SUPER_INFO_OFFSET, O_RDWR); >>>> diff --git a/restore.c b/restore.c >>>> index 80afb84..4efc8b5 100644 >>>> --- a/restore.c >>>> +++ b/restore.c >>>> @@ -771,6 +771,11 @@ int main(int argc, char **argv) >>>> int super_mirror = 0; >>>> int find_dir = 0; >>>> + if (geteuid() != 0) { >>>> + fprintf(stderr,"Error: %s must run as root\n", argv[0]); >>>> + exit(1); >>>> + } >>>> + >>>> while ((opt = getopt(argc, argv, "sviot:u:df:")) != -1) { >>>> switch (opt) { >>>> case 's': >>>> >>> 21 times copy & paste, you set a new record :) >>> >> I was very tempted to do a little more. I know that there is no >> standard that says the two parameters of main() are named argc and argv >> but it is traditional. I could not believe I got errors because it was >> named av instead of argv. But, patches like this should stay on topic. > What I wanted to say was, put this duplicated code in a subfunction :) > You'll still have to copy & paste 21 times, but just a one-liner. > > util.c: > void exit_if_not_superuser(const char *progname) > ... > > I though of doing that but this is just as easy. Also, if some but not all of these need to become warnings, that will be easier. In addition, there would be the cases where utils.o is not normally included in a specific program. The big question I have for others is if this is worth it. Gene -- 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
On Fri, 25 Jan 2013, Roman Mamedov <rm@romanrm.ru> wrote: > The user does not have to be root, they can be a member of the group "disk" > to manage this device. > > Also some or all of the tools accept not just a block device, but also a > regular file as their parameter. > > Wouldn't it be better to check whether or not the running user has > write access to the device or file to be operated on, before failing? Also UID==0 doesn't necessarily mean ultimate access to the system. The case where the process is running as root but still lacks the access to perform the operations in question should also be handled. Yes, ability to read/write the device/file or whatever is being operated on should be the criteria that is used not UID etc.
OK, I think I have gotten the message that this is a bad idea as
implemented and that it should be dropped as such. I believe that there
are some things ("btrfs fi show" comes to mind) which will need root and
I am going to explore doing something for that case. And it also might
be reasonable for some situations to issue the message about root if
something errors-out.
Anyway, this approach is dead and I will continue to give this some thought.
Comments?
Gene
On 01/25/2013 06:32 AM, Gene Czarcinski wrote:
> This patch hits a lot of files but adds little code. It
> could be considered a bugfix, Currently, when one of the
> btrfs user-space programs is executed by a regular user,
> the result if oftem a number of strange error messages
> which do not indicate the real problem. This patch changes
> that situation.
>
> A test is performed as to whether the program is running
> as root. If it is not, issue an error message and exit.
> Signed-off-by: Gene Czarcinski<gene@czarc.net>
> ---
> btrfs-corrupt-block.c | 5 +++++
> btrfs-image.c | 5 +++++
> btrfs-map-logical.c | 5 +++++
> btrfs-select-super.c | 5 +++++
> btrfs-show-super.c | 5 +++++
> btrfs-show.c | 5 +++++
> btrfs-vol.c | 5 +++++
> btrfs-zero-log.c | 5 +++++
> btrfs.c | 6 ++++++
> btrfsck.c | 5 +++++
> btrfsctl.c | 5 +++++
> btrfstune.c | 5 +++++
> calc-size.c | 5 +++++
> convert.c | 6 ++++++
> debug-tree.c | 5 +++++
> dir-test.c | 5 +++++
> find-root.c | 5 +++++
> ioctl-test.c | 6 ++++++
> mkfs.c | 5 +++++
> quick-test.c | 6 ++++++
> restore.c | 5 +++++
> 21 files changed, 109 insertions(+)
--
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
On 1/25/13 5:32 AM, Gene Czarcinski wrote: > This patch hits a lot of files but adds little code. It > could be considered a bugfix, Currently, when one of the > btrfs user-space programs is executed by a regular user, > the result if oftem a number of strange error messages > which do not indicate the real problem. This patch changes > that situation. > > A test is performed as to whether the program is running > as root. If it is not, issue an error message and exit. > Signed-off-by: Gene Czarcinski <gene@czarc.net> > --- I agree with others that this isn't the right approach, I'm afraid. But can we back up a little - >> Currently, when one of the >> btrfs user-space programs is executed by a regular user, >> the result if oftem a number of strange error messages >> which do not indicate the real problem. Can you elaborate on what situations those are, and what messages appear? I'm guessing that it just requires some error handling fixes. -Eric -- 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
On 01/25/2013 10:04 AM, Gene Czarcinski wrote: > OK, I think I have gotten the message that this is a bad idea as > implemented and that it should be dropped as such. I believe that > there are some things ("btrfs fi show" comes to mind) which will need > root and I am going to explore doing something for that case. And it > also might be reasonable for some situations to issue the message > about root if something errors-out. > > Anyway, this approach is dead and I will continue to give this some > thought. > > Comments? > > Gene > > > > On 01/25/2013 06:32 AM, Gene Czarcinski wrote: >> This patch hits a lot of files but adds little code. It >> could be considered a bugfix, Currently, when one of the >> btrfs user-space programs is executed by a regular user, >> the result if oftem a number of strange error messages >> which do not indicate the real problem. This patch changes >> that situation. >> >> A test is performed as to whether the program is running >> as root. If it is not, issue an error message and exit. >> Signed-off-by: Gene Czarcinski<gene@czarc.net> >> --- >> btrfs-corrupt-block.c | 5 +++++ >> btrfs-image.c | 5 +++++ >> btrfs-map-logical.c | 5 +++++ >> btrfs-select-super.c | 5 +++++ >> btrfs-show-super.c | 5 +++++ >> btrfs-show.c | 5 +++++ >> btrfs-vol.c | 5 +++++ >> btrfs-zero-log.c | 5 +++++ >> btrfs.c | 6 ++++++ >> btrfsck.c | 5 +++++ >> btrfsctl.c | 5 +++++ >> btrfstune.c | 5 +++++ >> calc-size.c | 5 +++++ >> convert.c | 6 ++++++ >> debug-tree.c | 5 +++++ >> dir-test.c | 5 +++++ >> find-root.c | 5 +++++ >> ioctl-test.c | 6 ++++++ >> mkfs.c | 5 +++++ >> quick-test.c | 6 ++++++ >> restore.c | 5 +++++ >> 21 files changed, 109 insertions(+) > BTW, I want to thank all of you who commented. I have seen far too many submitted patches land with a thump and nothing more ... no comments ... either good or bad. Gene -- 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
On 25/01/13 14:43, Hugo Mills wrote: > On Fri, Jan 25, 2013 at 07:29:44AM -0500, Gene Czarcinski wrote: >> On 01/25/2013 06:55 AM, Roman Mamedov wrote: >>> On Fri, 25 Jan 2013 06:32:30 -0500 >>> Gene Czarcinski <gene@czarc.net> wrote: >>> >>>> This patch hits a lot of files but adds little code. It >>>> could be considered a bugfix, Currently, when one of the >>>> btrfs user-space programs is executed by a regular user, >>>> the result if oftem a number of strange error messages >>>> which do not indicate the real problem. This patch changes >>>> that situation. >>>> >>>> A test is performed as to whether the program is running >>>> as root. If it is not, issue an error message and exit. >>>> Signed-off-by: Gene Czarcinski <gene@czarc.net> >>> $ ls -la /dev/sda >>> brw-rw---T 1 root disk 8, 0 Jan 15 12:11 /dev/sda >>> >>> The user does not have to be root, they can be a member of the group "disk" to >>> manage this device. >>> >>> Also some or all of the tools accept not just a block device, but also a >>> regular file as their parameter. >>> >>> Wouldn't it be better to check whether or not the running user has >>> *write access* to the device or file to be operated on, before failing? >> I knew there would be corner cases where root was not required for >> execution. After all, I do not need to be root to execute "btrfs >> --version". Now, is it worth the effort to determine the corner >> cases and do you have a proposed solution as to determining what >> privileges are needed when? I can understand when it could be a >> regular file but is it all that common for users to be part of group >> disk? > Don't try to check all the possible success conditions beforehand > -- that's what leads to websites that fail to work because your > browser is not IE, but work perfectly when you change your user-agent > string to "MSIE". This is highly frustrating for users. > > Instead, try whatever it is you were trying to do (open a file, > send an ioctl), and determine, as well as you can, why it failed by > looking at the error codes that you get back, and report that. > "Permission denied" -> means you don't have permissions -> you need to > be root, or have yourself put in the disk group, or get the > disk-management-capability. Let the user work out which of those > solutions they need, rather than forcing them to use the one you > thought of. > > Hugo. As Hugo suggested, I'd rather that we fix or refine the code in order to get better error messages. All the different exceptions to requiring or not requiring root overly complicates things that, strictly speaking, shouldn't need to be handled in advance.
On Fri, Jan 25, 2013 at 9:04 AM, Gene Czarcinski <gene@czarc.net> wrote: > OK, I think I have gotten the message that this is a bad idea as implemented > and that it should be dropped as such. I believe that there are some things > ("btrfs fi show" comes to mind) which will need root and I am going to > explore doing something for that case. And it also might be reasonable for > some situations to issue the message about root if something errors-out. Eh? That's one of the clearest cases where you _may not_ need root. cwillu@cwillu-home:~$ groups cwillu adm dialout cdrom audio video plugdev mlocate lpadmin admin sambashare cwillu@cwillu-home:~$ btrfs fi show /dev/sda3 failed to read /dev/sda failed to read /dev/sda1 failed to read /dev/sda2 failed to read /dev/sda3 failed to read /dev/sdb Btrfs v0.19-152-g1957076 cwillu@cwillu-home:~$ sudo addgroup cwillu disk cwillu@cwillu-home:~$ su cwillu cwillu@cwillu-home:~$ groups cwillu adm disk dialout cdrom audio video plugdev mlocate lpadmin admin sambashare cwillu@cwillu-home:~$ btrfs fi show /dev/sda3 Label: none uuid: ede59711-6230-474f-992d-f1e3deeddab7 Total devices 1 FS bytes used 72.12GB devid 1 size 104.34GB used 104.34GB path /dev/sda3 Btrfs v0.19-152-g1957076 -- 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
On Sat, 26 Jan 2013, Gene Czarcinski <gene@czarc.net> wrote: > OK, I think I have gotten the message that this is a bad idea as > implemented and that it should be dropped as such. I believe that there > are some things ("btrfs fi show" comes to mind) which will need root and > I am going to explore doing something for that case. And it also might > be reasonable for some situations to issue the message about root if > something errors-out. I think that a message such as Eric proposed of "failed to open /dev/sda: Permission denied" is clear enough. If you run as non-root on a system with no security system other than Unix permissions then it will be quite obvious that such an error can be fixed by running as root. But if you are running SE Linux or some other security system then you could be prevented from running the program without the root/non-root status of it being relevant.
On 01/26/2013 03:18 AM, Russell Coker wrote: > On Sat, 26 Jan 2013, Gene Czarcinski<gene@czarc.net> wrote: >> OK, I think I have gotten the message that this is a bad idea as >> implemented and that it should be dropped as such. I believe that there >> are some things ("btrfs fi show" comes to mind) which will need root and >> I am going to explore doing something for that case. And it also might >> be reasonable for some situations to issue the message about root if >> something errors-out. > > I think that a message such as Eric proposed of "failed to open /dev/sda: > Permission denied" is clear enough. If you run as non-root on a system with > no security system other than Unix permissions then it will be quite obvious > that such an error can be fixed by running as root. Being pedantic, I would point out that the kernel is checking if the user has CAP_SYS_ADMIN, which could be different than geteuid() == 0. I can have both a root users without CAP_SYS_ADMIN and a normal user with this capability. If I can make a suggestion, the work of Gene could be changed in checking which command really requires to be root. To day I can create a subvolume, and remove a subvolume [1] without needing root. I am guessing if there is a valid reason to require "root" to list the subvolumes. I know that behind the command "find-subvolumes" there is the ioctl BTRFS_IOC_TREE_SEARCH which requires to be root, because it could export some sensible information. But this can be easy solved creating an ioctl which export only not-sensible data (i.e. it export only the name and the path of the subvolume). I think that a good analysis of these situations could improve the btrfs usability. My 2ยข BR G.Baroncelli > > But if you are running SE Linux or some other security system then you could > be prevented from running the program without the root/non-root status of it > being relevant. > [1] Passing user_subvol_rm_allowed in option
diff --git a/btrfs-corrupt-block.c b/btrfs-corrupt-block.c index b57e757..083fd50 100644 --- a/btrfs-corrupt-block.c +++ b/btrfs-corrupt-block.c @@ -296,6 +296,11 @@ int main(int ac, char **av) srand(128); + if (geteuid() != 0) { + fprintf(stderr,"Error: %s must run as root\n", argv[0]); + exit(1); + } + while(1) { int c; c = getopt_long(ac, av, "l:c:b:eEk", long_options, diff --git a/btrfs-image.c b/btrfs-image.c index 7dc131d..fd9b28a 100644 --- a/btrfs-image.c +++ b/btrfs-image.c @@ -831,6 +831,11 @@ int main(int argc, char *argv[]) int ret; FILE *out; + if (geteuid() != 0) { + fprintf(stderr,"Error: %s must run as root\n", argv[0]); + exit(1); + } + while (1) { int c = getopt(argc, argv, "rc:t:"); if (c < 0) diff --git a/btrfs-map-logical.c b/btrfs-map-logical.c index fa4fb3f..59f2f0e 100644 --- a/btrfs-map-logical.c +++ b/btrfs-map-logical.c @@ -116,6 +116,11 @@ int main(int ac, char **av) int out_fd = 0; int err; + if (geteuid() != 0) { + fprintf(stderr,"Error: %s must run as root\n", av[0]); + exit(1); + } + while(1) { int c; c = getopt_long(ac, av, "l:c:o:b:", long_options, diff --git a/btrfs-select-super.c b/btrfs-select-super.c index 0c4f5c0..049379d 100644 --- a/btrfs-select-super.c +++ b/btrfs-select-super.c @@ -46,6 +46,11 @@ int main(int ac, char **av) int num; u64 bytenr = 0; + if (geteuid() != 0) { + fprintf(stderr,"Error: %s must run as root\n", argv[0]); + exit(1); + } + while(1) { int c; c = getopt(ac, av, "s:"); diff --git a/btrfs-show-super.c b/btrfs-show-super.c index a9e2524..2fa4776 100644 --- a/btrfs-show-super.c +++ b/btrfs-show-super.c @@ -63,6 +63,11 @@ int main(int argc, char **argv) int arg, i; u64 sb_bytenr = btrfs_sb_offset(0); + if (geteuid() != 0) { + fprintf(stderr,"Error: %s must run as root\n", argv[0]); + exit(1); + } + while ((opt = getopt(argc, argv, "ai:")) != -1) { switch (opt) { case 'i': diff --git a/btrfs-show.c b/btrfs-show.c index 8210fd2..6b3b91a 100644 --- a/btrfs-show.c +++ b/btrfs-show.c @@ -122,6 +122,11 @@ int main(int ac, char **av) "** Please consider to switch to the btrfs utility\n" "**\n"); + if (geteuid() != 0) { + fprintf(stderr,"Error: %s must run as root\n", av[0]); + exit(1); + } + while(1) { int c; c = getopt_long(ac, av, "", long_options, diff --git a/btrfs-vol.c b/btrfs-vol.c index ad824bd..7e02f72 100644 --- a/btrfs-vol.c +++ b/btrfs-vol.c @@ -83,6 +83,11 @@ int main(int ac, char **av) "** Please consider to switch to the btrfs utility\n" "**\n"); + if (geteuid() != 0) { + fprintf(stderr,"Error: %s must run as root\n", av[0]); + exit(1); + } + while(1) { int c; c = getopt_long(ac, av, "a:br:", long_options, diff --git a/btrfs-zero-log.c b/btrfs-zero-log.c index 1ea867b..80e4e38 100644 --- a/btrfs-zero-log.c +++ b/btrfs-zero-log.c @@ -45,6 +45,11 @@ int main(int ac, char **av) struct btrfs_trans_handle *trans; int ret; + if (geteuid() != 0) { + fprintf(stderr,"Error: %s must run as root\n", av[0]); + exit(1); + } + if (ac != 2) print_usage(); diff --git a/btrfs.c b/btrfs.c index 687acec..328966b 100644 --- a/btrfs.c +++ b/btrfs.c @@ -18,6 +18,7 @@ #include <stdio.h> #include <stdlib.h> #include <string.h> +#include <unistd.h> #include "crc32c.h" #include "commands.h" @@ -261,6 +262,11 @@ int main(int argc, char **argv) { const struct cmd_struct *cmd; + if (geteuid() != 0) { + fprintf(stderr,"Error: %s must run as root\n", argv[0]); + exit(1); + } + crc32c_optimization_init(); argc--; diff --git a/btrfsck.c b/btrfsck.c index 6274ff7..bdfdfc5 100644 --- a/btrfsck.c +++ b/btrfsck.c @@ -3501,6 +3501,11 @@ int main(int ac, char **av) int init_csum_tree = 0; int rw = 0; + if (geteuid() != 0) { + fprintf(stderr,"Error: %s must run as root\n", av[0]); + exit(1); + } + while(1) { int c; c = getopt_long(ac, av, "as:", long_options, diff --git a/btrfsctl.c b/btrfsctl.c index 049a5f3..cbe41e7 100644 --- a/btrfsctl.c +++ b/btrfsctl.c @@ -113,6 +113,11 @@ int main(int ac, char **av) "** Please consider to switch to the btrfs utility\n" "**\n"); + if (geteuid() != 0) { + fprintf(stderr,"Error: %s must run as root\n", av[0]); + exit(1); + } + if (ac == 2 && strcmp(av[1], "-a") == 0) { fprintf(stderr, "Scanning for Btrfs filesystems\n"); btrfs_scan_one_dir("/dev", 1); diff --git a/btrfstune.c b/btrfstune.c index 6950f74..d4017f1 100644 --- a/btrfstune.c +++ b/btrfstune.c @@ -79,6 +79,11 @@ int main(int argc, char *argv[]) int seeding_value = 0; int ret; + if (geteuid() != 0) { + fprintf(stderr,"Error: %s must run as root\n", argv[0]); + exit(1); + } + while(1) { int c = getopt(argc, argv, "S:"); if (c < 0) diff --git a/calc-size.c b/calc-size.c index c4adfb0..0d3442c 100644 --- a/calc-size.c +++ b/calc-size.c @@ -194,6 +194,11 @@ int main(int argc, char **argv) int opt; int ret = 0; + if (geteuid() != 0) { + fprintf(stderr,"Error: %s must run as root\n", argv[0]); + exit(1); + } + while ((opt = getopt(argc, argv, "vb")) != -1) { switch (opt) { case 'v': diff --git a/convert.c b/convert.c index 1de2a44..1b0e27c 100644 --- a/convert.c +++ b/convert.c @@ -2770,6 +2770,12 @@ int main(int argc, char *argv[]) int datacsum = 1; int rollback = 0; char *file; + + if (geteuid() != 0) { + fprintf(stderr,"Error: %s must run as root\n", argv[0]); + exit(1); + } + while(1) { int c = getopt(argc, argv, "dinr"); if (c < 0) diff --git a/debug-tree.c b/debug-tree.c index f6bd5d8..5b2f531 100644 --- a/debug-tree.c +++ b/debug-tree.c @@ -129,6 +129,11 @@ int main(int ac, char **av) u64 block_only = 0; struct btrfs_root *tree_root_scan; + if (geteuid() != 0) { + fprintf(stderr,"Error: %s must run as root\n", av[0]); + exit(1); + } + radix_tree_init(); while(1) { diff --git a/dir-test.c b/dir-test.c index c7644d6..9fa5b06 100644 --- a/dir-test.c +++ b/dir-test.c @@ -433,6 +433,11 @@ int main(int ac, char **av) int err = 0; int initial_only = 0; struct btrfs_trans_handle *trans; + if (geteuid() != 0) { + fprintf(stderr,"Error: %s must run as root\n", argv[0]); + exit(1); + } + radix_tree_init(); root = open_ctree(av[ac-1], &super, 0); diff --git a/find-root.c b/find-root.c index 83f1592..06465eb 100644 --- a/find-root.c +++ b/find-root.c @@ -414,6 +414,11 @@ int main(int argc, char **argv) int opt; int ret; + if (geteuid() != 0) { + fprintf(stderr,"Error: %s must run as root\n", argv[0]); + exit(1); + } + while ((opt = getopt(argc, argv, "vo:")) != -1) { switch(opt) { case 'v': diff --git a/ioctl-test.c b/ioctl-test.c index 1c27d61..299d2af 100644 --- a/ioctl-test.c +++ b/ioctl-test.c @@ -1,5 +1,6 @@ #include <stdio.h> #include <stdlib.h> +#include <unistd.h> #include "kerncompat.h" #include "ioctl.h" @@ -28,6 +29,11 @@ unsigned long ioctls[] = { int main(int ac, char **av) { int i = 0; + if (geteuid() != 0) { + fprintf(stderr,"Error: %s must run as root\n", argv[0]); + exit(1); + } + while(ioctls[i]) { printf("%lu\n" ,ioctls[i]); i++; diff --git a/mkfs.c b/mkfs.c index a129ec4..501e384 100644 --- a/mkfs.c +++ b/mkfs.c @@ -1274,6 +1274,11 @@ int main(int ac, char **av) u64 source_dir_size = 0; char *pretty_buf; + if (geteuid() != 0) { + fprintf(stderr,"Error: %s must run as root\n", av[0]); + exit(1); + } + while(1) { int c; c = getopt_long(ac, av, "A:b:l:n:s:m:d:L:r:VMK", long_options, diff --git a/quick-test.c b/quick-test.c index 05d73fd..e2d6f78 100644 --- a/quick-test.c +++ b/quick-test.c @@ -19,6 +19,7 @@ #include <stdio.h> #include <stdlib.h> #include <fcntl.h> +#include <unistd.h> #include "kerncompat.h" #include "radix-tree.h" #include "ctree.h" @@ -49,6 +50,11 @@ int main(int ac, char **av) { buf = malloc(512); memset(buf, 0, 512); + if (geteuid() != 0) { + fprintf(stderr,"Error: %s must run as root\n", argv[0]); + exit(1); + } + radix_tree_init(); root = open_ctree(av[1], BTRFS_SUPER_INFO_OFFSET, O_RDWR); diff --git a/restore.c b/restore.c index 80afb84..4efc8b5 100644 --- a/restore.c +++ b/restore.c @@ -771,6 +771,11 @@ int main(int argc, char **argv) int super_mirror = 0; int find_dir = 0; + if (geteuid() != 0) { + fprintf(stderr,"Error: %s must run as root\n", argv[0]); + exit(1); + } + while ((opt = getopt(argc, argv, "sviot:u:df:")) != -1) { switch (opt) { case 's':
This patch hits a lot of files but adds little code. It could be considered a bugfix, Currently, when one of the btrfs user-space programs is executed by a regular user, the result if oftem a number of strange error messages which do not indicate the real problem. This patch changes that situation. A test is performed as to whether the program is running as root. If it is not, issue an error message and exit. Signed-off-by: Gene Czarcinski <gene@czarc.net> --- btrfs-corrupt-block.c | 5 +++++ btrfs-image.c | 5 +++++ btrfs-map-logical.c | 5 +++++ btrfs-select-super.c | 5 +++++ btrfs-show-super.c | 5 +++++ btrfs-show.c | 5 +++++ btrfs-vol.c | 5 +++++ btrfs-zero-log.c | 5 +++++ btrfs.c | 6 ++++++ btrfsck.c | 5 +++++ btrfsctl.c | 5 +++++ btrfstune.c | 5 +++++ calc-size.c | 5 +++++ convert.c | 6 ++++++ debug-tree.c | 5 +++++ dir-test.c | 5 +++++ find-root.c | 5 +++++ ioctl-test.c | 6 ++++++ mkfs.c | 5 +++++ quick-test.c | 6 ++++++ restore.c | 5 +++++ 21 files changed, 109 insertions(+)