[patch] fileio.exp FAILs if run as root

[patch] fileio.exp FAILs if run as root

[Jan Kratochvil]

[-- Attachment #1: Type: text/plain, Size: 249 bytes --]

Hi,

if you run gdb.base/fileio.exp as UID 0 it will print:
	FAIL: gdb.base/fileio.exp: Open for write but no write permission returns EACCES
	FAIL: gdb.base/fileio.exp: Unlinking a file in a directory w/o write access returns EACCES


Regards,
Jan

[-- Attachment #2: gdb-test-fileio-root.patch --]
[-- Type: text/plain, Size: 3063 bytes --]

2007-12-08  Jan Kratochvil  <jan.kratochvil@redhat.com>

	* gdb.base/fileio.c (ROOTSUBDIR): New macro.
	(main): CHDIR into ROOTSUBDIR.  CHOWN ROOTSUBDIR and CHDIR into
	ROOTSUBDIR if we are being run as root.
	* gdb.base/fileio.exp: Change the startup and finish cleanup.
	Change the test file reference to be into the `fileio.dir' directory.

--- ./gdb/testsuite/gdb.base/fileio.c	13 Jun 2006 08:55:22 -0000	1.10
+++ ./gdb/testsuite/gdb.base/fileio.c	8 Dec 2007 16:04:10 -0000
@@ -58,6 +58,8 @@ system (const char * string);
 1) Invalid string/command. -  returns 127.  */
 static const char *strerrno (int err);
 
+#define ROOTSUBDIR "fileio.dir"
+
 #define FILENAME    "foo.fileio.test"
 #define RENAMED     "bar.fileio.test"
 #define NONEXISTANT "nofoo.fileio.test"
@@ -542,6 +544,37 @@ strerrno (int err)
 int
 main ()
 {
+  /* ROOTSUBDIR is already prepared by fileio.exp.  We use it for easy cleanup
+     (by fileio.exp) if we are run by multiple users in the same directory.  */
+
+  if (chdir (ROOTSUBDIR) != 0)
+    {
+      printf ("chdir " ROOTSUBDIR ": %s\n", strerror (errno));
+      exit (1);
+    }
+
+  /* These tests
+       Open for write but no write permission returns EACCES
+       Unlinking a file in a directory w/o write access returns EACCES
+     fail if we are being run as root - drop the privileges here.  */
+
+  if (geteuid () == 0)
+    {
+      uid_t uid = 99;
+
+      if (chown (".", uid, uid) != 0)
+	{
+	  printf ("chown %d.%d " ROOTSUBDIR ": %s\n", (int) uid, (int) uid,
+		  strerror (errno));
+	  exit (1);
+	}
+      if (setuid (uid) || geteuid () == 0)
+	{
+	  printf ("setuid %d: %s\n", (int) uid, strerror (errno));
+	  exit (1);
+	}
+    }
+
   /* Don't change the order of the calls.  They partly depend on each other */
   test_open ();
   test_write ();
--- ./gdb/testsuite/gdb.base/fileio.exp	23 Aug 2007 18:14:16 -0000	1.12
+++ ./gdb/testsuite/gdb.base/fileio.exp	8 Dec 2007 16:04:10 -0000
@@ -46,8 +46,8 @@ if [get_compiler_info ${binfile}] {
     return -1;
 }
 
-remote_exec build {sh -xc test\ -r\ dir2.fileio.test\ &&\ chmod\ -f\ +w\ dir2.fileio.test}
-remote_exec build {sh -xc rm\ -rf\ *.fileio.test}
+remote_exec build {sh -xc rm\ -rf\ fileio.dir}
+remote_exec build {sh -xc mkdir\ -m777\ fileio.dir}
 
 set oldtimeout $timeout
 set timeout [expr "$timeout + 60"]
@@ -88,7 +88,7 @@ gdb_test continue \
 "Opening nonexistant file returns ENOENT"
 
 send_gdb "continue\n" ; gdb_expect -re "$gdb_prompt $"
-catch "system \"chmod -f -w nowrt.fileio.test\""
+catch "system \"chmod -f -w fileio.dir/nowrt.fileio.test\""
 
 gdb_test continue \
 "Continuing\\..*open 5:.*EACCES$stop_msg" \
@@ -252,8 +252,8 @@ gdb_test continue \
 send_gdb "quit\n"
 send_gdb "y\n"
 
-remote_exec build {sh -xc test\ -r\ dir2.fileio.test\ &&\ chmod\ -f\ +w\ dir2.fileio.test}
-remote_exec build {sh -xc rm\ -rf\ *.fileio.test}
+remote_exec build {sh -xc test\ -r\ fileio.dir/dir2.fileio.test\ &&\ chmod\ -f\ +w\ fileio.dir/dir2.fileio.test}
+remote_exec build {sh -xc rm\ -rf\ fileio.dir}
 
 set timeout $oldtimeout
 return 0

Re: [patch] fileio.exp FAILs if run as root

[Mark Kettenis]

> Date: Sat, 8 Dec 2007 19:14:22 +0100
> From: Jan Kratochvil <jan.kratochvil@redhat.com>
> 
> Hi,
> 
> if you run gdb.base/fileio.exp as UID 0 it will print:
> 	FAIL: gdb.base/fileio.exp: Open for write but no write permission returns EACCES
> 	FAIL: gdb.base/fileio.exp: Unlinking a file in a directory w/o write access returns EACCES

People running the testsuite as root deserve what they get.  I don't
think we should complicate our code to make that possible, especially
if it involves calling setuid() which is notoriously unportable.

> 2007-12-08  Jan Kratochvil  <jan.kratochvil@redhat.com>
> 
> 	* gdb.base/fileio.c (ROOTSUBDIR): New macro.
> 	(main): CHDIR into ROOTSUBDIR.  CHOWN ROOTSUBDIR and CHDIR into
> 	ROOTSUBDIR if we are being run as root.
> 	* gdb.base/fileio.exp: Change the startup and finish cleanup.
> 	Change the test file reference to be into the `fileio.dir' directory.

Re: [patch] fileio.exp FAILs if run as root

[Jan Kratochvil]

[-- Attachment #1: Type: text/plain, Size: 692 bytes --]

On Sat, 08 Dec 2007 19:58:05 +0100, Mark Kettenis wrote:
> > Date: Sat, 8 Dec 2007 19:14:22 +0100
> > From: Jan Kratochvil <jan.kratochvil@redhat.com>
> > 
> > Hi,
> > 
> > if you run gdb.base/fileio.exp as UID 0 it will print:
> > 	FAIL: gdb.base/fileio.exp: Open for write but no write permission returns EACCES
> > 	FAIL: gdb.base/fileio.exp: Unlinking a file in a directory w/o write access returns EACCES
> 
> People running the testsuite as root deserve what they get.  I don't
> think we should complicate our code to make that possible, especially
> if it involves calling setuid() which is notoriously unportable.

OK, this is the other possibility I was considering.


Regards,
Jan

[-- Attachment #2: gdb-run-root.patch --]
[-- Type: text/plain, Size: 500 bytes --]

2007-12-08  Jan Kratochvil  <jan.kratochvil@redhat.com>

	* lib/gdb.exp: Refuse to run as root.

--- ./gdb/testsuite/lib/gdb.exp	30 Oct 2007 19:23:18 -0000	1.92
+++ ./gdb/testsuite/lib/gdb.exp	8 Dec 2007 19:12:00 -0000
@@ -26,6 +26,14 @@ if {$tool == ""} {
     exit 2
 }
 
+set uidfile [open "|id -u" r];
+gets $uidfile uid
+catch {close $uidfile}
+if {$uid == 0} {
+    send_error "Root privileges give false results, run as a regular user!\n"
+    exit 2
+}
+
 load_lib libgloss.exp
 
 global GDB

Re: [patch] fileio.exp FAILs if run as root

[Michael Snyder]

On Sat, 2007-12-08 at 20:13 +0100, Jan Kratochvil wrote:
> On Sat, 08 Dec 2007 19:58:05 +0100, Mark Kettenis wrote:
> > > Date: Sat, 8 Dec 2007 19:14:22 +0100
> > > From: Jan Kratochvil <jan.kratochvil@redhat.com>
> > > 
> > > Hi,
> > > 
> > > if you run gdb.base/fileio.exp as UID 0 it will print:
> > > 	FAIL: gdb.base/fileio.exp: Open for write but no write permission returns EACCES
> > > 	FAIL: gdb.base/fileio.exp: Unlinking a file in a directory w/o write access returns EACCES
> > 
> > People running the testsuite as root deserve what they get.  I don't
> > think we should complicate our code to make that possible, especially
> > if it involves calling setuid() which is notoriously unportable.
> 
> OK, this is the other possibility I was considering.

Clever -- but should it be a warning instead of a terminate?

Somebody might have a legitimate reason to run as root, 
even if we can't think of it right now.


> plain text document attachment (gdb-run-root.patch)
> 2007-12-08  Jan Kratochvil  <jan.kratochvil@redhat.com>
> 
> 	* lib/gdb.exp: Refuse to run as root.
> 
> --- ./gdb/testsuite/lib/gdb.exp	30 Oct 2007 19:23:18 -0000	1.92
> +++ ./gdb/testsuite/lib/gdb.exp	8 Dec 2007 19:12:00 -0000
> @@ -26,6 +26,14 @@ if {$tool == ""} {
>      exit 2
>  }
>  
> +set uidfile [open "|id -u" r];
> +gets $uidfile uid
> +catch {close $uidfile}
> +if {$uid == 0} {
> +    send_error "Root privileges give false results, run as a regular user!\n"
> +    exit 2
> +}
> +
>  load_lib libgloss.exp
>  
>  global GDB

Re: [patch] fileio.exp FAILs if run as root

[Jan Kratochvil]

On Tue, 11 Dec 2007 18:13:32 +0100, Michael Snyder wrote:
> On Sat, 2007-12-08 at 20:13 +0100, Jan Kratochvil wrote:
> > On Sat, 08 Dec 2007 19:58:05 +0100, Mark Kettenis wrote:
> > > > Date: Sat, 8 Dec 2007 19:14:22 +0100
> > > > From: Jan Kratochvil <jan.kratochvil@redhat.com>
> > > > 
> > > > Hi,
> > > > 
> > > > if you run gdb.base/fileio.exp as UID 0 it will print:
> > > > 	FAIL: gdb.base/fileio.exp: Open for write but no write permission returns EACCES
> > > > 	FAIL: gdb.base/fileio.exp: Unlinking a file in a directory w/o write access returns EACCES
> > > 
> > > People running the testsuite as root deserve what they get.  I don't
> > > think we should complicate our code to make that possible, especially
> > > if it involves calling setuid() which is notoriously unportable.
> > 
> > OK, this is the other possibility I was considering.
> 
> Clever -- but should it be a warning instead of a terminate?
> 
> Somebody might have a legitimate reason to run as root, 
> even if we can't think of it right now.

This my patch
	http://sources.redhat.com/ml/gdb-patches/2007-12/msg00137.html

was more a demonstration that while running the testsuite as 'root' may be
wrong it happens and it probably should not be disabled.


This is repeating the history, warnings are not enough:
	http://sources.redhat.com/ml/gdb-patches/2007-01/threads.html#00326

There are various warnings around, at least I see now on the console
	WARNING: Couldn't find the global config file.

but apparently nobody cares (do you also see this warning?).  I had to start
resolving the testsuite regression to find out the reason is the 'root' run.
I am sure I would ignore another warning message appearing during the start.

There are multiple possibilities:

(1) Keeping there setuid() before somebody complains it does not work.
    http://sources.redhat.com/ml/gdb-patches/2007-12/msg00135.html

(2) Pointing me/anyone at the system where this setuid() code does not work.

(3) Skipping (not FAILing) the two tests known they FAIL on the 'root' run.

(4) Disabling running the testsuite as 'root'.

(5) Giving just a warning during the start.

(6) I am too dumb to become a GDB user.

more?

(4) and (5) are hopefully out of the question now.


Regards,
Jan

Re: [patch] fileio.exp FAILs if run as root

[Daniel Jacobowitz]

On Tue, Dec 11, 2007 at 06:46:26PM +0100, Jan Kratochvil wrote:
> There are various warnings around, at least I see now on the console
> 	WARNING: Couldn't find the global config file.
> 
> but apparently nobody cares (do you also see this warning?).  I had to start

It comes from DejaGNU.  There's nothing much we can do about it; set
DEJAGNU in your environment to point to a local site.exp, and it will
go away.

> There are multiple possibilities:
> 
> (1) Keeping there setuid() before somebody complains it does not work.
>     http://sources.redhat.com/ml/gdb-patches/2007-12/msg00135.html
> 
> (2) Pointing me/anyone at the system where this setuid() code does not work.

It will not work on any of the systems the testcase was designed for,
i.e. those which use the remote File I/O protocol for file operations.

> (3) Skipping (not FAILing) the two tests known they FAIL on the 'root' run.
> 
> (4) Disabling running the testsuite as 'root'.
> 
> (5) Giving just a warning during the start.
> 
> (6) I am too dumb to become a GDB user.
> 
> more?

Just ignoring the two failures if you need to run as root for some
reason, or xfailing them locally, IMO.  I think a warning is quite
sufficient.  Perhaps in the FAIL message.

-- 
Daniel Jacobowitz
CodeSourcery

Re: [patch] fileio.exp FAILs if run as root

[Jan Kratochvil]

[-- Attachment #1: Type: text/plain, Size: 232 bytes --]

On Tue, 11 Dec 2007 18:54:23 +0100, Daniel Jacobowitz wrote:
...
> I think a warning is quite sufficient.  Perhaps in the FAIL message.

Attached.

Still there is an XFAIL for Cygwin so it IMO could be even for UNIX.


Regards,
Jan

[-- Attachment #2: gdb-cvs-test-fileio-root-run2.patch --]
[-- Type: text/plain, Size: 1358 bytes --]

2007-12-11  Jan Kratochvil  <jan.kratochvil@redhat.com>

	* gdb.base/fileio.exp: Add the testcase purpose comment at the top.
	Embed a warning into the name of two testcases known to fail if we run
	as root.

--- ./gdb/testsuite/gdb.base/fileio.exp	23 Aug 2007 18:14:16 -0000	1.12
+++ ./gdb/testsuite/gdb.base/fileio.exp	11 Dec 2007 18:22:44 -0000
@@ -17,6 +17,8 @@
 # bug-gdb@prep.ai.mit.edu
 
 # This file was written by Corinna Vinschen <vinschen@redhat.com>
+# Its main purpose is to test the GDB implementation of the remote file I/O
+# protocol.
 
 if [target_info exists gdb,nofileio] {
     verbose "Skipping fileio.exp because of no fileio capabilities."
@@ -92,7 +94,7 @@ catch "system \"chmod -f -w nowrt.fileio
 
 gdb_test continue \
 "Continuing\\..*open 5:.*EACCES$stop_msg" \
-"Open for write but no write permission returns EACCES"
+"Open for write but no write permission returns EACCES (it also fails if running as root)"
 
 gdb_test continue \
 "Continuing\\..*write 1:.*OK$stop_msg" \
@@ -234,7 +236,7 @@ if [ishost *cygwin*] {
 }
 gdb_test continue \
 "Continuing\\..*unlink 2:.*EACCES$stop_msg" \
-"Unlinking a file in a directory w/o write access returns EACCES"
+"Unlinking a file in a directory w/o write access returns EACCES (it also fails if running as root)"
 
 gdb_test continue \
 "Continuing\\..*unlink 3:.*ENOENT$stop_msg" \