tools: acrn-crashlog: remove unsafe apis in usercrash

Since strlen/vsnprintf/ato* api are not safe, so use strnlen instead of
strlen, use vasprintf instead of vsnprintf and use strtol instead of
atoi.

Tracked-On: #1254
Signed-off-by: xiaojin2 <xiaojing.liu@intel.com>
Reviewed-by: Huang Yonghua <yonghua.huang@intel.com>
Reviewed-by: Liu, Xinwu <xinwu.liu@intel.com>
Acked-by: Chen Gang <gang.c.chen@intel.com>

Author: xiaojingrich

tools/acrn-crashlog/usercrash/client.c

@@ -85,7 +85,8 @@ static int usercrashd_connect(int pid, int *usercrashd_socket,
 		LOGE("crash process name is NULL\n");
 		return -1;
 	}
-	sockfd = socket_local_client(SOCKET_NAME, SOCK_SEQPACKET);
+	sockfd = socket_local_client(SOCKET_NAME, strlen(SOCKET_NAME),
+			SOCK_SEQPACKET);
 	if (sockfd == -1) {
 		LOGE("failed to connect to usercrashd, error (%s)\n",
 			strerror(errno));
@@ -212,8 +213,8 @@ int main(int argc, char *argv[])
 
 	if (argc == 4) {
 		/* it's from coredump */
-		pid = atoi(argv[1]);
-		sig = atoi(argv[3]);
+		pid = (int)strtol(argv[1], NULL, 10);
+		sig = (int)strtol(argv[3], NULL, 10);
 		ret = usercrashd_connect(pid, &sock, &out_fd, argv[2]);
 		if (ret) {
 			LOGE("usercrashd_connect failed, error (%s)\n",

tools/acrn-crashlog/usercrash/crash_dump.c

@@ -24,27 +24,29 @@
 #define DUMP_FILE "/tmp/core"
 #define BUFFER_SIZE 8196
 #define LINK_LEN 512
+#define TID "Tgid:"
 /* 128 means the length of the DUMP_FILE */
 #define FORMAT_LENGTH (LINK_LEN + 128)
 
 static void loginfo(int fd, const char *fmt, ...)
 {
-	char buf[512];
+	char *buf;
 	va_list ap;
-	size_t len, ret;
+	int len, ret;
 
 	va_start(ap, fmt);
-	vsnprintf(buf, sizeof(buf), fmt, ap);
-	va_end(ap);
-
-	len = strlen(buf);
-	if (len <= 0)
+	len = vasprintf(&buf, fmt, ap);
+	if (len == -1) {
+		LOGE("write to buf failed\n");
 		return;
+	}
+	va_end(ap);
 
 	ret = write(fd, buf, len);
 	if (ret != len) {
 		LOGE("write in loginfo failed\n");
 	}
+	free(buf);
 }
 
 static const char *get_signame(int sig)
@@ -249,7 +251,7 @@ static int save_usercrash_file(int pid, int tgid, const char *comm,
 }
 
 static int get_key_value(int pid, const char *path, const char *key,
-		char *value, size_t value_len)
+		const size_t klen, char *value, const size_t value_len)
 {
 	int len;
 	int ret;
@@ -276,7 +278,7 @@ static int get_key_value(int pid, const char *path, const char *key,
 	end = strchr(msg, '\n');
 	if (end == NULL)
 		end = data + size;
-	start = msg + strlen(key);
+	start = msg + klen;
 	len = end - start;
 	if (len >= (int)value_len) {
 		free(data);
@@ -312,12 +314,13 @@ void crash_dump(int pid, int sig, int out_fd)
 	}
 	comm[ret] = '\0';
 
-	ret = get_key_value(pid, GET_TID, "Tgid:", result, sizeof(result));
+	ret = get_key_value(pid, GET_TID, TID, strlen(TID),
+			result, sizeof(result));
 	if (ret) {
 		LOGE("get Tgid error\n");
 		return;
 	}
-	tgid = atoi(result);
+	tgid = (int)strtol(result, NULL, 10);
 	if (!sig)
 		sig = DEBUGGER_SIGNAL;
 	if (save_usercrash_file(pid, tgid, comm, sig, out_fd))

tools/acrn-crashlog/usercrash/debugger.c

@@ -53,7 +53,7 @@ int main(int argc, char *argv[])
 
 	if (argc == 2) {
 		/* it's from shell cmd */
-		pid = atoi(argv[1]);
+		pid = (int)strtol(argv[1], NULL, 10);
 		crash_dump(pid, 0, STDOUT_FILENO);
 	} else {
 		print_usage();

tools/acrn-crashlog/usercrash/include/protocol.h

@@ -14,10 +14,11 @@
 #include <sys/cdefs.h>
 
 #define RESERVED_SOCKET_PREFIX "/tmp/"
+#define SOCKET_PATH_MAX 128
 
-int linux_get_control_socket(const char *name);
+int create_socket_server(const char *name, int type);
 
-int socket_local_client(const char *name, int type);
+int socket_local_client(const char *name, const size_t len, int type);
 ssize_t send_fd(int sockfd, const void *data, size_t len, int fd);
 ssize_t recv_fd(int sockfd, void *data, size_t len, int *out_fd);
 

tools/acrn-crashlog/usercrash/protocol.c

@@ -31,23 +31,21 @@
 	(sizeof(struct sockaddr_un) - offsetof(struct sockaddr_un, sun_path))
 
 /* Documented in header file. */
-static int socket_make_sockaddr_un(const char *name,
+static int socket_make_sockaddr_un(const char *name, const size_t len,
 		struct sockaddr_un *p_addr, socklen_t *alen)
 {
-	size_t name_len;
 	size_t socket_len;
 
 	socket_len = strlen(RESERVED_SOCKET_PREFIX);
 	if (socket_len >= SUN_PATH_MAX)
 		return -1;
 	strncpy(p_addr->sun_path, RESERVED_SOCKET_PREFIX, socket_len + 1);
-	name_len = strlen(name);
-	if (name_len >= (SUN_PATH_MAX - socket_len))
+	if (len >= (SUN_PATH_MAX - socket_len))
 		return -1;
-	strncat(p_addr->sun_path, name, name_len);
+	strncat(p_addr->sun_path, name, len);
 
 	p_addr->sun_family = AF_LOCAL;
-	*alen = name_len + socket_len +
+	*alen = len + socket_len +
 		offsetof(struct sockaddr_un, sun_path) + 1;
 	return 0;
 }
@@ -58,13 +56,14 @@ static int socket_make_sockaddr_un(const char *name,
  * fd is not closed on error. that's your job.
  *
  */
-static int socket_local_client_connect(int fd, const char *name)
+static int socket_local_client_connect(int fd, const char *name,
+		const size_t len)
 {
 	struct sockaddr_un addr;
 	socklen_t alen;
 	int err;
 
-	err = socket_make_sockaddr_un(name, &addr, &alen);
+	err = socket_make_sockaddr_un(name, len, &addr, &alen);
 
 	if (err < 0)
 		goto error;
@@ -85,15 +84,15 @@ static int socket_local_client_connect(int fd, const char *name)
  * connect to peer named "name"
  * returns fd or -1 on error
  */
-int socket_local_client(const char *name, int type)
+int socket_local_client(const char *name, const size_t len, int type)
 {
 	int s;
 
 	s = socket(AF_LOCAL, type, 0);
 	if (s < 0)
 		return -1;
 
-	if (socket_local_client_connect(s, name) < 0) {
+	if (socket_local_client_connect(s, name, len) < 0) {
 		close(s);
 		return -1;
 	}
@@ -108,20 +107,20 @@ static int socket_bind(int fd, const char *name)
 	size_t name_len;
 
 	addr.sun_family = AF_UNIX;
-	name_len = strlen(name);
+	name_len = strnlen(name, SOCKET_PATH_MAX);
 	if (name_len >= SUN_PATH_MAX)
 		return -1;
 	strncpy(addr.sun_path, name, name_len + 1);
 	unlink(addr.sun_path);
-	alen = strlen(addr.sun_path) + sizeof(addr.sun_family);
+	alen = strnlen(addr.sun_path, SUN_PATH_MAX) + sizeof(addr.sun_family);
 
 	if (bind(fd, (struct sockaddr *)&addr, alen) == -1)
 		return -1;
 
 	return fd;
 }
 
-static int create_socket_server(const char *name, int type)
+int create_socket_server(const char *name, int type)
 {
 	int err;
 	int fd;
@@ -140,15 +139,6 @@ static int create_socket_server(const char *name, int type)
 	return fd;
 }
 
-int linux_get_control_socket(const char *name)
-{
-	char socketName[64];
-
-	snprintf(socketName, sizeof(socketName), RESERVED_SOCKET_PREFIX "%s",
-				name);
-	return create_socket_server(socketName, SOCK_SEQPACKET);
-}
-
 ssize_t send_fd(int sockfd, const void *data, size_t len, int fd)
 {
 	char cmsg_buf[CMSG_SPACE(sizeof(int))];

tools/acrn-crashlog/usercrash/server.c

@@ -43,6 +43,7 @@
 #include "protocol.h"
 #include "log_sys.h"
 #include "version.h"
+#include "strutils.h"
 
 #define FILE_PATH_LEN_MAX 256
 
@@ -377,9 +378,10 @@ static void print_usage(void)
 
 int main(int argc, char *argv[])
 {
-	char socket_path[128];
+	char socket_path[SOCKET_PATH_MAX];
 	DIR *dir;
 	int fd;
+	int len;
 	evutil_socket_t crash_socket;
 	int opt;
 	struct sigaction action;
@@ -434,22 +436,26 @@ int main(int argc, char *argv[])
 	debuggerd_register_handlers(&action);
 
 	find_oldest_usercrash();
-
+	len = snprintf(socket_path, sizeof(socket_path), "%s/%s",
+			RESERVED_SOCKET_PREFIX, SOCKET_NAME);
+	if (s_not_expect(len , sizeof(socket_path))) {
+		LOGE("construct socket path error\n");
+		exit(EXIT_FAILURE);
+	}
 	/**
 	 * evutil_socket_t on other platform except WIN32 platform is int
 	 * type, but on WIN32 platform evutil_socket_t is intptr_t type.
 	 * So, considering compatibility, here need to transfer socket fd to
 	 * evutil_socket_t type.
 	 */
-	crash_socket = (evutil_socket_t)linux_get_control_socket(SOCKET_NAME);
+	crash_socket = (evutil_socket_t)create_socket_server(socket_path,
+			SOCK_SEQPACKET);
 
 	if (crash_socket == -1) {
 		LOGE("failed to get socket from init, error (%s)\n",
 					strerror(errno));
 		exit(EXIT_FAILURE);
 	}
-	snprintf(socket_path, sizeof(socket_path), "%s/%s",
-			RESERVED_SOCKET_PREFIX, SOCKET_NAME);
 	if (chmod(socket_path, 0622) == -1) {
 		LOGE("failed to change usercrashd_crash priority\n");
 		goto fail;