[PATCH 0/3] tools/nolibc: fix fopen() errno and "rb+" modes

[PATCH 0/3] tools/nolibc: fix fopen() errno and "rb+" modes

[Danish Khateeb]

fopen() has two bugs:

- When open() fails, fopen() overwrites its errno with EBADF, so a
  missing file is reported as EBADF instead of ENOENT.

- "rb+", "wb+" and "ab+" are not recognised as update modes, so the
  stream is opened read-only or write-only.

Patches 1 and 2 fix them. Patch 3 adds nolibc-test cases for both.

Tested on top of nolibc/for-next 700ce4da6f6f, on x86_64 (GCC and
clang) and i386, and on arm, arm64 and sparc64 under qemu-user:

- nolibc-test, all tests: no failures with the series. The only
  difference from before is the three new tests, which pass. They also
  pass against glibc (make libc-test).

- Without patches 1 and 2, all three new tests fail on every build.

- On x86_64, a separate program opening a file with each of the twelve
  C mode strings now gets the same access mode from nolibc as from
  glibc, and the same errno for ENOENT and EACCES failures.

An LLM was used to help with this series.

Danish Khateeb (3):
  tools/nolibc: fix fopen() errno when open() fails
  tools/nolibc: fix fopen() with the "rb+", "wb+" and "ab+" modes
  selftests/nolibc: test fopen() errors and update modes

 tools/include/nolibc/stdio.h                 |  5 +++-
 tools/testing/selftests/nolibc/nolibc-test.c | 24 ++++++++++++++++++++
 2 files changed, 28 insertions(+), 1 deletion(-)


base-commit: 700ce4da6f6f172435349b687d2d916e791c6330
-- 
2.56.0

[PATCH 1/3] tools/nolibc: fix fopen() errno when open() fails

[Danish Khateeb]

When open() fails, fopen() passes its -1 on to fdopen(), which sets
errno to EBADF and so overwrites the error from open(). A missing file
is reported as EBADF instead of ENOENT, and a file without permission as
EBADF instead of EACCES.

Return NULL as soon as open() fails, leaving its errno in place.

Fixes: a009a0c6faa9 ("tools/nolibc: add fopen()")
Signed-off-by: Danish Khateeb <danishkhateeb03@gmail.com>
---
 tools/include/nolibc/stdio.h | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/tools/include/nolibc/stdio.h b/tools/include/nolibc/stdio.h
index ebdd413d13ec..00a95a267d90 100644
--- a/tools/include/nolibc/stdio.h
+++ b/tools/include/nolibc/stdio.h
@@ -79,6 +79,9 @@ FILE *fopen(const char *pathname, const char *mode)
 		flags = (flags & ~(O_RDONLY | O_WRONLY)) | O_RDWR;
 
 	fd = open(pathname, flags, 0666);
+	if (fd < 0)
+		return NULL;
+
 	return fdopen(fd, mode);
 }
 
-- 
2.56.0

[PATCH 2/3] tools/nolibc: fix fopen() with the "rb+", "wb+" and "ab+" modes

[Danish Khateeb]

The C standard allows the '+' of an update mode to come before or after
the 'b', so "rb+" means the same as "r+b". fopen() only looks for the
'+' right after the first character, so it opens a file read-only in
"rb+" mode, and write-only in "wb+" and "ab+" modes. Writing to such a
stream, or reading from it, then fails with EBADF.

Look for the '+' anywhere in the mode string.

Fixes: a009a0c6faa9 ("tools/nolibc: add fopen()")
Signed-off-by: Danish Khateeb <danishkhateeb03@gmail.com>
---
 tools/include/nolibc/stdio.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tools/include/nolibc/stdio.h b/tools/include/nolibc/stdio.h
index 00a95a267d90..205b06f18b4b 100644
--- a/tools/include/nolibc/stdio.h
+++ b/tools/include/nolibc/stdio.h
@@ -75,7 +75,7 @@ FILE *fopen(const char *pathname, const char *mode)
 		SET_ERRNO(EINVAL); return NULL;
 	}
 
-	if (mode[1] == '+')
+	if (strchr(mode, '+'))
 		flags = (flags & ~(O_RDONLY | O_WRONLY)) | O_RDWR;
 
 	fd = open(pathname, flags, 0666);
-- 
2.56.0

[PATCH 3/3] selftests/nolibc: test fopen() errors and update modes

[Danish Khateeb]

Check that fopen() of a missing file fails with ENOENT, and that a
stream opened in "rb+" or "wb+" mode can be both read and written.

Signed-off-by: Danish Khateeb <danishkhateeb03@gmail.com>
---
 tools/testing/selftests/nolibc/nolibc-test.c | 24 ++++++++++++++++++++
 1 file changed, 24 insertions(+)

diff --git a/tools/testing/selftests/nolibc/nolibc-test.c b/tools/testing/selftests/nolibc/nolibc-test.c
index f3d0d27cdbe1..be501b5298d7 100644
--- a/tools/testing/selftests/nolibc/nolibc-test.c
+++ b/tools/testing/selftests/nolibc/nolibc-test.c
@@ -1035,6 +1035,27 @@ int test_file_stream_wsr(void)
 	return 0;
 }
 
+/* checks that a stream opened with <mode> is readable and writable */
+int test_fopen_rw(const char *mode)
+{
+	FILE *f;
+	char c;
+	int r;
+
+	f = fopen("/dev/null", mode);
+	if (!f)
+		return -1;
+
+	/* reading /dev/null hits end-of-file, so a write may follow */
+	errno = 0;
+	r = fread(&c, 1, 1, f) != 0 || errno || fwrite("x", 1, 1, f) != 1;
+
+	if (fclose(f) == EOF)
+		return -1;
+
+	return r ? -1 : 0;
+}
+
 enum fork_type {
 	FORK_STANDARD,
 	FORK_VFORK,
@@ -1759,6 +1780,9 @@ int run_syscall(int min, int max)
 		CASE_TEST(fdopendir_notdir);  EXPECT_SYSER(1, (uintptr_t)fdopendir(STDIN_FILENO), (uintptr_t)NULL, ENOTDIR); break;
 		CASE_TEST(file_stream);       EXPECT_SYSZR(1, test_file_stream()); break;
 		CASE_TEST(file_stream_wsr);   EXPECT_SYSZR(1, test_file_stream_wsr()); break;
+		CASE_TEST(fopen_blah);        EXPECT_PTRER(1, fopen("/proc/self/blah", "r"), NULL, ENOENT); break;
+		CASE_TEST(fopen_rb_plus);     EXPECT_SYSZR(1, test_fopen_rw("rb+")); break;
+		CASE_TEST(fopen_wb_plus);     EXPECT_SYSZR(1, test_fopen_rw("wb+")); break;
 		CASE_TEST(fork);              EXPECT_SYSZR(1, test_fork(FORK_STANDARD)); break;
 		CASE_TEST(ftruncate);         EXPECT_SYSZR(1, test_ftruncate()); break;
 		CASE_TEST(getdents64_root);   EXPECT_SYSNE(1, test_getdents64("/"), -1); break;
-- 
2.56.0