NetBSD Problem Report #54382

From www@netbsd.org  Tue Jul 16 16:22:02 2019
Return-Path: <www@netbsd.org>
Received: from mail.netbsd.org (mail.netbsd.org [199.233.217.200])
	(using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits))
	(Client CN "mail.NetBSD.org", Issuer "mail.NetBSD.org CA" (not verified))
	by mollari.NetBSD.org (Postfix) with ESMTPS id 7682A7A18E
	for <gnats-bugs@gnats.NetBSD.org>; Tue, 16 Jul 2019 16:22:02 +0000 (UTC)
Message-Id: <20190716162201.3663B7A1BF@mollari.NetBSD.org>
Date: Tue, 16 Jul 2019 16:22:01 +0000 (UTC)
From: brooks@freebsd.org
Reply-To: brooks@freebsd.org
To: gnats-bugs@NetBSD.org
Subject: missing permissions in open(2) calls with O_CREAT
X-Send-Pr-Version: www-1.0

>Number:         54382
>Category:       misc
>Synopsis:       missing permissions in open(2) calls with O_CREAT
>Confidential:   no
>Severity:       serious
>Priority:       medium
>Responsible:    misc-bug-people
>State:          closed
>Class:          sw-bug
>Submitter-Id:   net
>Arrival-Date:   Tue Jul 16 16:25:00 +0000 2019
>Closed-Date:    Wed Jul 17 07:04:38 +0000 2019
>Last-Modified:  Wed Jul 17 07:04:38 +0000 2019
>Originator:     Brooks Davis
>Release:        current
>Organization:
>Environment:
Found in FreeBSD's import of the libc tests.
>Description:
The test suite contains a large number of instances of things like:

        fd = open(path, O_RDWR | O_CREAT);

When O_CREAT is specified, the third, variadic argument is required as the permission.  If on is not passed, then depending on the ABI, either the contents of the third argument register or some arbitrary stuff on the stack will be used as the permission.  This is clearly wrong.

In our local ABI (for the CHERI ISA), variadic arguments are stored in a bounded array which in this case is of length zero.  When read, a fault occurs and these tests crash.
>How-To-Repeat:

>Fix:
Known instances of this bug are fixed in this patch:

diff --git a/tests/lib/libc/gen/t_ftok.c b/tests/lib/libc/gen/t_ftok.c
index 4c1ab18950eb..6a44049d0f85 100644
--- a/tests/lib/libc/gen/t_ftok.c
+++ b/tests/lib/libc/gen/t_ftok.c
@@ -65,7 +65,7 @@ ATF_TC_BODY(ftok_link, tc)
 	key_t k1, k2, k3;
 	int fd;

-	fd = open(path, O_RDONLY | O_CREAT);
+	fd = open(path, O_RDONLY | O_CREAT, 0600);

 	ATF_REQUIRE(fd >= 0);
 	(void)close(fd);
diff --git a/tests/lib/libc/stdio/t_fopen.c b/tests/lib/libc/stdio/t_fopen.c
index c5b89215430f..1946c921aeb6 100644
--- a/tests/lib/libc/stdio/t_fopen.c
+++ b/tests/lib/libc/stdio/t_fopen.c
@@ -58,7 +58,7 @@ ATF_TC_BODY(fdopen_close, tc)
 	 * used to fdopen(3) a stream is
 	 * closed once the stream is closed.
 	 */
-	fd = open(path, O_RDWR | O_CREAT);
+	fd = open(path, O_RDWR | O_CREAT, 0600);

 	ATF_REQUIRE(fd >= 0);

@@ -85,7 +85,7 @@ ATF_TC_BODY(fdopen_err, tc)
 {
 	int fd;

-	fd = open(path, O_RDONLY | O_CREAT);
+	fd = open(path, O_RDONLY | O_CREAT, 0600);
 	ATF_REQUIRE(fd >= 0);

 	errno = 0;
@@ -126,7 +126,7 @@ ATF_TC_BODY(fdopen_seek, tc)
 	 * with the stream corresponds with the offset
 	 * set earlier for the file descriptor.
 	 */
-	fd = open(path, O_RDWR | O_CREAT);
+	fd = open(path, O_RDWR | O_CREAT, 0600);

 	ATF_REQUIRE(fd >= 0);
 	ATF_REQUIRE(write(fd, "garbage", 7) == 7);
diff --git a/tests/lib/libc/sys/t_access.c b/tests/lib/libc/sys/t_access.c
index f75617235972..d4a5cd1e75b1 100644
--- a/tests/lib/libc/sys/t_access.c
+++ b/tests/lib/libc/sys/t_access.c
@@ -58,7 +58,7 @@ ATF_TC_BODY(access_access, tc)
 	size_t i;
 	int fd;

-	fd = open(path, O_RDONLY | O_CREAT);
+	fd = open(path, O_RDONLY | O_CREAT, 0600);

 	if (fd < 0)
 		return;
diff --git a/tests/lib/libc/sys/t_mprotect.c b/tests/lib/libc/sys/t_mprotect.c
index 3526554b2863..a8b9ae853dbb 100644
--- a/tests/lib/libc/sys/t_mprotect.c
+++ b/tests/lib/libc/sys/t_mprotect.c
@@ -87,7 +87,7 @@ ATF_TC_BODY(mprotect_access, tc)
 	size_t i;
 	int fd;

-	fd = open(path, O_RDONLY | O_CREAT);
+	fd = open(path, O_RDONLY | O_CREAT, 0600);
 	ATF_REQUIRE(fd >= 0);

 	/*
diff --git a/tests/lib/libc/sys/t_stat.c b/tests/lib/libc/sys/t_stat.c
index adb32bb11efc..9d0136dae50b 100644
--- a/tests/lib/libc/sys/t_stat.c
+++ b/tests/lib/libc/sys/t_stat.c
@@ -64,7 +64,7 @@ ATF_TC_BODY(stat_chflags, tc)
 	(void)memset(&sa, 0, sizeof(struct stat));
 	(void)memset(&sb, 0, sizeof(struct stat));

-	fd = open(path, O_RDONLY | O_CREAT);
+	fd = open(path, O_RDONLY | O_CREAT, 0600);

 	ATF_REQUIRE(fd != -1);
 	ATF_REQUIRE(stat(path, &sa) == 0);
@@ -210,7 +210,7 @@ ATF_TC_BODY(stat_mtime, tc)
 		(void)memset(&sa, 0, sizeof(struct stat));
 		(void)memset(&sb, 0, sizeof(struct stat));

-		fd[i] = open(path, O_WRONLY | O_CREAT);
+		fd[i] = open(path, O_WRONLY | O_CREAT, 0600);

 		ATF_REQUIRE(fd[i] != -1);
 		ATF_REQUIRE(write(fd[i], "X", 1) == 1);
@@ -254,7 +254,7 @@ ATF_TC_BODY(stat_perm, tc)
 	uid = getuid();
 	gid = getgid();

-	fd = open(path, O_RDONLY | O_CREAT);
+	fd = open(path, O_RDONLY | O_CREAT, 0600);

 	ATF_REQUIRE(fd != -1);
 	ATF_REQUIRE(fstat(fd, &sa) == 0);
@@ -288,7 +288,7 @@ ATF_TC_BODY(stat_size, tc)
 	size_t i;
 	int fd;

-	fd = open(path, O_WRONLY | O_CREAT);
+	fd = open(path, O_WRONLY | O_CREAT, 0600);
 	ATF_REQUIRE(fd >= 0);

 	for (i = 0; i < n; i++) {
@@ -377,7 +377,7 @@ ATF_TC_BODY(stat_symlink, tc)
 	(void)memset(&sa, 0, sizeof(struct stat));
 	(void)memset(&sb, 0, sizeof(struct stat));

-	fd = open(path, O_WRONLY | O_CREAT);
+	fd = open(path, O_WRONLY | O_CREAT, 0600);

 	ATF_REQUIRE(fd >= 0);
 	ATF_REQUIRE(symlink(path, pathlink) == 0);
diff --git a/tests/lib/libc/sys/t_write.c b/tests/lib/libc/sys/t_write.c
index 576ce7e7c0b5..18bcaba4f7aa 100644
--- a/tests/lib/libc/sys/t_write.c
+++ b/tests/lib/libc/sys/t_write.c
@@ -69,7 +69,7 @@ ATF_TC_BODY(write_err, tc)
 	errno = 0;
 	ATF_REQUIRE_ERRNO(EBADF, write(-1, wbuf, sizeof(wbuf)) == -1);

-	fd = open(path, O_RDWR | O_CREAT);
+	fd = open(path, O_RDWR | O_CREAT, 0600);

 	if (fd >= 0) {

@@ -141,7 +141,7 @@ ATF_TC_BODY(write_pos, tc)
 	size_t i;
 	int fd;

-	fd = open(path, O_RDWR | O_CREAT);
+	fd = open(path, O_RDWR | O_CREAT, 0600);
 	ATF_REQUIRE(fd >= 0);

 	for (i = 0; i < n; i++) {
@@ -171,7 +171,7 @@ ATF_TC_BODY(write_ret, tc)
 	size_t i, j;
 	int fd;

-	fd = open(path, O_WRONLY | O_CREAT);
+	fd = open(path, O_WRONLY | O_CREAT, 0600);
 	ATF_REQUIRE(fd >= 0);

 	(void)memset(buf, 'x', sizeof(buf));

>Release-Note:

>Audit-Trail:
From: "Martin Husemann" <martin@netbsd.org>
To: gnats-bugs@gnats.NetBSD.org
Cc: 
Subject: PR/54382 CVS commit: src/tests
Date: Tue, 16 Jul 2019 17:29:18 +0000

 Module Name:	src
 Committed By:	martin
 Date:		Tue Jul 16 17:29:18 UTC 2019

 Modified Files:
 	src/tests/fs/nfs: t_mountd.c
 	src/tests/fs/union: t_pr.c
 	src/tests/fs/vfs: t_full.c t_io.c t_ro.c
 	src/tests/lib/libc/gen: t_ftok.c
 	src/tests/lib/libc/stdio: t_fopen.c
 	src/tests/lib/libc/sys: t_access.c t_mprotect.c t_stat.c t_write.c
 	src/tests/rump/rumpkern/h_client: h_forkcli.c

 Log Message:
 PR misc/54382: whenever open(2) is called with O_CREAT, make sure to
 pass an open mode argument.


 To generate a diff of this commit:
 cvs rdiff -u -r1.6 -r1.7 src/tests/fs/nfs/t_mountd.c
 cvs rdiff -u -r1.12 -r1.13 src/tests/fs/union/t_pr.c
 cvs rdiff -u -r1.10 -r1.11 src/tests/fs/vfs/t_full.c
 cvs rdiff -u -r1.17 -r1.18 src/tests/fs/vfs/t_io.c
 cvs rdiff -u -r1.6 -r1.7 src/tests/fs/vfs/t_ro.c
 cvs rdiff -u -r1.2 -r1.3 src/tests/lib/libc/gen/t_ftok.c
 cvs rdiff -u -r1.6 -r1.7 src/tests/lib/libc/stdio/t_fopen.c
 cvs rdiff -u -r1.2 -r1.3 src/tests/lib/libc/sys/t_access.c
 cvs rdiff -u -r1.7 -r1.8 src/tests/lib/libc/sys/t_mprotect.c
 cvs rdiff -u -r1.5 -r1.6 src/tests/lib/libc/sys/t_stat.c
 cvs rdiff -u -r1.6 -r1.7 src/tests/lib/libc/sys/t_write.c
 cvs rdiff -u -r1.1 -r1.2 src/tests/rump/rumpkern/h_client/h_forkcli.c

 Please note that diffs are not public domain; they are subject to the
 copyright notices on the relevant files.

State-Changed-From-To: open->closed
State-Changed-By: maya@NetBSD.org
State-Changed-When: Wed, 17 Jul 2019 07:04:38 +0000
State-Changed-Why:
Applied, thanks for the patch!


>Unformatted:

Home	PR Database Search