[PATCH V6 1/3] multipath-tools: New way to limit the IPC command length.

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Problem:

    mpath_recv_reply() return -EINVAL on command 'show maps json' with 2k paths.

Root cause:

    Commit 174e717d351789a3cb29e1417f8e910baabcdb16 introduced the
    limitation on max bytes(65535) of reply string from multipathd.
    With 2k paths(1k mpaths) simulated by scsi_debug, the 'show maps json'
    requires 1633217 bytes which trigged the EINVAL error.

Fix:
    * Remove the limitation of MAX_REPLY_LEN in libmpathcmd.

    * Remove uxsock.h from libmultipath, changed all non-daemon usage to
      libmpathcmd instead.

    * Rename send_packet() to send_packet_daemon_only() which is
      dedicated for multipathd socket listener.

    * Rename recv_packet() to recv_packet_daemon_only() which is
      dedicate for multipathd socket listener.

    * Enforce limitation(255) of IPC command string in
      recv_packet_daemon_only().

    * Removed unused read_all() from uxsock.h.

    * Moved write_all() to file.h of libmultipath as all usage of
      write_all() is against file rather than socket.

Changes caused by patch:

    * Data sent from IPC client(including multipathd -k) to multipathd
      is limited to 255 bytes maximum. This prevent malicious IPC client
      send something like 'fffffffffffffffffake command' to daemon which
      lead daemon to allocate a big chunk of memory.

    * Due to the removal of uxsock.h from libmultipath, all IPC connection
      except (multipathd daemon) should use libmpathcmd instead.

Signed-off-by: Gris Ge <fge@xxxxxxxxxx>
---
 libmpathcmd/mpath_cmd.c               |  2 -
 libmpathcmd/mpath_cmd.h               |  2 -
 libmpathpersist/mpath_updatepr.c      |  6 +--
 libmultipath/Makefile                 |  2 +-
 libmultipath/alias.c                  |  1 -
 libmultipath/configure.c              |  5 +--
 libmultipath/file.c                   | 24 +++++++++++-
 libmultipath/file.h                   |  1 +
 libmultipath/uxsock.h                 |  6 ---
 libmultipath/wwids.c                  |  1 -
 multipath/main.c                      |  1 -
 multipathd/Makefile                   |  2 +-
 multipathd/uxclnt.c                   | 13 +++----
 multipathd/uxlsnr.c                   | 12 +++---
 {libmultipath => multipathd}/uxsock.c | 69 ++++-------------------------------
 multipathd/uxsock.h                   | 13 +++++++
 16 files changed, 63 insertions(+), 97 deletions(-)
 delete mode 100644 libmultipath/uxsock.h
 rename {libmultipath => multipathd}/uxsock.c (67%)
 create mode 100644 multipathd/uxsock.h

diff --git a/libmpathcmd/mpath_cmd.c b/libmpathcmd/mpath_cmd.c
index 2290ecb..1aaf5ea 100644
--- a/libmpathcmd/mpath_cmd.c
+++ b/libmpathcmd/mpath_cmd.c
@@ -142,8 +142,6 @@ int mpath_recv_reply(int fd, char **reply, unsigned int timeout)
 	len = mpath_recv_reply_len(fd, timeout);
 	if (len <= 0)
 		return len;
-	if (len > MAX_REPLY_LEN)
-		return -EINVAL;
 	*reply = malloc(len);
 	if (!*reply)
 		return -1;
diff --git a/libmpathcmd/mpath_cmd.h b/libmpathcmd/mpath_cmd.h
index 92382e2..5ce300d 100644
--- a/libmpathcmd/mpath_cmd.h
+++ b/libmpathcmd/mpath_cmd.h
@@ -26,8 +26,6 @@ extern "C" {
 
 #define DEFAULT_SOCKET		"/org/kernel/linux/storage/multipathd"
 #define DEFAULT_REPLY_TIMEOUT	1000
-#define MAX_REPLY_LEN		65536
-
 
 /*
  * DESCRIPTION:
diff --git a/libmpathpersist/mpath_updatepr.c b/libmpathpersist/mpath_updatepr.c
index 0529d13..56736b7 100644
--- a/libmpathpersist/mpath_updatepr.c
+++ b/libmpathpersist/mpath_updatepr.c
@@ -7,13 +7,11 @@
 #include <fcntl.h>
 #include <sys/ioctl.h>
 #include <sys/types.h>
-#include <sys/socket.h>
 #include <sys/un.h>
 #include <sys/poll.h>
 #include <errno.h>
 #include <debug.h>
 #include <mpath_cmd.h>
-#include <uxsock.h>
 #include "memory.h"
 
 unsigned long mem_allocated;    /* Total memory used in Bytes */
@@ -33,12 +31,12 @@ int update_prflag(char * arg1, char * arg2, int noisy)
 
 	snprintf(str,sizeof(str),"map %s %s", arg1, arg2);
 	condlog (2, "%s: pr flag message=%s", arg1, str);
-	if (send_packet(fd, str) != 0) {
+	if (mpath_send_cmd(fd, str) != 0) {
 		condlog(2, "%s: message=%s send error=%d", arg1, str, errno);
 		mpath_disconnect(fd);
 		return -2;
 	}
-	ret = recv_packet(fd, &reply, DEFAULT_REPLY_TIMEOUT);
+	ret = mpath_recv_reply(fd, &reply, DEFAULT_REPLY_TIMEOUT);
 	if (ret < 0) {
 		condlog(2, "%s: message=%s recv error=%d", arg1, str, errno);
 		ret = -2;
diff --git a/libmultipath/Makefile b/libmultipath/Makefile
index a14d4b3..eabeef0 100644
--- a/libmultipath/Makefile
+++ b/libmultipath/Makefile
@@ -21,7 +21,7 @@ OBJS = memory.o parser.o vector.o devmapper.o callout.o \
        hwtable.o blacklist.o util.o dmparser.o config.o \
        structs.o discovery.o propsel.o dict.o \
        pgpolicies.o debug.o defaults.o uevent.o \
-       switchgroup.o uxsock.o print.o alias.o log_pthread.o \
+       switchgroup.o print.o alias.o log_pthread.o \
        log.o configure.o structs_vec.o sysfs.o prio.o checkers.o \
        lock.o waiter.o file.o wwids.o prioritizers/alua_rtpg.o
 
diff --git a/libmultipath/alias.c b/libmultipath/alias.c
index b86843a..2f08992 100644
--- a/libmultipath/alias.c
+++ b/libmultipath/alias.c
@@ -10,7 +10,6 @@
 #include <stdio.h>
 
 #include "debug.h"
-#include "uxsock.h"
 #include "alias.h"
 #include "file.h"
 #include "vector.h"
diff --git a/libmultipath/configure.c b/libmultipath/configure.c
index a9b9cf0..a9bcf63 100644
--- a/libmultipath/configure.c
+++ b/libmultipath/configure.c
@@ -37,7 +37,6 @@
 #include "alias.h"
 #include "prio.h"
 #include "util.h"
-#include "uxsock.h"
 #include "wwids.h"
 
 /* group paths in pg by host adapter
@@ -727,12 +726,12 @@ int check_daemon(void)
 	if (fd == -1)
 		return 0;
 
-	if (send_packet(fd, "show daemon") != 0)
+	if (mpath_send_cmd(fd, "show daemon") != 0)
 		goto out;
 	conf = get_multipath_config();
 	timeout = conf->uxsock_timeout;
 	put_multipath_config(conf);
-	if (recv_packet(fd, &reply, timeout) != 0)
+	if (mpath_recv_reply(fd, &reply, conf->uxsock_timeout) != 0)
 		goto out;
 
 	if (strstr(reply, "shutdown"))
diff --git a/libmultipath/file.c b/libmultipath/file.c
index 74cde64..b5b58b7 100644
--- a/libmultipath/file.c
+++ b/libmultipath/file.c
@@ -15,7 +15,6 @@
 
 #include "file.h"
 #include "debug.h"
-#include "uxsock.h"
 
 
 /*
@@ -178,3 +177,26 @@ fail:
 	close(fd);
 	return -1;
 }
+
+/*
+ * keep writing until it's all sent
+ */
+size_t write_all(int fd, const void *buf, size_t len)
+{
+	size_t total = 0;
+
+	while (len) {
+		ssize_t n = write(fd, buf, len);
+		if (n < 0) {
+			if ((errno == EINTR) || (errno == EAGAIN))
+				continue;
+			return total;
+		}
+		if (!n)
+			return total;
+		buf = n + (char *)buf;
+		len -= n;
+		total += n;
+	}
+	return total;
+}
diff --git a/libmultipath/file.h b/libmultipath/file.h
index 4f96dbf..5ea9bd3 100644
--- a/libmultipath/file.h
+++ b/libmultipath/file.h
@@ -7,5 +7,6 @@
 
 #define FILE_TIMEOUT 30
 int open_file(char *file, int *can_write, char *header);
+size_t write_all(int fd, const void *buf, size_t len);
 
 #endif /* _FILE_H */
diff --git a/libmultipath/uxsock.h b/libmultipath/uxsock.h
deleted file mode 100644
index c1cf81f..0000000
--- a/libmultipath/uxsock.h
+++ /dev/null
@@ -1,6 +0,0 @@
-/* some prototypes */
-int ux_socket_listen(const char *name);
-int send_packet(int fd, const char *buf);
-int recv_packet(int fd, char **buf, unsigned int timeout);
-size_t write_all(int fd, const void *buf, size_t len);
-ssize_t read_all(int fd, void *buf, size_t len, unsigned int timeout);
diff --git a/libmultipath/wwids.c b/libmultipath/wwids.c
index a7c3249..c0d7d79 100644
--- a/libmultipath/wwids.c
+++ b/libmultipath/wwids.c
@@ -10,7 +10,6 @@
 #include "vector.h"
 #include "structs.h"
 #include "debug.h"
-#include "uxsock.h"
 #include "file.h"
 #include "wwids.h"
 #include "defaults.h"
diff --git a/multipath/main.c b/multipath/main.c
index 907a96c..ae667d0 100644
--- a/multipath/main.c
+++ b/multipath/main.c
@@ -56,7 +56,6 @@
 #include <sys/time.h>
 #include <sys/resource.h>
 #include <wwids.h>
-#include <uxsock.h>
 #include <mpath_cmd.h>
 
 int logsink;
diff --git a/multipathd/Makefile b/multipathd/Makefile
index 1552458..d4c4aae 100644
--- a/multipathd/Makefile
+++ b/multipathd/Makefile
@@ -31,7 +31,7 @@ LDFLAGS += -ludev -ldl \
 #
 # object files
 #
-OBJS = main.o pidfile.o uxlsnr.o uxclnt.o cli.o cli_handlers.o
+OBJS = main.o pidfile.o uxlsnr.o uxclnt.o cli.o cli_handlers.o uxsock.o
 
 
 #
diff --git a/multipathd/uxclnt.c b/multipathd/uxclnt.c
index 37afaac..683714c 100644
--- a/multipathd/uxclnt.c
+++ b/multipathd/uxclnt.c
@@ -19,7 +19,6 @@
 #include <readline/history.h>
 
 #include <mpath_cmd.h>
-#include <uxsock.h>
 #include <memory.h>
 #include <defaults.h>
 
@@ -85,8 +84,8 @@ static void process(int fd, unsigned int timeout)
 		if (need_quit(line, llen))
 			break;
 
-		if (send_packet(fd, line) != 0) break;
-		ret = recv_packet(fd, &reply, timeout);
+		if (mpath_send_cmd(fd, line) != 0) break;
+		ret = mpath_recv_reply(fd, &reply, timeout);
 		if (ret != 0) break;
 
 		print_reply(reply);
@@ -104,16 +103,16 @@ static void process_req(int fd, char * inbuf, unsigned int timeout)
 	char *reply;
 	int ret;
 
-	if (send_packet(fd, inbuf) != 0) {
+	if (mpath_send_cmd(fd, inbuf) != 0) {
 		printf("cannot send packet\n");
 		return;
 	}
-	ret = recv_packet(fd, &reply, timeout);
+	ret = mpath_recv_reply(fd, &reply, timeout);
 	if (ret < 0) {
-		if (ret == -ETIMEDOUT)
+		if (errno == -ETIMEDOUT)
 			printf("timeout receiving packet\n");
 		else
-			printf("error %d receiving packet\n", ret);
+			printf("error %d receiving packet\n", errno);
 	} else {
 		printf("%s", reply);
 		FREE(reply);
diff --git a/multipathd/uxlsnr.c b/multipathd/uxlsnr.c
index abd1486..2b38868 100644
--- a/multipathd/uxlsnr.c
+++ b/multipathd/uxlsnr.c
@@ -28,7 +28,6 @@
 #include <vector.h>
 #include <structs.h>
 #include <structs_vec.h>
-#include <uxsock.h>
 #include <defaults.h>
 #include <config.h>
 #include <mpath_cmd.h>
@@ -36,6 +35,7 @@
 #include "main.h"
 #include "cli.h"
 #include "uxlsnr.h"
+#include "uxsock.h"
 
 struct timespec sleep_time = {5, 0};
 
@@ -234,8 +234,9 @@ void * uxsock_listen(uxsock_trigger_fn uxsock_trigger, void * trigger_data)
 				}
 				if (gettimeofday(&start_time, NULL) != 0)
 					start_time.tv_sec = 0;
-				if (recv_packet(c->fd, &inbuf,
-						uxsock_timeout) != 0) {
+				if (recv_packet_daemon_only(c->fd, &inbuf,
+							    uxsock_timeout)
+				    != 0) {
 					dead_client(c);
 					continue;
 				}
@@ -244,8 +245,9 @@ void * uxsock_listen(uxsock_trigger_fn uxsock_trigger, void * trigger_data)
 				uxsock_trigger(inbuf, &reply, &rlen,
 					       trigger_data);
 				if (reply) {
-					if (send_packet(c->fd,
-							reply) != 0) {
+					if (send_packet_daemon_only(c->fd,
+								    reply)
+					    != 0) {
 						dead_client(c);
 					} else {
 						condlog(4, "cli[%d]: "
diff --git a/libmultipath/uxsock.c b/multipathd/uxsock.c
similarity index 67%
rename from libmultipath/uxsock.c
rename to multipathd/uxsock.c
index e91abd9..2784051 100644
--- a/libmultipath/uxsock.c
+++ b/multipathd/uxsock.c
@@ -24,6 +24,9 @@
 #include "memory.h"
 #include "uxsock.h"
 #include "debug.h"
+
+#define _MAX_IPC_CMD_LEN	255
+
 /*
  * create a unix domain socket and start listening on it
  * return a file descriptor open on the socket
@@ -74,69 +77,9 @@ int ux_socket_listen(const char *name)
 }
 
 /*
- * keep writing until it's all sent
- */
-size_t write_all(int fd, const void *buf, size_t len)
-{
-	size_t total = 0;
-
-	while (len) {
-		ssize_t n = write(fd, buf, len);
-		if (n < 0) {
-			if ((errno == EINTR) || (errno == EAGAIN))
-				continue;
-			return total;
-		}
-		if (!n)
-			return total;
-		buf = n + (char *)buf;
-		len -= n;
-		total += n;
-	}
-	return total;
-}
-
-/*
- * keep reading until its all read
- */
-ssize_t read_all(int fd, void *buf, size_t len, unsigned int timeout)
-{
-	size_t total = 0;
-	ssize_t n;
-	int ret;
-	struct pollfd pfd;
-
-	while (len) {
-		pfd.fd = fd;
-		pfd.events = POLLIN;
-		ret = poll(&pfd, 1, timeout);
-		if (!ret) {
-			return -ETIMEDOUT;
-		} else if (ret < 0) {
-			if (errno == EINTR)
-				continue;
-			return -errno;
-		} else if (!pfd.revents & POLLIN)
-			continue;
-		n = read(fd, buf, len);
-		if (n < 0) {
-			if ((errno == EINTR) || (errno == EAGAIN))
-				continue;
-			return -errno;
-		}
-		if (!n)
-			return total;
-		buf = n + (char *)buf;
-		len -= n;
-		total += n;
-	}
-	return total;
-}
-
-/*
  * send a packet in length prefix format
  */
-int send_packet(int fd, const char *buf)
+int send_packet_daemon_only(int fd, const char *buf)
 {
 	int ret = 0;
 	sigset_t set, old;
@@ -157,7 +100,7 @@ int send_packet(int fd, const char *buf)
 /*
  * receive a packet in length prefix format
  */
-int recv_packet(int fd, char **buf, unsigned int timeout)
+int recv_packet_daemon_only(int fd, char **buf, unsigned int timeout)
 {
 	int err;
 	ssize_t len;
@@ -166,6 +109,8 @@ int recv_packet(int fd, char **buf, unsigned int timeout)
 	len = mpath_recv_reply_len(fd, timeout);
 	if (len <= 0)
 		return len;
+	if (len > _MAX_IPC_CMD_LEN)
+		return -EINVAL;
 	(*buf) = MALLOC(len);
 	if (!*buf)
 		return -ENOMEM;
diff --git a/multipathd/uxsock.h b/multipathd/uxsock.h
new file mode 100644
index 0000000..79e6243
--- /dev/null
+++ b/multipathd/uxsock.h
@@ -0,0 +1,13 @@
+/* some prototypes */
+int ux_socket_listen(const char *name);
+/*
+ * send_packet_daemon_only() is dedicated for multipathd socket listener.
+ * Other application should use mpathcmd.h instead.
+ */
+int send_packet_daemon_only(int fd, const char *buf);
+
+/*
+ * recv_packet_daemon_only() is dedicated for multipathd socket listener.
+ * Other application should use mpathcmd.h instead.
+ */
+int recv_packet_daemon_only(int fd, char **buf, unsigned int timeout);
-- 
2.9.0

--
dm-devel mailing list
dm-devel@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/dm-devel



[Index of Archives]     [DM Crypt]     [Fedora Desktop]     [ATA RAID]     [Fedora Marketing]     [Fedora Packaging]     [Fedora SELinux]     [Yosemite Discussion]     [KDE Users]     [Fedora Docs]

  Powered by Linux