* [LTP] [PATCH] Fix FILECAPS test hanging for more than 12 hours
@ 2010-05-04 18:20 Subrata Modak
2010-05-04 19:22 ` Serge E. Hallyn
0 siblings, 1 reply; 9+ messages in thread
From: Subrata Modak @ 2010-05-04 18:20 UTC (permalink / raw)
To: ltp-list; +Cc: Serge Hallyn
[-- Attachment #1: Type: text/plain, Size: 12091 bytes --]
Hi,
Recently running FILECAPS test on my Fedora system:
# uname -a
Linux 2.6.33.1-19.fc13.ppc64 #1 SMP Tue Mar 23 06:32:38 EDT 2010 ppc64
ppc64 ppc64 GNU/Linux,
I found that the test hangs for more than 12 hours. The following patch
by Serge fixes the issue. Kindly include it inside LTP.
Tested-by: Subrata Modak <subrata@linux.vnet.ibm.com>,
Serge, please add a Sign-off.
Issue before the patch:
=============================
# ./runltp -f filecaps
<<<test_start>>>
tag=Filecaps stime=1271951563
cmdline="filecapstest.sh"
contacts=""
analysis=exit
<<<test_output>>>
Running in:
cap_sys_admin tests
testing for correct caps
...
The test hangs here for more than 12 hours.
Following are various info about the processes running this test:
# ps ajxf
1608 1724 1608 1458 ? -1 S 0 0:00 \_
/opt/ltp/bin/ltp-pan -e -S -a 1608 -n 1608 -p -f /tmp/ltp-71wskF3epE/alltests
-l /opt/ltp/results/LTP_RUN_ON-20
1724 30311 30311 1458 ? -1 S 0 0:00 \_ /bin/sh
/opt/ltp/testcases/bin/filecapstest.sh
30311 30315 30311 1458 ? -1 S 0 0:00 \_
verify_caps_exec 1
30315 30316 30311 1458 ? -1 Z 1000 0:00 \_
[verify_caps_exe] <defunct>
STRACE on the PIDs does not show anything:
[root@alien5 ltp]# strace -p 30425
Process 30425 attached - interrupt to quit
waitpid(-1, ^C <unfinished ...>
Process 30425 detached
[root@alien5 ltp]# strace -p 30429
Process 30429 attached - interrupt to quit
open("caps_fifo", O_RDONLY^C <unfinished ...>
Process 30429 detached
[root@alien5 ltp]# strace -p 30430
attach: ptrace(PTRACE_ATTACH, ...): Operation not permitted
# getenforce
Permissive
[root@alien5 ltp]# tail -f /var/log/messages
2010-04-21T18:00:15.752320+05:18 alien5 setroubleshoot: SELinux is preventing
/sbin/rsyslogd access to a leaked
/opt/ltp/output/LTP_RUN_ON-2010_Apr_21-17h_51m_22s.output file descriptor. For
complete SELinux messages. run sealert -l 894e0d2d-23c3-45d1-9108-71ad97f5a45e
2010-04-21T18:00:15.794214+05:18 alien5 setroubleshoot: SELinux is preventing
/sbin/rsyslogd access to a leaked
/opt/ltp/output/LTP_RUN_ON-2010_Apr_21-17h_51m_22s.output file descriptor. For
complete SELinux messages. run sealert -l 894e0d2d-23c3-45d1-9108-71ad97f5a45e
2010-04-21T18:00:15.823557+05:18 alien5 setroubleshoot: SELinux is preventing
/sbin/rsyslogd access to a leaked
/opt/ltp/output/LTP_RUN_ON-2010_Apr_21-17h_51m_22s.output file descriptor. For
complete SELinux messages. run sealert -l 894e0d2d-23c3-45d1-9108-71ad97f5a45e
2010-04-21T18:00:17.721361+05:18 alien5 syslogtst: syslogtst:10 error level is
logged
Apr 21 18:00:19 alien5 kernel: imklog 4.4.2, log source = /proc/kmsg started.
Apr 21 18:00:19 alien5 rsyslogd: [origin software="rsyslogd" swVersion="4.4.2"
x-pid="2165" x-info="http://www.rsyslog.com"] (re)start
Apr 21 18:00:20 alien5 setroubleshoot: SELinux is preventing /sbin/rsyslogd
access to a leaked /opt/ltp/output/LTP_RUN_ON-2010_Apr_21-17h_51m_22s.output
file descriptor. For complete SELinux messages. run sealert -l
894e0d2d-23c3-45d1-9108-71ad97f5a45e
Apr 21 18:00:20 alien5 setroubleshoot: SELinux is preventing /sbin/rsyslogd
access to a leaked /opt/ltp/output/LTP_RUN_ON-2010_Apr_21-17h_51m_22s.output
file descriptor. For complete SELinux messages. run sealert -l
894e0d2d-23c3-45d1-9108-71ad97f5a45e
Apr 21 18:00:20 alien5 setroubleshoot: SELinux is preventing /sbin/rsyslogd
access to a leaked /opt/ltp/output/LTP_RUN_ON-2010_Apr_21-17h_51m_22s.output
file descriptor. For complete SELinux messages. run sealert -l
894e0d2d-23c3-45d1-9108-71ad97f5a45e
So, i executed the following command:
# sealert -l 894e0d2d-23c3-45d1-9108-71ad97f5a45e
exception when creating syslog handler: [Errno 2] No such file or directory
Summary:
SELinux is preventing /sbin/rsyslogd access to a leaked
/opt/ltp/output/LTP_RUN_ON-2010_Apr_21-17h_51m_22s.output file descriptor.
Detailed Description:
[rsyslogd has a permissive type (syslogd_t). This access was not denied.]
SELinux denied access requested by the rsyslogd command. It looks like this is
either a leaked descriptor or rsyslogd output was redirected to a file it is
not
allowed to access. Leaks usually can be ignored since SELinux is just closing
the leak and reporting the error. The application does not use the descriptor,
so it will run properly. If this is a redirection, you will not get output in
the /opt/ltp/output/LTP_RUN_ON-2010_Apr_21-17h_51m_22s.output. You should
generate a bugzilla on selinux-policy, and it will get routed to the
appropriate
package. You can safely ignore this avc.
Allowing Access:
You can generate a local policy module to allow this access - see FAQ
(http://docs.fedoraproject.org/selinux-faq-fc5/#id2961385)
Additional Information:
Source Context unconfined_u:system_r:syslogd_t:s0
Target Context unconfined_u:object_r:usr_t:s0
Target Objects /opt/ltp/output/LTP_RUN_ON-
2010_Apr_21-17h_51m_22s.output [ file ]
Source rsyslogd
Source Path /sbin/rsyslogd
Port <Unknown>
Host alien5.ltc.austin.ibm.com
Source RPM Packages rsyslog-4.4.2-6.fc13
Target RPM Packages
Policy RPM selinux-policy-3.7.15-4.fc13
Selinux Enabled True
Policy Type targeted
Enforcing Mode Enforcing
Plugin Name leaks
Host Name alien5.ltc.austin.ibm.com
Platform Linux alien5.ltc.austin.ibm.com
2.6.33.1-19.fc13.ppc64 #1 SMP Tue Mar 23 06:32:38
EDT 2010 ppc64 ppc64
Alert Count 186
First Seen Tue Apr 20 23:55:40 2010
Last Seen Wed Apr 21 18:00:19 2010
Local ID 894e0d2d-23c3-45d1-9108-71ad97f5a45e
Line Numbers
Raw Audit Messages
node= type=AVC msg=audit(1271853019.957:317): avc:
denied { append } for pid=2164 comm="rsyslogd"
path="/opt/ltp/output/LTP_RUN_ON-2010_Apr_21-17h_51m_22s.output" dev=sda3
ino=1188363 scontext=unconfined_u:system_r:syslogd_t:s0
tcontext=unconfined_u:object_r:usr_t:s0 tclass=file
node= type=AVC msg=audit(1271853019.957:317): avc:
denied { append } for pid=2164 comm="rsyslogd"
path="/opt/ltp/results/LTP_RUN_ON-2010_Apr_21-17h_51m_22s.log" dev=sda3
ino=1188362 scontext=unconfined_u:system_r:syslogd_t:s0
tcontext=unconfined_u:object_r:usr_t:s0 tclass=file
node= type=AVC msg=audit(1271853019.957:317): avc:
denied { append } for pid=2164 comm="rsyslogd"
path="/opt/ltp/output/LTP_RUN_ON-2010_Apr_21-17h_51m_22s.failed" dev=sda3
ino=1188364 scontext=unconfined_u:system_r:syslogd_t:s0
tcontext=unconfined_u:object_r:usr_t:s0 tclass=file
node= type=SYSCALL msg=audit(1271853019.957:317):
arch=14 syscall=11 success=yes exit=0 a0=1026c900 a1=1026b5b0 a2=1026b640
a3=1026b5a8 items=0 ppid=2163 pid=2164 auid=0 uid=0 gid=0 euid=0 suid=0 fsuid=0
egid=0 sgid=0 fsgid=0 tty=(none) ses=22 comm="rsyslogd" exe="/sbin/rsyslogd"
subj=unconfined_u:system_r:syslogd_t:s0 key=(null)
=============================
=============================
Test result after applying the patch
=============================
<<<test_start>>>
tag=Filecaps stime=1272996532
cmdline="filecapstest.sh"
contacts=""
analysis=exit
<<<test_output>>>
incrementing stop
Running in:
cap_sys_admin tests
filecaps 1 TPASS : could not set capabilities as non-root
testing for correct caps
filecaps 0 TINFO :
caps are =
filecaps 0 TINFO :
caps are =
filecaps 0 TINFO :
caps are =
filecaps 0 TINFO :
caps are =
filecaps 0 TINFO :
caps are =
filecaps 0 TINFO :
caps are =
filecaps 0 TINFO :
caps are =
filecaps 0 TINFO :
caps are =
filecaps 0 TINFO :
caps are =
filecaps 0 TINFO :
caps are =
filecaps 0 TINFO :
caps are =
filecaps 0 TINFO :
caps are =
filecaps 0 TINFO :
caps are =
filecaps 0 TINFO :
caps are =
filecaps 0 TINFO :
caps are =
filecaps 0 TINFO :
caps are =
filecaps 0 TINFO :
caps are =
filecaps 0 TINFO :
caps are =
filecaps 0 TINFO :
caps are =
filecaps 0 TINFO :
caps are =
filecaps 0 TINFO :
caps are =
filecaps 0 TINFO :
caps are =
filecaps 0 TINFO :
caps are =
filecaps 0 TINFO :
caps are =
filecaps 0 TINFO :
caps are =
filecaps 0 TINFO :
caps are =
filecaps 0 TINFO :
caps are =
filecaps 0 TINFO :
caps are =
filecaps 0 TINFO :
caps are =
filecaps 0 TINFO :
caps are =
filecaps 0 TINFO :
caps are =
filecaps 0 TINFO :
caps are =
filecaps 0 TINFO :
caps are =
filecaps 0 TINFO :
caps are =
filecaps 0 TINFO :
caps are =
filecaps 0 TINFO :
caps are =
filecaps 0 TINFO :
caps are =
filecaps 0 TINFO :
caps are =
filecaps 0 TINFO :
caps are =
filecaps 0 TINFO :
caps are =
filecaps 0 TINFO :
caps are =
filecaps 0 TINFO :
caps are =
filecaps 0 TINFO :
caps are =
filecaps 0 TINFO :
caps are =
filecaps 0 TINFO :
caps are =
filecaps 0 TINFO :
caps are =
filecaps 0 TINFO :
caps are =
filecaps 0 TINFO :
caps are =
filecaps 0 TINFO :
caps are =
filecaps 0 TINFO :
caps are =
filecaps 0 TINFO :
caps are =
filecaps 0 TINFO :
caps are =
filecaps 0 TINFO :
caps are =
filecaps 0 TINFO :
caps are =
filecaps 0 TINFO :
caps are =
filecaps 0 TINFO :
caps are =
filecaps 0 TINFO :
caps are =
filecaps 0 TINFO :
caps are =
filecaps 0 TINFO :
caps are =
filecaps 0 TINFO :
caps are =
filecaps 0 TINFO :
caps are =
filecaps 0 TINFO :
caps are =
filecaps 0 TINFO :
caps are =
filecaps 0 TINFO :
caps are =
filecaps 0 TINFO :
caps are =
filecaps 0 TINFO :
caps are =
filecaps 0 TINFO :
caps are =
filecaps 0 TINFO :
caps are =
filecaps 0 TINFO : 0
filecaps 0 TINFO : 1
filecaps 0 TINFO : 2
filecaps 0 TINFO : 3
filecaps 0 TINFO : 4
filecaps 0 TINFO : 5
filecaps 0 TINFO : 6
filecaps 0 TINFO : 7
filecaps 0 TINFO : 8
filecaps 0 TINFO : 9
filecaps 0 TINFO : 10
filecaps 0 TINFO : 11
filecaps 0 TINFO : 12
filecaps 0 TINFO : 13
filecaps 0 TINFO : 14
filecaps 0 TINFO : 15
filecaps 0 TINFO : 16
filecaps 0 TINFO : 17
filecaps 0 TINFO : 18
filecaps 0 TINFO : 19
filecaps 0 TINFO : 20
filecaps 0 TINFO : 21
filecaps 0 TINFO : 22
filecaps 0 TINFO : 23
filecaps 0 TINFO : 24
filecaps 0 TINFO : 25
filecaps 0 TINFO : 26
filecaps 0 TINFO : 27
filecaps 0 TINFO : 28
filecaps 0 TINFO : 29
filecaps 0 TINFO : 30
filecaps 0 TINFO : 31
filecaps 0 TINFO : 32
filecaps 0 TINFO : 33
filecaps 1 TPASS : All tests passed
testing for correct pI checks
filecaps 0 TINFO : start
filecaps 0 TINFO : =ep
filecaps 0 TINFO : after raising all caps
filecaps 0 TINFO : =eip
filecaps 0 TINFO : after first drop cap_sys_admin
filecaps 0 TINFO : =eip cap_sys_admin-eip
filecaps 0 TINFO : after first raise cap_sys_admin
filecaps 0 TINFO : =eip cap_sys_admin-ep
filecaps 0 TINFO : after drop cappset
filecaps 0 TINFO : =ip cap_sys_admin-p
filecaps 0 TINFO : after second drop cap_sys_admin
filecaps 0 TINFO : =eip cap_setpcap-e cap_sys_admin-eip
filecaps 0 TINFO : final
filecaps 0 TINFO : =eip cap_setpcap-e cap_sys_admin-eip
filecaps 1 TPASS : pI is properly capped
<<<execution_status>>>
initiation_status="ok"
duration=0 termination_type=exited termination_id=0 corefile=no
cutime=0 cstime=4
<<<test_end>>>
=============================
Regards--
Subrata
[-- Attachment #2: 0001-make-filecaps-tests-succeed.patch --]
[-- Type: application/mbox, Size: 3107 bytes --]
[-- Attachment #3: Type: text/plain, Size: 79 bytes --]
------------------------------------------------------------------------------
[-- Attachment #4: Type: text/plain, Size: 155 bytes --]
_______________________________________________
Ltp-list mailing list
Ltp-list@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/ltp-list
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [LTP] [PATCH] Fix FILECAPS test hanging for more than 12 hours
2010-05-04 18:20 [LTP] [PATCH] Fix FILECAPS test hanging for more than 12 hours Subrata Modak
@ 2010-05-04 19:22 ` Serge E. Hallyn
2010-05-04 21:02 ` Garrett Cooper
0 siblings, 1 reply; 9+ messages in thread
From: Serge E. Hallyn @ 2010-05-04 19:22 UTC (permalink / raw)
To: Subrata Modak; +Cc: ltp-list
Quoting Subrata Modak (subrata@linux.vnet.ibm.com):
> Serge, please add a Sign-off.
It's there in the patch in your attachment...
-serge
------------------------------------------------------------------------------
_______________________________________________
Ltp-list mailing list
Ltp-list@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/ltp-list
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [LTP] [PATCH] Fix FILECAPS test hanging for more than 12 hours
2010-05-04 19:22 ` Serge E. Hallyn
@ 2010-05-04 21:02 ` Garrett Cooper
2010-05-04 22:33 ` Serge E. Hallyn
0 siblings, 1 reply; 9+ messages in thread
From: Garrett Cooper @ 2010-05-04 21:02 UTC (permalink / raw)
To: Serge E. Hallyn; +Cc: ltp-list
On Tue, May 4, 2010 at 12:22 PM, Serge E. Hallyn <serue@us.ibm.com> wrote:
> Quoting Subrata Modak (subrata@linux.vnet.ibm.com):
>> Serge, please add a Sign-off.
>
> It's there in the patch in your attachment...
Serge,
Please address the items in the previous review and we'll get it committed.
Thanks,
-Garrett
------------------------------------------------------------------------------
_______________________________________________
Ltp-list mailing list
Ltp-list@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/ltp-list
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [LTP] [PATCH] Fix FILECAPS test hanging for more than 12 hours
2010-05-04 21:02 ` Garrett Cooper
@ 2010-05-04 22:33 ` Serge E. Hallyn
2010-05-05 5:19 ` Garrett Cooper
0 siblings, 1 reply; 9+ messages in thread
From: Serge E. Hallyn @ 2010-05-04 22:33 UTC (permalink / raw)
To: Garrett Cooper; +Cc: ltp-list
Quoting Garrett Cooper (yanegomi@gmail.com):
> On Tue, May 4, 2010 at 12:22 PM, Serge E. Hallyn <serue@us.ibm.com> wrote:
> > Quoting Subrata Modak (subrata@linux.vnet.ibm.com):
> >> Serge, please add a Sign-off.
> >
> > It's there in the patch in your attachment...
>
> Serge,
> Please address the items in the previous review and we'll get it committed.
> Thanks,
> -Garrett
Ah, I see. You only broke the testcase to get my attention so you
could get me to use more appropriate ltp helpers. Sneaky! And since
I learned some things in the process, great. Hopefully I'll get them
right when I submit the next set.
From 2077a47e9cd2f85a8e54695c71396a1a8397d3d6 Mon Sep 17 00:00:00 2001
From: Serge E. Hallyn <serue@us.ibm.com>
Date: Wed, 28 Apr 2010 11:26:58 -0500
Subject: [PATCH 1/1] make filecaps tests succeed
Most of these are belated cleanup after the move to using /opt/ltp.
But come on, replacing 'return' with tst_exit(), are you just trying
to mess with my head?
Changelog: may 4: address Garrett's feedback
1. single return 0 in print_caps.c
2. use $TMP if defined for location of caps_fifo
3. use tst_brkm in place of tst_resm.
Signed-off-by: Serge E. Hallyn <serue@us.ibm.com>
---
testcases/kernel/security/filecaps/filecapstest.sh | 10 +++-
testcases/kernel/security/filecaps/print_caps.c | 5 +-
.../kernel/security/filecaps/verify_caps_exec.c | 48 ++++++++------------
3 files changed, 28 insertions(+), 35 deletions(-)
diff --git a/testcases/kernel/security/filecaps/filecapstest.sh b/testcases/kernel/security/filecaps/filecapstest.sh
index 43582dc..8e2ba11 100755
--- a/testcases/kernel/security/filecaps/filecapstest.sh
+++ b/testcases/kernel/security/filecaps/filecapstest.sh
@@ -22,8 +22,12 @@
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"
+TMP=${TMP:=/tmp}
+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 +50,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..1c3fc1b 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[])
{
@@ -65,7 +65,6 @@ int main(int argc, char *argv[])
close(fd);
cap_free(cap);
-#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..c3f65a9 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;
@@ -78,16 +78,19 @@ int drop_root(int keep_perms)
prctl(PR_SET_KEEPCAPS, 1);
ret = setresuid(1000, 1000, 1000);
if (ret) {
- perror("setresuid");
- tst_resm(TFAIL, "Error dropping root privs\n");
+ tst_brkm(TFAIL | TERRNO, tst_exit, "Error dropping root privs\n");
tst_exit();
}
if (keep_perms) {
cap_t cap = cap_from_text("=eip");
- cap_set_proc(cap);
+ int ret;
+ if (!cap)
+ tst_brkm(TBROK | TERRNO, tst_exit, "cap_from_text failed\n");
+ ret = cap_set_proc(cap);
+ if (ret < 0)
+ tst_brkm(TBROK | TERRNO, tst_exit, "cap_set_proc failed\n");
cap_free(cap);
}
- tst_exit();
}
int perms_test(void)
@@ -114,17 +117,14 @@ int perms_test(void)
return ret;
}
-#define FIFOFILE "caps_fifo"
+#define FIFOFILE "/tmp/caps_fifo"
void create_fifo(void)
{
int ret;
ret = mkfifo(FIFOFILE, S_IRWXU | S_IRWXG | S_IRWXO);
- if (ret == -1 && errno != EEXIST) {
- perror("mkfifo");
- tst_resm(TFAIL, "failed creating %s\n", FIFOFILE);
- tst_exit();
- }
+ if (ret == -1 && errno != EEXIST)
+ tst_brkm(TFAIL | TERRNO, tst_exit, "failed creating %s\n", FIFOFILE);
}
void write_to_fifo(char *buf)
@@ -142,11 +142,8 @@ void read_from_fifo(char *buf)
memset(buf, 0, 200);
fd = open(FIFOFILE, O_RDONLY);
- if (fd < 0) {
- perror("open");
- tst_resm(TFAIL, "Failed opening fifo\n");
- tst_exit();
- }
+ if (fd < 0)
+ tst_brkm(TFAIL | TERRNO, tst_exit, "Failed opening fifo\n");
read(fd, buf, 199);
close(fd);
}
@@ -162,23 +159,18 @@ int fork_drop_and_exec(int keepperms, cap_t expected_caps)
static int seqno = 0;
pid = fork();
- if (pid < 0) {
- perror("fork");
- tst_resm(TFAIL, "%s: failed fork\n", __FUNCTION__);
- tst_exit();
- }
+ if (pid < 0)
+ tst_brkm(TFAIL | TERRNO, tst_exit, "%s: failed fork\n", __FUNCTION__);
if (pid == 0) {
drop_root(keepperms);
print_my_caps();
sprintf(buf, "%d", seqno);
ret = execlp(TSTPATH, TSTPATH, buf, NULL);
- perror("execl");
- tst_resm(TFAIL, "%s: exec failed\n", __FUNCTION__);
capstxt = cap_to_text(expected_caps, NULL);
snprintf(buf, 200, "failed to run as %s\n", capstxt);
cap_free(capstxt);
write_to_fifo(buf);
- tst_exit();
+ tst_brkm(TFAIL, tst_exit, "%s: exec failed\n", __FUNCTION__);
} else {
p = buf;
while (1) {
@@ -191,10 +183,8 @@ int fork_drop_and_exec(int keepperms, cap_t expected_caps)
c, s, seqno);
}
p = index(buf, '.')+1;
- if (p==(char *)1) {
- tst_resm(TFAIL, "got a bad message from print_caps\n");
- tst_exit();
- }
+ if (p==(char *)1)
+ tst_brkm(TFAIL, tst_exit, "got a bad message from print_caps\n");
actual_caps = cap_from_text(p);
if (cap_compare(actual_caps, expected_caps) != 0) {
capstxt = cap_to_text(expected_caps, NULL);
--
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] 9+ messages in thread
* Re: [LTP] [PATCH] Fix FILECAPS test hanging for more than 12 hours
2010-05-04 22:33 ` Serge E. Hallyn
@ 2010-05-05 5:19 ` Garrett Cooper
2010-05-05 14:18 ` Serge E. Hallyn
0 siblings, 1 reply; 9+ messages in thread
From: Garrett Cooper @ 2010-05-05 5:19 UTC (permalink / raw)
To: Serge E. Hallyn; +Cc: ltp-list
On Tue, May 4, 2010 at 3:33 PM, Serge E. Hallyn <serue@us.ibm.com> wrote:
> Quoting Garrett Cooper (yanegomi@gmail.com):
>> On Tue, May 4, 2010 at 12:22 PM, Serge E. Hallyn <serue@us.ibm.com> wrote:
>> > Quoting Subrata Modak (subrata@linux.vnet.ibm.com):
>> >> Serge, please add a Sign-off.
>> >
>> > It's there in the patch in your attachment...
>>
>> Serge,
>> Please address the items in the previous review and we'll get it committed.
>> Thanks,
>> -Garrett
>
> Ah, I see. You only broke the testcase to get my attention so you
> could get me to use more appropriate ltp helpers. Sneaky! And since
> I learned some things in the process, great. Hopefully I'll get them
> right when I submit the next set.
??? You're getting the glory, not me (and FWIW I haven't committed to
LTP in about a month)...
> From 2077a47e9cd2f85a8e54695c71396a1a8397d3d6 Mon Sep 17 00:00:00 2001
> From: Serge E. Hallyn <serue@us.ibm.com>
> Date: Wed, 28 Apr 2010 11:26:58 -0500
> Subject: [PATCH 1/1] make filecaps tests succeed
>
> Most of these are belated cleanup after the move to using /opt/ltp.
> But come on, replacing 'return' with tst_exit(), are you just trying
> to mess with my head?
Yeah, I admit that was stupid :(... I want trying to preserve the exit
code in the event that the call would fail *sigh*. It doesn't matter
that much now I suppose.
[...]
> p = index(buf, '.')+1;
> - if (p==(char *)1) {
> - tst_resm(TFAIL, "got a bad message from print_caps\n");
> - tst_exit();
> - }
> + if (p==(char *)1)
> + tst_brkm(TFAIL, tst_exit, "got a bad message from print_caps\n");
This is a really incorrect way to do things. I think that the
assumption made was that index(3) would return 0 ('\0') if it fails to
find '.'. That's incorrect and would cause a segfault on some systems
(does on FreeBSD at least... don't see why it would pass on Linux):
$ ~/test_null_inc
Segmentation fault: 11 (core dumped)
[garrcoop@bioshock ~]$ cat ~/test_null_inc.c
#include <stdio.h>
int
main(void)
{
printf("%s\n", (NULL + 1));
return 0;
}
Could you please change this to check and see whether or not index
returns NULL instead of accessing memory like that?
Other than that, patch looks good.
Thanks!
-Garrett
------------------------------------------------------------------------------
_______________________________________________
Ltp-list mailing list
Ltp-list@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/ltp-list
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [LTP] [PATCH] Fix FILECAPS test hanging for more than 12 hours
2010-05-05 5:19 ` Garrett Cooper
@ 2010-05-05 14:18 ` Serge E. Hallyn
2010-05-06 7:50 ` Garrett Cooper
0 siblings, 1 reply; 9+ messages in thread
From: Serge E. Hallyn @ 2010-05-05 14:18 UTC (permalink / raw)
To: Garrett Cooper; +Cc: ltp-list
Quoting Garrett Cooper (yanegomi@gmail.com):
> > p = index(buf, '.')+1;
Jinkeys! The intertubes archives insist I wrote that, but I'm finding
it hard to believe.
> > - if (p==(char *)1) {
> > - tst_resm(TFAIL, "got a bad message from print_caps\n");
> > - tst_exit();
> > - }
> > + if (p==(char *)1)
> > + tst_brkm(TFAIL, tst_exit, "got a bad message from print_caps\n");
>
> This is a really incorrect way to do things. I think that the
> assumption made was that index(3) would return 0 ('\0') if it fails to
> find '.'. That's incorrect and would cause a segfault on some systems
> (does on FreeBSD at least... don't see why it would pass on Linux):
>
> $ ~/test_null_inc
> Segmentation fault: 11 (core dumped)
> [garrcoop@bioshock ~]$ cat ~/test_null_inc.c
> #include <stdio.h>
> int
> main(void)
> {
> printf("%s\n", (NULL + 1));
> return 0;
> }
Well, that's different - you're dereferencing NULL+1, whereas I'm
just checking the the value of the pointer.
Still what I did is darned ugly, cleanup below.
thanks,
-serge
> Could you please change this to check and see whether or not index
> returns NULL instead of accessing memory like that?
> Other than that, patch looks good.
From: Serge E. Hallyn <serue@us.ibm.com>
Date: Wed, 5 May 2010 02:59:05 -0500
Subject: [PATCH 1/1] check for index(3) returning NULL
Signed-off-by: Serge E. Hallyn <serue@us.ibm.com>
---
.../kernel/security/filecaps/verify_caps_exec.c | 5 +++--
1 files changed, 3 insertions(+), 2 deletions(-)
diff --git a/testcases/kernel/security/filecaps/verify_caps_exec.c b/testcases/kernel/security/filecaps/verify_caps_exec.c
index c3f65a9..605f0f6 100644
--- a/testcases/kernel/security/filecaps/verify_caps_exec.c
+++ b/testcases/kernel/security/filecaps/verify_caps_exec.c
@@ -182,9 +182,10 @@ int fork_drop_and_exec(int keepperms, cap_t expected_caps)
tst_resm(TINFO, "got a bad seqno (c=%d, s=%d, seqno=%d)",
c, s, seqno);
}
- p = index(buf, '.')+1;
- if (p==(char *)1)
+ p = index(buf, '.');
+ if (!p)
tst_brkm(TFAIL, tst_exit, "got a bad message from print_caps\n");
+ p += 1;
actual_caps = cap_from_text(p);
if (cap_compare(actual_caps, expected_caps) != 0) {
capstxt = cap_to_text(expected_caps, NULL);
--
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] 9+ messages in thread
* Re: [LTP] [PATCH] Fix FILECAPS test hanging for more than 12 hours
2010-05-05 14:18 ` Serge E. Hallyn
@ 2010-05-06 7:50 ` Garrett Cooper
2010-05-06 13:55 ` Serge E. Hallyn
0 siblings, 1 reply; 9+ messages in thread
From: Garrett Cooper @ 2010-05-06 7:50 UTC (permalink / raw)
To: Serge E.Hallyn; +Cc: ltp-list
On May 5, 2010, at 7:18 AM, Serge E. Hallyn wrote:
> Quoting Garrett Cooper (yanegomi@gmail.com):
>>> p = index(buf, '.')+1;
>
> Jinkeys! The intertubes archives insist I wrote that, but I'm finding
> it hard to believe.
>
>>> - if (p==(char *)1) {
>>> - tst_resm(TFAIL, "got a bad message from print_caps\n");
>>> - tst_exit();
>>> - }
>>> + if (p==(char *)1)
>>> + tst_brkm(TFAIL, tst_exit, "got a bad message from print_caps\n");
>>
>> This is a really incorrect way to do things. I think that the
>> assumption made was that index(3) would return 0 ('\0') if it fails to
>> find '.'. That's incorrect and would cause a segfault on some systems
>> (does on FreeBSD at least... don't see why it would pass on Linux):
>>
>> $ ~/test_null_inc
>> Segmentation fault: 11 (core dumped)
>> [garrcoop@bioshock ~]$ cat ~/test_null_inc.c
>> #include <stdio.h>
>> int
>> main(void)
>> {
>> printf("%s\n", (NULL + 1));
>> return 0;
>> }
>
> Well, that's different - you're dereferencing NULL+1, whereas I'm
> just checking the the value of the pointer.
>
> Still what I did is darned ugly, cleanup below.
>
> thanks,
> -serge
>
>> Could you please change this to check and see whether or not index
>> returns NULL instead of accessing memory like that?
>> Other than that, patch looks good.
>
> From: Serge E. Hallyn <serue@us.ibm.com>
> Date: Wed, 5 May 2010 02:59:05 -0500
> Subject: [PATCH 1/1] check for index(3) returning NULL
>
> Signed-off-by: Serge E. Hallyn <serue@us.ibm.com>
> ---
> .../kernel/security/filecaps/verify_caps_exec.c | 5 +++--
> 1 files changed, 3 insertions(+), 2 deletions(-)
>
> diff --git a/testcases/kernel/security/filecaps/verify_caps_exec.c b/testcases/kernel/security/filecaps/verify_caps_exec.c
> index c3f65a9..605f0f6 100644
> --- a/testcases/kernel/security/filecaps/verify_caps_exec.c
> +++ b/testcases/kernel/security/filecaps/verify_caps_exec.c
> @@ -182,9 +182,10 @@ int fork_drop_and_exec(int keepperms, cap_t expected_caps)
> tst_resm(TINFO, "got a bad seqno (c=%d, s=%d, seqno=%d)",
> c, s, seqno);
> }
> - p = index(buf, '.')+1;
> - if (p==(char *)1)
> + p = index(buf, '.');
> + if (!p)
> tst_brkm(TFAIL, tst_exit, "got a bad message from print_caps\n");
> + p += 1;
> actual_caps = cap_from_text(p);
> if (cap_compare(actual_caps, expected_caps) != 0) {
> capstxt = cap_to_text(expected_caps, NULL);
Looks good! If that's the complete diff, then Acked-by: Garrett Cooper <yanegomi@gmail.com>
------------------------------------------------------------------------------
_______________________________________________
Ltp-list mailing list
Ltp-list@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/ltp-list
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [LTP] [PATCH] Fix FILECAPS test hanging for more than 12 hours
2010-05-06 7:50 ` Garrett Cooper
@ 2010-05-06 13:55 ` Serge E. Hallyn
2010-05-06 14:28 ` Subrata Modak
0 siblings, 1 reply; 9+ messages in thread
From: Serge E. Hallyn @ 2010-05-06 13:55 UTC (permalink / raw)
To: Garrett Cooper; +Cc: ltp-list
Quoting Garrett Cooper (yanegomi@gmail.com):
> On May 5, 2010, at 7:18 AM, Serge E. Hallyn wrote:
>
> > Quoting Garrett Cooper (yanegomi@gmail.com):
> >>> p = index(buf, '.')+1;
> >
> > Jinkeys! The intertubes archives insist I wrote that, but I'm finding
> > it hard to believe.
> >
> >>> - if (p==(char *)1) {
> >>> - tst_resm(TFAIL, "got a bad message from print_caps\n");
> >>> - tst_exit();
> >>> - }
> >>> + if (p==(char *)1)
> >>> + tst_brkm(TFAIL, tst_exit, "got a bad message from print_caps\n");
> >>
> >> This is a really incorrect way to do things. I think that the
> >> assumption made was that index(3) would return 0 ('\0') if it fails to
> >> find '.'. That's incorrect and would cause a segfault on some systems
> >> (does on FreeBSD at least... don't see why it would pass on Linux):
> >>
> >> $ ~/test_null_inc
> >> Segmentation fault: 11 (core dumped)
> >> [garrcoop@bioshock ~]$ cat ~/test_null_inc.c
> >> #include <stdio.h>
> >> int
> >> main(void)
> >> {
> >> printf("%s\n", (NULL + 1));
> >> return 0;
> >> }
> >
> > Well, that's different - you're dereferencing NULL+1, whereas I'm
> > just checking the the value of the pointer.
> >
> > Still what I did is darned ugly, cleanup below.
> >
> > thanks,
> > -serge
> >
> >> Could you please change this to check and see whether or not index
> >> returns NULL instead of accessing memory like that?
> >> Other than that, patch looks good.
> >
> > From: Serge E. Hallyn <serue@us.ibm.com>
> > Date: Wed, 5 May 2010 02:59:05 -0500
> > Subject: [PATCH 1/1] check for index(3) returning NULL
> >
> > Signed-off-by: Serge E. Hallyn <serue@us.ibm.com>
> > ---
> > .../kernel/security/filecaps/verify_caps_exec.c | 5 +++--
> > 1 files changed, 3 insertions(+), 2 deletions(-)
> >
> > diff --git a/testcases/kernel/security/filecaps/verify_caps_exec.c b/testcases/kernel/security/filecaps/verify_caps_exec.c
> > index c3f65a9..605f0f6 100644
> > --- a/testcases/kernel/security/filecaps/verify_caps_exec.c
> > +++ b/testcases/kernel/security/filecaps/verify_caps_exec.c
> > @@ -182,9 +182,10 @@ int fork_drop_and_exec(int keepperms, cap_t expected_caps)
> > tst_resm(TINFO, "got a bad seqno (c=%d, s=%d, seqno=%d)",
> > c, s, seqno);
> > }
> > - p = index(buf, '.')+1;
> > - if (p==(char *)1)
> > + p = index(buf, '.');
> > + if (!p)
> > tst_brkm(TFAIL, tst_exit, "got a bad message from print_caps\n");
> > + p += 1;
> > actual_caps = cap_from_text(p);
> > if (cap_compare(actual_caps, expected_caps) != 0) {
> > capstxt = cap_to_text(expected_caps, NULL);
>
> Looks good! If that's the complete diff, then Acked-by: Garrett Cooper <yanegomi@gmail.com>
Right - that one on top of the previous longer one, please. (or I can rebase-squash
them and resend if Subrata prefers)
-serge
------------------------------------------------------------------------------
_______________________________________________
Ltp-list mailing list
Ltp-list@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/ltp-list
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [LTP] [PATCH] Fix FILECAPS test hanging for more than 12 hours
2010-05-06 13:55 ` Serge E. Hallyn
@ 2010-05-06 14:28 ` Subrata Modak
0 siblings, 0 replies; 9+ messages in thread
From: Subrata Modak @ 2010-05-06 14:28 UTC (permalink / raw)
To: Serge E. Hallyn; +Cc: ltp-list
On Thu, 2010-05-06 at 08:55 -0500, Serge E. Hallyn wrote:
> Quoting Garrett Cooper (yanegomi@gmail.com):
> > On May 5, 2010, at 7:18 AM, Serge E. Hallyn wrote:
> >
> > > Quoting Garrett Cooper (yanegomi@gmail.com):
> > >>> p = index(buf, '.')+1;
> > >
> > > Jinkeys! The intertubes archives insist I wrote that, but I'm finding
> > > it hard to believe.
> > >
> > >>> - if (p==(char *)1) {
> > >>> - tst_resm(TFAIL, "got a bad message from print_caps\n");
> > >>> - tst_exit();
> > >>> - }
> > >>> + if (p==(char *)1)
> > >>> + tst_brkm(TFAIL, tst_exit, "got a bad message from print_caps\n");
> > >>
> > >> This is a really incorrect way to do things. I think that the
> > >> assumption made was that index(3) would return 0 ('\0') if it fails to
> > >> find '.'. That's incorrect and would cause a segfault on some systems
> > >> (does on FreeBSD at least... don't see why it would pass on Linux):
> > >>
> > >> $ ~/test_null_inc
> > >> Segmentation fault: 11 (core dumped)
> > >> [garrcoop@bioshock ~]$ cat ~/test_null_inc.c
> > >> #include <stdio.h>
> > >> int
> > >> main(void)
> > >> {
> > >> printf("%s\n", (NULL + 1));
> > >> return 0;
> > >> }
> > >
> > > Well, that's different - you're dereferencing NULL+1, whereas I'm
> > > just checking the the value of the pointer.
> > >
> > > Still what I did is darned ugly, cleanup below.
> > >
> > > thanks,
> > > -serge
> > >
> > >> Could you please change this to check and see whether or not index
> > >> returns NULL instead of accessing memory like that?
> > >> Other than that, patch looks good.
> > >
> > > From: Serge E. Hallyn <serue@us.ibm.com>
> > > Date: Wed, 5 May 2010 02:59:05 -0500
> > > Subject: [PATCH 1/1] check for index(3) returning NULL
> > >
> > > Signed-off-by: Serge E. Hallyn <serue@us.ibm.com>
> > > ---
> > > .../kernel/security/filecaps/verify_caps_exec.c | 5 +++--
> > > 1 files changed, 3 insertions(+), 2 deletions(-)
> > >
> > > diff --git a/testcases/kernel/security/filecaps/verify_caps_exec.c b/testcases/kernel/security/filecaps/verify_caps_exec.c
> > > index c3f65a9..605f0f6 100644
> > > --- a/testcases/kernel/security/filecaps/verify_caps_exec.c
> > > +++ b/testcases/kernel/security/filecaps/verify_caps_exec.c
> > > @@ -182,9 +182,10 @@ int fork_drop_and_exec(int keepperms, cap_t expected_caps)
> > > tst_resm(TINFO, "got a bad seqno (c=%d, s=%d, seqno=%d)",
> > > c, s, seqno);
> > > }
> > > - p = index(buf, '.')+1;
> > > - if (p==(char *)1)
> > > + p = index(buf, '.');
> > > + if (!p)
> > > tst_brkm(TFAIL, tst_exit, "got a bad message from print_caps\n");
> > > + p += 1;
> > > actual_caps = cap_from_text(p);
> > > if (cap_compare(actual_caps, expected_caps) != 0) {
> > > capstxt = cap_to_text(expected_caps, NULL);
> >
> > Looks good! If that's the complete diff, then Acked-by: Garrett Cooper <yanegomi@gmail.com>
>
> Right - that one on top of the previous longer one, please. (or I can rebase-squash
> them and resend if Subrata prefers)
A resend will be great.
Regards--
Subrata
>
> -serge
------------------------------------------------------------------------------
_______________________________________________
Ltp-list mailing list
Ltp-list@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/ltp-list
^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2010-05-06 14:29 UTC | newest]
Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2010-05-04 18:20 [LTP] [PATCH] Fix FILECAPS test hanging for more than 12 hours Subrata Modak
2010-05-04 19:22 ` Serge E. Hallyn
2010-05-04 21:02 ` Garrett Cooper
2010-05-04 22:33 ` Serge E. Hallyn
2010-05-05 5:19 ` Garrett Cooper
2010-05-05 14:18 ` Serge E. Hallyn
2010-05-06 7:50 ` Garrett Cooper
2010-05-06 13:55 ` Serge E. Hallyn
2010-05-06 14:28 ` Subrata Modak
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.