Hi,
Here is the v2 series to fix build warnings and erorrs on kselftest safesetid. This version includes a fix for a runtime error.
Thank you,
---
Masami Hiramatsu (3): selftests: safesetid: Move link library to LDLIBS selftests: safesetid: Check the return value of setuid/setgid selftests: safesetid: Fix Makefile to set correct test program
tools/testing/selftests/safesetid/Makefile | 5 +++-- tools/testing/selftests/safesetid/safesetid-test.c | 15 ++++++++++----- 2 files changed, 13 insertions(+), 7 deletions(-)
-- Masami Hiramatsu (Linaro) mhiramat@kernel.org
Move -lcap to LDLIBS from CFLAGS because it is a library to be linked.
Without this, safesetid failed to build with link error as below.
---- /usr/bin/ld: /tmp/ccL8rZHT.o: in function `drop_caps': safesetid-test.c:(.text+0xe7): undefined reference to `cap_get_proc' /usr/bin/ld: safesetid-test.c:(.text+0x107): undefined reference to `cap_set_flag' /usr/bin/ld: safesetid-test.c:(.text+0x10f): undefined reference to `cap_set_proc' /usr/bin/ld: safesetid-test.c:(.text+0x117): undefined reference to `cap_free' /usr/bin/ld: safesetid-test.c:(.text+0x136): undefined reference to `cap_clear' collect2: error: ld returned 1 exit status ----
Fixes: c67e8ec03f3f ("LSM: SafeSetID: add selftest") Signed-off-by: Masami Hiramatsu mhiramat@kernel.org --- tools/testing/selftests/safesetid/Makefile | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/tools/testing/selftests/safesetid/Makefile b/tools/testing/selftests/safesetid/Makefile index 98da7a504737..cac42cd36a1b 100644 --- a/tools/testing/selftests/safesetid/Makefile +++ b/tools/testing/selftests/safesetid/Makefile @@ -1,6 +1,7 @@ # SPDX-License-Identifier: GPL-2.0 # Makefile for mount selftests. -CFLAGS = -Wall -lcap -O2 +CFLAGS = -Wall -O2 +LDLIBS = -lcap
TEST_PROGS := run_tests.sh TEST_GEN_FILES := safesetid-test
Check the return value of setuid() and setgid(). This fixes the following warnings and improves test result.
safesetid-test.c: In function ‘main’: safesetid-test.c:294:2: warning: ignoring return value of ‘setuid’, declared with attribute warn_unused_result [-Wunused-result] setuid(NO_POLICY_USER); ^~~~~~~~~~~~~~~~~~~~~~ safesetid-test.c:295:2: warning: ignoring return value of ‘setgid’, declared with attribute warn_unused_result [-Wunused-result] setgid(NO_POLICY_USER); ^~~~~~~~~~~~~~~~~~~~~~ safesetid-test.c:309:2: warning: ignoring return value of ‘setuid’, declared with attribute warn_unused_result [-Wunused-result] setuid(RESTRICTED_PARENT); ^~~~~~~~~~~~~~~~~~~~~~~~~ safesetid-test.c:310:2: warning: ignoring return value of ‘setgid’, declared with attribute warn_unused_result [-Wunused-result] setgid(RESTRICTED_PARENT); ^~~~~~~~~~~~~~~~~~~~~~~~~ safesetid-test.c: In function ‘test_setuid’: safesetid-test.c:216:3: warning: ignoring return value of ‘setuid’, declared with attribute warn_unused_result [-Wunused-result] setuid(child_uid); ^~~~~~~~~~~~~~~~~
Fixes: c67e8ec03f3f ("LSM: SafeSetID: add selftest") Signed-off-by: Masami Hiramatsu mhiramat@kernel.org --- tools/testing/selftests/safesetid/safesetid-test.c | 15 ++++++++++----- 1 file changed, 10 insertions(+), 5 deletions(-)
diff --git a/tools/testing/selftests/safesetid/safesetid-test.c b/tools/testing/selftests/safesetid/safesetid-test.c index 8f40c6ecdad1..0c4d50644c13 100644 --- a/tools/testing/selftests/safesetid/safesetid-test.c +++ b/tools/testing/selftests/safesetid/safesetid-test.c @@ -213,7 +213,8 @@ static void test_setuid(uid_t child_uid, bool expect_success) }
if (cpid == 0) { /* Code executed by child */ - setuid(child_uid); + if (setuid(child_uid) < 0) + exit(EXIT_FAILURE); if (getuid() == child_uid) exit(EXIT_SUCCESS); else @@ -291,8 +292,10 @@ int main(int argc, char **argv)
// First test to make sure we can write userns mappings from a user // that doesn't have any restrictions (as long as it has CAP_SETUID); - setuid(NO_POLICY_USER); - setgid(NO_POLICY_USER); + if (setuid(NO_POLICY_USER) < 0) + die("Error with set uid(%d)\n", NO_POLICY_USER); + if (setgid(NO_POLICY_USER) < 0) + die("Error with set gid(%d)\n", NO_POLICY_USER);
// Take away all but setid caps drop_caps(true); @@ -306,8 +309,10 @@ int main(int argc, char **argv) die("test_userns failed when it should work\n"); }
- setuid(RESTRICTED_PARENT); - setgid(RESTRICTED_PARENT); + if (setuid(RESTRICTED_PARENT) < 0) + die("Error with set uid(%d)\n", RESTRICTED_PARENT); + if (setgid(RESTRICTED_PARENT) < 0) + die("Error with set gid(%d)\n", RESTRICTED_PARENT);
test_setuid(ROOT_USER, false); test_setuid(ALLOWED_CHILD1, true);
Fix Makefile to set safesetid-test.sh to TEST_PROGS instead of non existing run_tests.sh.
Without this fix, I got following error. ---- TAP version 13 1..1 # selftests: safesetid: run_tests.sh # Warning: file run_tests.sh is missing! not ok 1 selftests: safesetid: run_tests.sh ----
Fixes: c67e8ec03f3f ("LSM: SafeSetID: add selftest") Signed-off-by: Masami Hiramatsu mhiramat@kernel.org --- tools/testing/selftests/safesetid/Makefile | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/tools/testing/selftests/safesetid/Makefile b/tools/testing/selftests/safesetid/Makefile index cac42cd36a1b..fa02c4d5ec13 100644 --- a/tools/testing/selftests/safesetid/Makefile +++ b/tools/testing/selftests/safesetid/Makefile @@ -3,7 +3,7 @@ CFLAGS = -Wall -O2 LDLIBS = -lcap
-TEST_PROGS := run_tests.sh +TEST_PROGS := safesetid-test.sh TEST_GEN_FILES := safesetid-test
include ../lib.mk
On Thu, Dec 5, 2019 at 4:20 AM Masami Hiramatsu mhiramat@kernel.org wrote:
Hi,
Here is the v2 series to fix build warnings and erorrs on kselftest safesetid. This version includes a fix for a runtime error.
Thank you,
Masami Hiramatsu (3): selftests: safesetid: Move link library to LDLIBS selftests: safesetid: Check the return value of setuid/setgid selftests: safesetid: Fix Makefile to set correct test program
These 3 fixes look good, thanks. Were you thinking they would go through my SafeSetID tree or is there a dedicated one for selftests? I guess if you're not sure someone else on here can chime in, or I can just take them through my tree if I don't hear anything.
tools/testing/selftests/safesetid/Makefile | 5 +++-- tools/testing/selftests/safesetid/safesetid-test.c | 15 ++++++++++----- 2 files changed, 13 insertions(+), 7 deletions(-)
-- Masami Hiramatsu (Linaro) mhiramat@kernel.org
On 12/5/19 9:40 AM, Micah Morton wrote:
On Thu, Dec 5, 2019 at 4:20 AM Masami Hiramatsu mhiramat@kernel.org wrote:
Hi,
Here is the v2 series to fix build warnings and erorrs on kselftest safesetid. This version includes a fix for a runtime error.
Thank you,
Masami Hiramatsu (3): selftests: safesetid: Move link library to LDLIBS selftests: safesetid: Check the return value of setuid/setgid selftests: safesetid: Fix Makefile to set correct test program
These 3 fixes look good, thanks. Were you thinking they would go through my SafeSetID tree or is there a dedicated one for selftests? I guess if you're not sure someone else on here can chime in, or I can just take them through my tree if I don't hear anything.
Yes. There is a linux-kselftest tree dedicated to selftests. I can take them.
tools/testing/selftests/safesetid/Makefile | 5 +++-- tools/testing/selftests/safesetid/safesetid-test.c | 15 ++++++++++----- 2 files changed, 13 insertions(+), 7 deletions(-)
--
thanks, -- Shuah
On Thu, 5 Dec 2019 09:44:52 -0700 shuah shuah@kernel.org wrote:
On 12/5/19 9:40 AM, Micah Morton wrote:
On Thu, Dec 5, 2019 at 4:20 AM Masami Hiramatsu mhiramat@kernel.org wrote:
Hi,
Here is the v2 series to fix build warnings and erorrs on kselftest safesetid. This version includes a fix for a runtime error.
Thank you,
Masami Hiramatsu (3): selftests: safesetid: Move link library to LDLIBS selftests: safesetid: Check the return value of setuid/setgid selftests: safesetid: Fix Makefile to set correct test program
These 3 fixes look good, thanks. Were you thinking they would go through my SafeSetID tree or is there a dedicated one for selftests? I guess if you're not sure someone else on here can chime in, or I can just take them through my tree if I don't hear anything.
Yes. There is a linux-kselftest tree dedicated to selftests. I can take them.
Thanks Micah and Shuah!
tools/testing/selftests/safesetid/Makefile | 5 +++-- tools/testing/selftests/safesetid/safesetid-test.c | 15 ++++++++++----- 2 files changed, 13 insertions(+), 7 deletions(-)
--
thanks, -- Shuah
linux-kselftest-mirror@lists.linaro.org
-
Masami Hiramatsu -
Micah Morton -
shuah