public inbox for ltp@lists.linux.it
 help / color / mirror / Atom feed
* [LTP] [PATCH ltp] make filecaps tests succeed
@ 2010-04-28 22:47 Serge E. Hallyn
  2010-04-28 23:02 ` Garrett Cooper
  0 siblings, 1 reply; 3+ messages in thread
From: Serge E. Hallyn @ 2010-04-28 22:47 UTC (permalink / raw)
  To: LTP list

Most of these are belated cleanup after the move to using /opt/ltp.
Also undoing an ill-advised replacement of return with tst_exit.  All
filecaps tests now succeed on fedora 10.

Signed-off-by: Serge E. Hallyn <serue@us.ibm.com>
---
 testcases/kernel/security/filecaps/filecapstest.sh |    9 ++++++---
 testcases/kernel/security/filecaps/print_caps.c    |    3 ++-
 .../kernel/security/filecaps/verify_caps_exec.c    |   19 ++++++++++++++-----
 3 files changed, 22 insertions(+), 9 deletions(-)

diff --git a/testcases/kernel/security/filecaps/filecapstest.sh b/testcases/kernel/security/filecaps/filecapstest.sh
index 43582dc..6864de4 100755
--- a/testcases/kernel/security/filecaps/filecapstest.sh
+++ b/testcases/kernel/security/filecaps/filecapstest.sh
@@ -22,8 +22,11 @@
 echo "Running in:"
 #rm -f print_caps
 #cp $LTPROOT/testcases/bin/print_caps .
-mkfifo caps_fifo
-chmod 777 caps_fifo
+#FIFOFILE="$LTPROOT/testcases/bin/caps_fifo"
+FIFOFILE="/tmp/caps_fifo"
+rm -f $FIFOFILE
+mkfifo $FIFOFILE
+chmod 777 $FIFOFILE
 exit_code=0
 echo "cap_sys_admin tests"
 verify_caps_exec 0
@@ -46,5 +49,5 @@ if [ $tmp -ne 0 ]; then
 	exit_code=$tmp
 fi
 
-unlink caps_fifo
+unlink $FIFOFILE
 exit $exit_code
diff --git a/testcases/kernel/security/filecaps/print_caps.c b/testcases/kernel/security/filecaps/print_caps.c
index f0e9bce..b887738 100644
--- a/testcases/kernel/security/filecaps/print_caps.c
+++ b/testcases/kernel/security/filecaps/print_caps.c
@@ -36,7 +36,7 @@
 #include <sys/capability.h>
 #endif
 
-#define FIFOFILE "caps_fifo"
+#define FIFOFILE "/tmp/caps_fifo"
 
 int main(int argc, char *argv[])
 {
@@ -68,4 +68,5 @@ int main(int argc, char *argv[])
 #else
 	return 0;
 #endif
+	return 0;
 }
diff --git a/testcases/kernel/security/filecaps/verify_caps_exec.c b/testcases/kernel/security/filecaps/verify_caps_exec.c
index 5250007..7360d4a 100644
--- a/testcases/kernel/security/filecaps/verify_caps_exec.c
+++ b/testcases/kernel/security/filecaps/verify_caps_exec.c
@@ -43,7 +43,7 @@
 #include <sys/prctl.h>
 #include <test.h>
 
-#define TSTPATH "./print_caps"
+#define TSTPATH "print_caps"
 char *TCID = "filecaps";
 int TST_TOTAL=1;
 
@@ -70,7 +70,7 @@ void print_my_caps()
 	cap_free(txt);
 }
 
-int drop_root(int keep_perms)
+void drop_root(int keep_perms)
 {
 	int ret;
 
@@ -84,10 +84,19 @@ int drop_root(int keep_perms)
 	}
 	if (keep_perms) {
 		cap_t cap = cap_from_text("=eip");
-		cap_set_proc(cap);
+		int ret;
+		if (!cap) {
+			tst_resm(TBROK, "cap_from_text failed\n");
+			tst_exit();
+		}
+		ret = cap_set_proc(cap);
+		if (ret < 0) {
+			perror(cap_set_proc);
+			tst_resm(TBROK | TERRNO, "cap_set_proc failed\n");
+			tst_exit();
+		}
 		cap_free(cap);
 	}
-	tst_exit();
 }
 
 int perms_test(void)
@@ -114,7 +123,7 @@ int perms_test(void)
 	return ret;
 }
 
-#define FIFOFILE "caps_fifo"
+#define FIFOFILE "/tmp/caps_fifo"
 void create_fifo(void)
 {
 	int ret;
-- 
1.6.0.6


------------------------------------------------------------------------------
_______________________________________________
Ltp-list mailing list
Ltp-list@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/ltp-list

^ permalink raw reply related	[flat|nested] 3+ messages in thread

* Re: [LTP] [PATCH ltp] make filecaps tests succeed
  2010-04-28 22:47 [LTP] [PATCH ltp] make filecaps tests succeed Serge E. Hallyn
@ 2010-04-28 23:02 ` Garrett Cooper
  2010-04-29  0:25   ` Serge E. Hallyn
  0 siblings, 1 reply; 3+ messages in thread
From: Garrett Cooper @ 2010-04-28 23:02 UTC (permalink / raw)
  To: Serge E. Hallyn; +Cc: LTP list

On Wed, Apr 28, 2010 at 3:47 PM, Serge E. Hallyn <serue@us.ibm.com> wrote:
> Most of these are belated cleanup after the move to using /opt/ltp.
> Also undoing an ill-advised replacement of return with tst_exit.  All
> filecaps tests now succeed on fedora 10.
>
> Signed-off-by: Serge E. Hallyn <serue@us.ibm.com>
> ---
>  testcases/kernel/security/filecaps/filecapstest.sh |    9 ++++++---
>  testcases/kernel/security/filecaps/print_caps.c    |    3 ++-
>  .../kernel/security/filecaps/verify_caps_exec.c    |   19 ++++++++++++++-----
>  3 files changed, 22 insertions(+), 9 deletions(-)
>
> diff --git a/testcases/kernel/security/filecaps/filecapstest.sh b/testcases/kernel/security/filecaps/filecapstest.sh
> index 43582dc..6864de4 100755
> --- a/testcases/kernel/security/filecaps/filecapstest.sh
> +++ b/testcases/kernel/security/filecaps/filecapstest.sh
> @@ -22,8 +22,11 @@
>  echo "Running in:"
>  #rm -f print_caps
>  #cp $LTPROOT/testcases/bin/print_caps .
> -mkfifo caps_fifo
> -chmod 777 caps_fifo
> +#FIFOFILE="$LTPROOT/testcases/bin/caps_fifo"
> +FIFOFILE="/tmp/caps_fifo"

Why not

TMP=${TMP:=/tmp}
$TMP/caps_fifo

etc? FWIW if you're in $TMP already, a lot of these changes aren't
required, correct?

> +rm -f $FIFOFILE
> +mkfifo $FIFOFILE
> +chmod 777 $FIFOFILE
>  exit_code=0
>  echo "cap_sys_admin tests"
>  verify_caps_exec 0
> @@ -46,5 +49,5 @@ if [ $tmp -ne 0 ]; then
>        exit_code=$tmp
>  fi
>
> -unlink caps_fifo
> +unlink $FIFOFILE
>  exit $exit_code
> diff --git a/testcases/kernel/security/filecaps/print_caps.c b/testcases/kernel/security/filecaps/print_caps.c
> index f0e9bce..b887738 100644
> --- a/testcases/kernel/security/filecaps/print_caps.c
> +++ b/testcases/kernel/security/filecaps/print_caps.c
> @@ -36,7 +36,7 @@
>  #include <sys/capability.h>
>  #endif
>
> -#define FIFOFILE "caps_fifo"
> +#define FIFOFILE "/tmp/caps_fifo"
>
>  int main(int argc, char *argv[])
>  {
> @@ -68,4 +68,5 @@ int main(int argc, char *argv[])
>  #else
>        return 0;
>  #endif
> +       return 0;

What's the value returned for #if ..? If it's `return 0', then why not
just remove the other two references in the preprocessor blocks?

>  }
> diff --git a/testcases/kernel/security/filecaps/verify_caps_exec.c b/testcases/kernel/security/filecaps/verify_caps_exec.c
> index 5250007..7360d4a 100644
> --- a/testcases/kernel/security/filecaps/verify_caps_exec.c
> +++ b/testcases/kernel/security/filecaps/verify_caps_exec.c
> @@ -43,7 +43,7 @@
>  #include <sys/prctl.h>
>  #include <test.h>
>
> -#define TSTPATH "./print_caps"
> +#define TSTPATH "print_caps"

Ok.

>  char *TCID = "filecaps";
>  int TST_TOTAL=1;
>
> @@ -70,7 +70,7 @@ void print_my_caps()
>        cap_free(txt);
>  }
>
> -int drop_root(int keep_perms)
> +void drop_root(int keep_perms)
>  {
>        int ret;
>
> @@ -84,10 +84,19 @@ int drop_root(int keep_perms)
>        }
>        if (keep_perms) {
>                cap_t cap = cap_from_text("=eip");
> -               cap_set_proc(cap);
> +               int ret;
> +               if (!cap) {
> +                       tst_resm(TBROK, "cap_from_text failed\n");
> +                       tst_exit();
> +               }
> +               ret = cap_set_proc(cap);
> +               if (ret < 0) {
> +                       perror(cap_set_proc);

Why??? This could potentially fubar errno too...

> +                       tst_resm(TBROK | TERRNO, "cap_set_proc failed\n");
> +                       tst_exit();

tst_brkm(TBROK | TERRNO, tst_exit, "cap_set..."); is better

> +               }
>                cap_free(cap);
>        }
> -       tst_exit();
>  }
>
>  int perms_test(void)
> @@ -114,7 +123,7 @@ int perms_test(void)
>        return ret;
>  }
>
> -#define FIFOFILE "caps_fifo"
> +#define FIFOFILE "/tmp/caps_fifo"
>  void create_fifo(void)
>  {
>        int ret;
> --
> 1.6.0.6

------------------------------------------------------------------------------
_______________________________________________
Ltp-list mailing list
Ltp-list@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/ltp-list

^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [LTP] [PATCH ltp] make filecaps tests succeed
  2010-04-28 23:02 ` Garrett Cooper
@ 2010-04-29  0:25   ` Serge E. Hallyn
  0 siblings, 0 replies; 3+ messages in thread
From: Serge E. Hallyn @ 2010-04-29  0:25 UTC (permalink / raw)
  To: Garrett Cooper; +Cc: LTP list

Quoting Garrett Cooper (yanegomi@gmail.com):
> On Wed, Apr 28, 2010 at 3:47 PM, Serge E. Hallyn <serue@us.ibm.com> wrote:
> > Most of these are belated cleanup after the move to using /opt/ltp.
> > Also undoing an ill-advised replacement of return with tst_exit.  All
> > filecaps tests now succeed on fedora 10.
> >
> > Signed-off-by: Serge E. Hallyn <serue@us.ibm.com>
> > ---
> >  testcases/kernel/security/filecaps/filecapstest.sh |    9 ++++++---
> >  testcases/kernel/security/filecaps/print_caps.c    |    3 ++-
> >  .../kernel/security/filecaps/verify_caps_exec.c    |   19 ++++++++++++++-----
> >  3 files changed, 22 insertions(+), 9 deletions(-)
> >
> > diff --git a/testcases/kernel/security/filecaps/filecapstest.sh b/testcases/kernel/security/filecaps/filecapstest.sh
> > index 43582dc..6864de4 100755
> > --- a/testcases/kernel/security/filecaps/filecapstest.sh
> > +++ b/testcases/kernel/security/filecaps/filecapstest.sh
> > @@ -22,8 +22,11 @@
> >  echo "Running in:"
> >  #rm -f print_caps
> >  #cp $LTPROOT/testcases/bin/print_caps .
> > -mkfifo caps_fifo
> > -chmod 777 caps_fifo
> > +#FIFOFILE="$LTPROOT/testcases/bin/caps_fifo"
> > +FIFOFILE="/tmp/caps_fifo"
> 
> Why not
> 
> TMP=${TMP:=/tmp}
> $TMP/caps_fifo
> 
> etc? FWIW if you're in $TMP already, a lot of these changes aren't
> required, correct?

No objection - though then the definition of /tmp/caps_fifo below
needs to be changed as well.

> > +rm -f $FIFOFILE
> > +mkfifo $FIFOFILE
> > +chmod 777 $FIFOFILE
> >  exit_code=0
> >  echo "cap_sys_admin tests"
> >  verify_caps_exec 0
> > @@ -46,5 +49,5 @@ if [ $tmp -ne 0 ]; then
> >        exit_code=$tmp
> >  fi
> >
> > -unlink caps_fifo
> > +unlink $FIFOFILE
> >  exit $exit_code
> > diff --git a/testcases/kernel/security/filecaps/print_caps.c b/testcases/kernel/security/filecaps/print_caps.c
> > index f0e9bce..b887738 100644
> > --- a/testcases/kernel/security/filecaps/print_caps.c
> > +++ b/testcases/kernel/security/filecaps/print_caps.c
> > @@ -36,7 +36,7 @@
> >  #include <sys/capability.h>
> >  #endif
> >
> > -#define FIFOFILE "caps_fifo"
> > +#define FIFOFILE "/tmp/caps_fifo"
> >
> >  int main(int argc, char *argv[])
> >  {
> > @@ -68,4 +68,5 @@ int main(int argc, char *argv[])
> >  #else
> >        return 0;
> >  #endif
> > +       return 0;
> 
> What's the value returned for #if ..? If it's `return 0', then why not
> just remove the other two references in the preprocessor blocks?

There was no return in that case.  So really we can just get rid of
the #else altogether.

> >  }
> > diff --git a/testcases/kernel/security/filecaps/verify_caps_exec.c b/testcases/kernel/security/filecaps/verify_caps_exec.c
> > index 5250007..7360d4a 100644
> > --- a/testcases/kernel/security/filecaps/verify_caps_exec.c
> > +++ b/testcases/kernel/security/filecaps/verify_caps_exec.c
> > @@ -43,7 +43,7 @@
> >  #include <sys/prctl.h>
> >  #include <test.h>
> >
> > -#define TSTPATH "./print_caps"
> > +#define TSTPATH "print_caps"
> 
> Ok.
> 
> >  char *TCID = "filecaps";
> >  int TST_TOTAL=1;
> >
> > @@ -70,7 +70,7 @@ void print_my_caps()
> >        cap_free(txt);
> >  }
> >
> > -int drop_root(int keep_perms)
> > +void drop_root(int keep_perms)
> >  {
> >        int ret;
> >
> > @@ -84,10 +84,19 @@ int drop_root(int keep_perms)
> >        }
> >        if (keep_perms) {
> >                cap_t cap = cap_from_text("=eip");
> > -               cap_set_proc(cap);
> > +               int ret;
> > +               if (!cap) {
> > +                       tst_resm(TBROK, "cap_from_text failed\n");
> > +                       tst_exit();
> > +               }
> > +               ret = cap_set_proc(cap);
> > +               if (ret < 0) {
> > +                       perror(cap_set_proc);
> 
> Why??? This could potentially fubar errno too...

The perror doesn't need to be there.

> > +                       tst_resm(TBROK | TERRNO, "cap_set_proc failed\n");
> > +                       tst_exit();
> 
> tst_brkm(TBROK | TERRNO, tst_exit, "cap_set..."); is better
> 
> > +               }
> >                cap_free(cap);
> >        }
> > -       tst_exit();
> >  }
> >
> >  int perms_test(void)
> > @@ -114,7 +123,7 @@ int perms_test(void)
> >        return ret;
> >  }
> >
> > -#define FIFOFILE "caps_fifo"
> > +#define FIFOFILE "/tmp/caps_fifo"
> >  void create_fifo(void)
> >  {
> >        int ret;
> > --
> > 1.6.0.6

------------------------------------------------------------------------------
_______________________________________________
Ltp-list mailing list
Ltp-list@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/ltp-list

^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2010-04-29  2:24 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2010-04-28 22:47 [LTP] [PATCH ltp] make filecaps tests succeed Serge E. Hallyn
2010-04-28 23:02 ` Garrett Cooper
2010-04-29  0:25   ` Serge E. Hallyn

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox