[CRIU] Re: [PATCH cr] proc: add a static buffer to prevent segv (v3)

[Cyrill Gorcunov]

On Wed, May 02, 2012 at 03:22:14PM +0400, Andrey Vagin wrote:
> A few of our functions use buffer and string functions.
> All these functions require that a string contains '\0' at the end.
> Before this patch we didn't guarantee that.
> 
> I've seen segmentation fault in parse_pid_stat_small.
> 
> v2: simplify code
> v3: remove assignment, because it's redundant and a size of object file
> become bigger.

OK. Andrey, I'm personally fine with either fix (though I would
prefer it to be like below but still if Pavel prefer the former
one -- I'm fine :)

	Cyrill
---
From: Cyrill Gorcunov <gorcunov at openvz.org>
Date: Wed, 2 May 2012 15:23:08 +0400
Subject: [PATCH] proc_parse: Use read_string helper to make sure EOS is present in data read

Otherwise strrchr might cross memory data granted for program
and cause sigsegv.

Reported-by: Andrey Vagin <avagin at openvz.org>
Signed-off-by: Cyrill Gorcunov <gorcunov at openvz.org>
---
 include/util.h |   10 ++++++++++
 proc_parse.c   |    5 ++---
 2 files changed, 12 insertions(+), 3 deletions(-)

diff --git a/include/util.h b/include/util.h
index 18933f3..a3ca01c 100644
--- a/include/util.h
+++ b/include/util.h
@@ -162,6 +162,16 @@ extern void pr_vma(unsigned int loglevel, const struct vma_area *vma_area);
 		p__;						\
 	})
 
+#define read_eos(fd, buf, size)					\
+	({							\
+		int r__ = read(fd, buf, size);			\
+		if (buf[(size) - 1])				\
+			buf[(size) - 1] = '\0';			\
+		r__;						\
+	})
+
+#define read_string(fd, buf)	read_eos(fd, buf, sizeof(buf))
+
 extern int move_img_fd(int *img_fd, int want_fd);
 extern int close_safe(int *fd);
 
diff --git a/proc_parse.c b/proc_parse.c
index c574d31..d50b8af 100644
--- a/proc_parse.c
+++ b/proc_parse.c
@@ -194,8 +194,7 @@ int parse_pid_stat_small(pid_t pid, struct proc_pid_stat_small *s)
 	fd = open_proc(pid, "stat");
 	if (fd < 0)
 		return -1;
-
-	n = read(fd, buf, sizeof(buf));
+	n = read_string(fd, buf);
 	if (n < 1) {
 		pr_err("stat for %d is corrupted\n", pid);
 		close(fd);
@@ -244,7 +243,7 @@ int parse_pid_stat(pid_t pid, struct proc_pid_stat *s)
 	if (fd < 0)
 		return -1;
 
-	n = read(fd, buf, sizeof(buf));
+	n = read_string(fd, buf);
 	if (n < 1) {
 		pr_err("stat for %d is corrupted\n", pid);
 		close(fd);
-- 
1.7.7.6