[LTP] [PATCH] Fix getcwd03 testcase by zeroing target buffer for readlink()

[Helge Deller <deller@gmx.de>]

* Cyril Hrubis <chrubis@suse.cz>:
> Hi!
> > According to the man(2) page of readlink(), a null byte is not appended to the
> > target buffer. So applications need to make sure that the target buffer is
> > zero-initialized, otherwise random bytes at the end of the returned string may
> > exist. 
> 
> Good catch.
> 
> > This patch zero-initializes the on-stack char array "link" and thus fixes the
> > testcase failure of getcwd03 on the hppa/parisc architecture (and maybe others).
> 
> Can we use the return value form the readlink instead?
> 
> 	r = SAFE_READLINK(.., buf, sizeof(buf)-1);
> 	buf[r] = '\0';

Yes, that's possible.
But we have in total three testcase which exhibit the same problem:

testcases/kernel/syscalls/openat/openat03.c:    SAFE_READLINK(cleanup, path, tmp, PATH_MAX);
testcases/kernel/syscalls/getcwd/getcwd03.c:    SAFE_READLINK(dir_link, link, sizeof(link));
testcases/kernel/syscalls/open/open14.c:        SAFE_READLINK(cleanup, path, tmp, PATH_MAX);

All of those use a temporary un-initialized buffer on the stack, so all
would need this fix. On the hppa architecture I found those testcases
failing randomly.

> The SAFE_READLINK() does catch the case where readlink() returns -1 and
> passing size decreased by one should handle the corner case where the
> path is truncated and return value is the size of the buffer we passed
> to the readlink() call.

This still doesn't zero-terminate the buffer (which is on stack).

Instead I'd propose the patch below. It always zero-terminates the
buffer and as such we would handle newly added testcases in future which
will probably miss this readlink behaviour too.

Helge

----------------

[PATCH] SAFE_READLINK() should return zero-terminated target buffer

According to the man(2) page of readlink(), a null byte is not appended
to the target buffer. So applications need to make sure that the target
buffer is zero-initialized, otherwise random bytes at the end of the
returned string may exist.

Currently all users of SAFE_READLINK() get it wrong. This include the
openat03, getcwd03 and open14 testcases which pass a temporary,
un-initialized on-stack buffer to readlink().

This patch fixes SAFE_READLINK to always return a zero-terminated string
and thus fixes the the failure of getcwd03 on the hppa/parisc
architecture (and probably others).

Signed-off-by: Helge Deller <deller@gmx.de>

diff --git a/lib/safe_macros.c b/lib/safe_macros.c
index bffc5a17a..1267b0098 100644
--- a/lib/safe_macros.c
+++ b/lib/safe_macros.c
@@ -408,9 +408,17 @@ ssize_t safe_readlink(const char *file, const int lineno,
 	rval = readlink(path, buf, bufsize);
 
 	if (rval == -1) {
+		buf[0] = 0; /* clear buffer on error */
 		tst_brkm(TBROK | TERRNO, cleanup_fn,
 			 "%s:%d: readlink(%s,%p,%zu) failed",
 			 file, lineno, path, buf, bufsize);
+	} else {
+		/* readlink does not append a null byte to the buffer.
+		 * Add it now. */
+		if ((size_t) rval < bufsize)
+			buf[rval] = 0;
+		else
+			buf[bufsize-1] = 0;
 	}
 
 	return rval;