Skip to contentSkip to conversation

Move getcwd05 to realpath01 and fix for old distro

PATCH New Michael Moese 4 participants 7 messages Aug 1, 2018 – Aug 14, 2018

lore
Share & tools
lore

Michael Moese

author
#0 lore raw

PATCH 1/1 Move getcwd05 to realpath01 and fix for old distro

The testcase getcwd05 is a regression test for cve-2018-1000001 [1]. However, there were changes in the behavior of libc functions, that some older distributions refused to backport.
The testcase was two testcases, one for getcwd() and one for realpath(). While the behavior of getcwd() changed, it is totally independent from the thestcase for the vulnerability in realpath. So, this test should be moved to realpath/realpath01.c. In addition, the test of getcwd() is totally unneeded here to test realpath() for the fix.

[1] https://sourceware.org/git/gitweb.cgi?p=glibc.git;a=commitdiff;h=52a713fdd0a30e1bd79818e2e3c4ab44ddca1a94

Signed-off-by Michael Moese <[email protected]>
---
 runtest/cve                                   |  2 +-
 runtest/syscalls                              |  3 ++-
 testcases/kernel/syscalls/getcwd/.gitignore   |  1 -
 testcases/kernel/syscalls/realpath/.gitignore |  1 +
 testcases/kernel/syscalls/realpath/Makefile   | 23 ++++++++++++++++
 .../getcwd05.c => realpath/realpath01.c}      | 26 +++++--------------
 6 files changed, 33 insertions(+), 23 deletions(-)
 create mode 100644 testcases/kernel/syscalls/realpath/.gitignore
 create mode 100644 testcases/kernel/syscalls/realpath/Makefile
 rename testcases/kernel/syscalls/{getcwd/getcwd05.c => realpath/realpath01.c} (72%)
runtest/cve +1 -1
index 58d8f12b2..b38fb3503 100644
@@ -34,4 +34,4 @@ cve-2017-16939 cve-2017-16939
 cve-2017-17053 cve-2017-17053
 cve-2017-18075 pcrypt_aead01
 cve-2018-5803 sctp_big_chunk
-cve-2018-1000001 getcwd05
+cve-2018-1000001 realpath01
runtest/syscalls +2 -1
index dc72484cb..4e813e9a7 100644
@@ -348,7 +348,6 @@ getcwd01 getcwd01
 getcwd02 getcwd02
 getcwd03 getcwd03
 getcwd04 getcwd04
-getcwd05 getcwd05
 
 getdents01 getdents01
 getdents02 getdents02

@@ -895,6 +894,8 @@ readv01 readv01
 readv02 readv02
 readv03 readv03
 
+realpath01 realpath01
+
 reboot01 reboot01
 reboot02 reboot02
testcases/kernel/syscalls/getcwd/.gitignore +0 -1
index 99b2ba19f..338852b62 100644
@@ -2,4 +2,3 @@
 /getcwd02
 /getcwd03
 /getcwd04
-/getcwd05
testcases/kernel/syscalls/realpath/.gitignore new file +1 -0
new file mode 100644
index 000000000..54860c088
@@ -0,0 +1 @@
+/realpath01
testcases/kernel/syscalls/realpath/Makefile new file +23 -0
new file mode 100644
index 000000000..bd617d806
@@ -0,0 +1,23 @@
+#
+#  Copyright (c) International Business Machines  Corp., 2001
+#
+#  This program is free software;  you can redistribute it and/or modify
+#  it under the terms of the GNU General Public License as published by
+#  the Free Software Foundation; either version 2 of the License, or
+#  (at your option) any later version.
+#
+#  This program is distributed in the hope that it will be useful,
+#  but WITHOUT ANY WARRANTY;  without even the implied warranty of
+#  MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See
+#  the GNU General Public License for more details.
+#
+#  You should have received a copy of the GNU General Public License
+#  along with this program;  if not, write to the Free Software
+#  Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA  02110-1301  USA
+#
+
+top_srcdir		?= ../../../..
+
+include $(top_srcdir)/include/mk/testcases.mk
+
+include $(top_srcdir)/include/mk/generic_leaf_target.mk
testcases/kernel/syscalls/realpath/realpath01.c renamed from testcases/kernel/syscalls/getcwd/getcwd05.c +6 -20
similarity index 72%
rename from testcases/kernel/syscalls/getcwd/getcwd05.c
rename to testcases/kernel/syscalls/realpath/realpath01.c
index f39df4b1d..dcd6785fc 100644
@@ -20,36 +20,22 @@ static void setup(void)
 	SAFE_CHROOT(CHROOT_DIR);
 }
 
-static void run(unsigned int i)
+static void run(void)
 {
-	int fail = 0;
-
-	if (i) {
-		tst_res(TINFO, "testing realpath()");
-		TESTPTR(realpath(".", NULL));
-	} else {
-		tst_res(TINFO, "testing getcwd()");
-		TESTPTR(getcwd(NULL, 0));
-	}
+	TESTPTR(realpath(".", NULL));
 
 	if (TST_ERR != ENOENT) {
 		tst_res(TFAIL | TTERRNO, "returned unexpected errno");
-		fail = 1;
-	}
-
-	if (TST_RET_PTR != NULL) {
+	} else	if (TST_RET_PTR != NULL) {
 		tst_res(TFAIL, "syscall didn't return NULL: '%s'",
 				(char *)TST_RET_PTR);
-		fail = 1;
-	}
-
-	if (!fail)
+	} else {
 		tst_res(TPASS, "bug not reproduced");
+	}
 }
 
 static struct tst_test test = {
-	.test = run,
-	.tcnt = 2,
+	.test_all = run,
 	.setup = setup,
 	.needs_root = 1,
 	.needs_tmpdir = 1,
Signature · 1 line
2.18.0
#1 lore raw

PATCH 1/1 Move getcwd05 to realpath01 and fix for old distro

----- Original Message -----

Quoted 10 lines

The testcase getcwd05 is a regression test for cve-2018-1000001 [1]. However, there were changes in the behavior of libc functions, that some older
distributions refused to backport.
The testcase was two testcases, one for getcwd() and one for realpath(). While the behavior of getcwd() changed, it is totally independent from the thestcase for the vulnerability in realpath. So, this test should be moved to realpath/realpath01.c. In addition, the test of getcwd() is totally unneeded here to test realpath() for the fix.

Quoted 4 lines

[1]
https://sourceware.org/git/gitweb.cgi?p=glibc.git;a=commitdiff;h=52a713fdd0a30e1bd79818e2e3c4ab44ddca1a94

Signed-off-by Michael Moese <[email protected]>

Looks good to me, ack.
Adding Petr, as he wrote the original test.

Regards,
Jan

#2 lore raw

PATCH 1/1 Move getcwd05 to realpath01 and fix for old distro

In reply to Michael Moese

Hi Michael,

Quoted 9 lines

The testcase getcwd05 is a regression test for cve-2018-1000001 [1]. However, there were changes in the behavior of libc functions, that some older distributions refused to backport.
The testcase was two testcases, one for getcwd() and one for realpath(). While the behavior of getcwd() changed, it is totally independent from the thestcase for the vulnerability in realpath. So, this test should be moved to realpath/realpath01.c. In addition, the test of getcwd() is totally unneeded here to test realpath() for the fix.

Quoted 3 lines

[1] https://sourceware.org/git/gitweb.cgi?p=glibc.git;a=commitdiff;h=52a713fdd0a30e1bd79818e2e3c4ab44ddca1a94

Signed-off-by Michael Moese <[email protected]>
Acked-by Petr Vorel <[email protected]>

Thanks for your patch. ACK with 2 minor issues bellow. I can fix them (no need to repost a patch) if you agree.

Quoted 1 line
 rename testcases/kernel/syscalls/{getcwd/getcwd05.c => realpath/realpath01.c} (72%)

I wonder whether file should be named realpath_buffer_underflow.c to be more descriptive.

testcases/kernel/syscalls/realpath/Makefile line 2

Quoted 8 lines
testcases/kernel/syscalls/realpath/Makefile new file +2 -0
new file mode 100644
index 000000000..bd617d806
@@ -0,0 +1,23 @@
+#
+#  Copyright (c) International Business Machines  Corp., 2001

Copy paste error. + I'd prefer to use 'SPDX-License-Identifier: GPL-2.0-or-later' as it's shorter.

Quoted 4 lines
testcases/kernel/syscalls/realpath/Makefile new file +4 -0
+#
+#  This program is free software;  you can redistribute it and/or modify
+#  it under the terms of the GNU General Public License as published by
+#  the Free Software Foundation; either version 2 of the License, or

testcases/kernel/syscalls/realpath/Makefile line 16

Quoted 10 lines
testcases/kernel/syscalls/realpath/Makefile new file +10 -0
+#  (at your option) any later version.
+#
+#  This program is distributed in the hope that it will be useful,
+#  but WITHOUT ANY WARRANTY;  without even the implied warranty of
+#  MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See
+#  the GNU General Public License for more details.
+#
+#  You should have received a copy of the GNU General Public License
+#  along with this program;  if not, write to the Free Software
+#  Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA  02110-1301  USA

Kind regards,
Petr

#3 lore raw

PATCH 1/1 Move getcwd05 to realpath01 and fix for old distro

Hi!

Quoted 3 lines
Quoted 1 line
 rename testcases/kernel/syscalls/{getcwd/getcwd05.c => realpath/realpath01.c} (72%)

I wonder whether file should be named realpath_buffer_underflow.c to be more descriptive.

We usually keep just the names short and simple, so I would keep it as it is.

Quoted 9 lines

testcases/kernel/syscalls/realpath/Makefile line 2

Quoted 8 lines
testcases/kernel/syscalls/realpath/Makefile new file +2 -0
new file mode 100644
index 000000000..bd617d806
@@ -0,0 +1,23 @@
+#
+#  Copyright (c) International Business Machines  Corp., 2001

Copy paste error. + I'd prefer to use 'SPDX-License-Identifier: GPL-2.0-or-later' as it's shorter.

Just fix this before you push the patch.

Quoted 14 lines
Quoted 14 lines
testcases/kernel/syscalls/realpath/Makefile new file +14 -0
+#
+#  This program is free software;  you can redistribute it and/or modify
+#  it under the terms of the GNU General Public License as published by
+#  the Free Software Foundation; either version 2 of the License, or
+#  (at your option) any later version.
+#
+#  This program is distributed in the hope that it will be useful,
+#  but WITHOUT ANY WARRANTY;  without even the implied warranty of
+#  MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See
+#  the GNU General Public License for more details.
+#
+#  You should have received a copy of the GNU General Public License
+#  along with this program;  if not, write to the Free Software
+#  Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA  02110-1301  USA
Signature · 2 lines
Cyril Hrubis
[email protected]
#4 lore raw

PATCH 1/1 Move getcwd05 to realpath01 and fix for old distro

Hi Cyril,

Quoted 7 lines

Hi!

Quoted 3 lines
Quoted 1 line
 rename testcases/kernel/syscalls/{getcwd/getcwd05.c => realpath/realpath01.c} (72%)

I wonder whether file should be named realpath_buffer_underflow.c to be more descriptive.

We usually keep just the names short and simple, so I would keep it as it is.

I think CVE related are the only exception (see Eric Biggers's post [1]), but I'm ok with keeping it only realpath01.c.

Kind regards,
Petr

[1] http://lists.linux.it/pipermail/ltp/2018-March/007388.html

#5 lore raw

PATCH 1/1 Move getcwd05 to realpath01 and fix for old distro

Hi!

Quoted 4 lines
Quoted 2 lines

We usually keep just the names short and simple, so I would keep it as it is.

I think CVE related are the only exception (see Eric Biggers's post [1]), but I'm ok with keeping it only realpath01.c.

Let's keep the names short, we do have a bunch of regression tests named as syscallXY.c anyways. And the CVE was in realpath() implementation anyways, so it's not like we named wrongly to begin with.

Quoted 1 line

[1] http://lists.linux.it/pipermail/ltp/2018-March/007388.html

Signature · 2 lines
Cyril Hrubis
[email protected]
#6 lore raw

PATCH 1/1 Move getcwd05 to realpath01 and fix for old distro

Hi,

Quoted 8 lines
Quoted 4 lines
Quoted 2 lines

We usually keep just the names short and simple, so I would keep it as it is.

I think CVE related are the only exception (see Eric Biggers's post [1]), but I'm ok with keeping it only realpath01.c.

Let's keep the names short, we do have a bunch of regression tests named as syscallXY.c anyways. And the CVE was in realpath() implementation anyways, so it's not like we named wrongly to begin with.

OK, pushed with the original name.
Thanks for your patch, Michael.

I did tiny changes:
Fixed copyright in Makefile and state reproducer info ("cve-2018-1000001 realpath buffer underflow") in source file. This should have been done by me in the original post (when renaming it form cve-2018-1000001.c to getcwd05.c in later versions of my original post).

Kind regards,
Petr

Quoted 1 line
Quoted 1 line

[1] http://lists.linux.it/pipermail/ltp/2018-March/007388.html