Re: [LTP] [PATCH] Fix FILECAPS test hanging for more than 12 hours
On May 5, 2010, at 7:18 AM, Serge E. Hallyn wrote:
Quoting Garrett Cooper (yaneg...@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)
[garrc...@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 se...@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 se...@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
yaneg...@gmail.com
--
___
Ltp-list mailing list
Ltp-list@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/ltp-list
Re: [LTP] [PATCH] Fix FILECAPS test hanging for more than 12 hours
Quoting Garrett Cooper (yaneg...@gmail.com):
On May 5, 2010, at 7:18 AM, Serge E. Hallyn wrote:
Quoting Garrett Cooper (yaneg...@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)
[garrc...@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 se...@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 se...@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
yaneg...@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
Re: [LTP] [PATCH] Fix FILECAPS test hanging for more than 12 hours
On Thu, 2010-05-06 at 08:55 -0500, Serge E. Hallyn wrote:
Quoting Garrett Cooper (yaneg...@gmail.com):
On May 5, 2010, at 7:18 AM, Serge E. Hallyn wrote:
Quoting Garrett Cooper (yaneg...@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)
[garrc...@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 se...@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 se...@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
yaneg...@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
Re: [LTP] [PATCH] Fix FILECAPS test hanging for more than 12 hours
Quoting Garrett Cooper (yaneg...@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)
[garrc...@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 se...@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 se...@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
[LTP] [PATCH] Fix FILECAPS test hanging for more than 12 hours
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 subr...@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 S0 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 S0 0:00 \_ /bin/sh /opt/ltp/testcases/bin/filecapstest.sh 30311 30315 30311 1458 ? -1 S0 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: [r...@alien5 ltp]# strace -p 30425 Process 30425 attached - interrupt to quit waitpid(-1, ^C unfinished ... Process 30425 detached [r...@alien5 ltp]# strace -p 30429 Process 30429 attached - interrupt to quit open(caps_fifo, O_RDONLY^C unfinished ... Process 30429 detached [r...@alien5 ltp]# strace -p 30430 attach: ptrace(PTRACE_ATTACH, ...): Operation not permitted # getenforce Permissive [r...@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 Contextunconfined_u:system_r:syslogd_t:s0 Target Contextunconfined_u:object_r:usr_t:s0 Target Objects/opt/ltp/output/LTP_RUN_ON- 2010_Apr_21-17h_51m_22s.output [ file ] Source
Re: [LTP] [PATCH] Fix FILECAPS test hanging for more than 12 hours
Quoting Subrata Modak (subr...@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
Re: [LTP] [PATCH] Fix FILECAPS test hanging for more than 12 hours
On Tue, May 4, 2010 at 12:22 PM, Serge E. Hallyn se...@us.ibm.com wrote: Quoting Subrata Modak (subr...@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
Re: [LTP] [PATCH] Fix FILECAPS test hanging for more than 12 hours
On Tue, May 4, 2010 at 3:33 PM, Serge E. Hallyn se...@us.ibm.com wrote:
Quoting Garrett Cooper (yaneg...@gmail.com):
On Tue, May 4, 2010 at 12:22 PM, Serge E. Hallyn se...@us.ibm.com wrote:
Quoting Subrata Modak (subr...@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 se...@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)
[garrc...@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
