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) <[email protected]>
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 <[email protected]>
---
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);
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 <[email protected]>
---
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
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 <[email protected]>
---
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 <[email protected]> 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) <[email protected]>
On 12/5/19 9:40 AM, Micah Morton wrote:
> On Thu, Dec 5, 2019 at 4:20 AM Masami Hiramatsu <[email protected]> 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 <[email protected]> wrote:
> On 12/5/19 9:40 AM, Micah Morton wrote:
> > On Thu, Dec 5, 2019 at 4:20 AM Masami Hiramatsu <[email protected]> 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
>
--
Masami Hiramatsu <[email protected]>