Re: [PATCH] Btrfs-progs: Exit if not running as root
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 g...@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();
Re: [PATCH] Btrfs-progs: Exit if not running as root
On Fri, 25 Jan 2013 06:32:30 -0500 Gene Czarcinski g...@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 g...@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. signature.asc Description: PGP signature
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 g...@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)
Re: [PATCH] Btrfs-progs: Exit if not running as root
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 g...@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
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 g...@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 g...@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 majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] Btrfs-progs: Exit if not running as root
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 g...@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 g...@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. --- signature.asc Description: Digital signature
Re: [PATCH] Btrfs-progs: Exit if not running as root
On Fri, 25 Jan 2013 07:29:44 -0500 Gene Czarcinski g...@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. signature.asc Description: PGP signature
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 g...@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
+++
Re: [PATCH] Btrfs-progs: Exit if not running as root
On Fri, 25 Jan 2013, Roman Mamedov r...@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 Bloghttp://doc.coker.com.au/ -- To unsubscribe from this list: send the line unsubscribe linux-btrfs in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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 Czarcinskig...@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 majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] Btrfs-progs: Exit if not running as root
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 g...@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 majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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 Czarcinskig...@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 majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] Btrfs-progs: Exit if not running as root
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 g...@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 g...@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 majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] Btrfs-progs: Exit if not running as root
On Fri, Jan 25, 2013 at 9:04 AM, Gene Czarcinski g...@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 devid1 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 majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] Btrfs-progs: Exit if not running as root
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 sand...@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 majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] Btrfs-progs: Exit if not running as root
On Sat, 26 Jan 2013, Gene Czarcinski g...@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 Bloghttp://doc.coker.com.au/ -- To unsubscribe from this list: send the line unsubscribe linux-btrfs in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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 Czarcinskig...@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 majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
