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'': -- 1.8.1 -- 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 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(+)-- With respect, Roman ~~~~~~~~~~~~~~~~~~~~~~~~~~~ "Stallman had a printer, with code he could not see. So he began to tinker, and set the software free."
Gene Czarcinski
2013-Jan-25 12:03 UTC
Re: [PATCH] Btrfs-progs: Exit if not running as root
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
Gene Czarcinski
2013-Jan-25 12:29 UTC
Re: [PATCH] Btrfs-progs: Exit if not running as root
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(+) >-- === Hugo Mills: hugo@... carfax.org.uk | darksatanic.net | lug.org.uk == PGP key: 515C238D from wwwkeys.eu.pgp.net or http://www.carfax.org.uk --- Quidquid latine dictum sit, altum videtur. ---
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. -- With respect, Roman ~~~~~~~~~~~~~~~~~~~~~~~~~~~ "Stallman had a printer, with code he could not see. So he began to tinker, and set the software free."
Gene Czarcinski
2013-Jan-25 13:22 UTC
Re: [PATCH] Btrfs-progs: Exit if not running as root
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. -- My Main Blog http://etbe.coker.com.au/ My Documents Blog http://doc.coker.com.au/ -- 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
Gene Czarcinski
2013-Jan-25 15:04 UTC
Re: [PATCH] Btrfs-progs: Exit if not running as root
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
Gene Czarcinski
2013-Jan-25 15:10 UTC
Re: [PATCH] Btrfs-progs: Exit if not running as root
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. -- __________ Brendan Hide http://swiftspirit.co.za/ http://www.webafrica.co.za/?AFF1E97 -- 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 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 1/25/13 9: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.So, in that particular case, I think the right fix is to make the code in that spot be more informative; there are probably a whole lot of places that could use fixes like this, though, not just this one. Still, it would be helpful to the user, I think. i.e. show: [testuser@host btrfs-progs]$ whoami testuser [testuser@host btrfs-progs]$ ./btrfs fi show failed to open /dev/sda: Permission denied failed to open /dev/sda1: Permission denied ... [PATCH] print more informative error when we fail to open a device If open() fails, we should let the user know why it failed. Signed-off-by: Eric Sandeen <sandeen@redhat.com> --- diff --git a/utils.c b/utils.c index 595850b..a5673f7 100644 --- a/utils.c +++ b/utils.c @@ -1237,7 +1237,8 @@ scan_again: fd = open(fullpath, O_RDONLY); if (fd < 0) { - fprintf(stderr, "failed to read %s\n", fullpath); + fprintf(stderr, "failed to open %s: %s\n", + fullpath, strerror(errno)); continue; } ret = btrfs_scan_one_device(fd, fullpath, &tmp_devices, -- 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. -- My Main Blog http://etbe.coker.com.au/ My Documents Blog http://doc.coker.com.au/ -- 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
Goffredo Baroncelli
2013-Jan-26 07:46 UTC
Re: [PATCH] Btrfs-progs: Exit if not running as root
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 -- gpg @keyserver.linux.it: Goffredo Baroncelli (kreijackATinwind.it> Key fingerprint BBF5 1610 0B64 DAC6 5F7D 17B2 0EDA 9B37 8B82 E0B5 -- 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