All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 0/5] [multipath-tools] Misc fixes for JSON API.
@ 2017-01-20 13:38 Gris Ge
  2017-01-20 13:38 ` [PATCH 1/5] multipath-tools: libmultipath: fix gcc link failure on rcu_register_thread_mb Gris Ge
                   ` (4 more replies)
  0 siblings, 5 replies; 10+ messages in thread
From: Gris Ge @ 2017-01-20 13:38 UTC (permalink / raw)
  To: dm-devel; +Cc: Gris Ge

Patch 5/5 is a resend.
Other patches are rework of 'multipath: Introducing multipath C API' patch set.

Gris Ge (5):
  multipath-tools: libmultipath: fix gcc link failure on
    rcu_register_thread_mb.
  multipath-tools: libmultipath: Removed unused functions.
  multipath-tools: New way to limit the IPC command length.
  multipath-tools: Set errno mpath_recv_reply() when failure
  multipath-tools: Perform socket client uid check on IPC commands.

 libmpathcmd/mpath_cmd.c |  6 +--
 libmultipath/Makefile   |  2 +-
 libmultipath/uxsock.c   | 99 +++++++++++++++----------------------------------
 libmultipath/uxsock.h   | 11 +++++-
 multipathd/main.c       | 12 +++++-
 multipathd/uxlsnr.c     | 27 ++++++++++++--
 multipathd/uxlsnr.h     |  4 +-
 7 files changed, 80 insertions(+), 81 deletions(-)

-- 
1.8.3.1

^ permalink raw reply	[flat|nested] 10+ messages in thread

* [PATCH 1/5] multipath-tools: libmultipath: fix gcc link failure on rcu_register_thread_mb.
  2017-01-20 13:38 [PATCH 0/5] [multipath-tools] Misc fixes for JSON API Gris Ge
@ 2017-01-20 13:38 ` Gris Ge
  2017-02-10 11:33   ` Christophe Varoqui
  2017-01-20 13:38 ` [PATCH 2/5] multipath-tools: libmultipath: Removed unused functions Gris Ge
                   ` (3 subsequent siblings)
  4 siblings, 1 reply; 10+ messages in thread
From: Gris Ge @ 2017-01-20 13:38 UTC (permalink / raw)
  To: dm-devel; +Cc: Gris Ge

Error:
    ../libmultipath/libmultipath.so: undefined reference to
    `rcu_register_thread_mb'
    ../libmultipath/libmultipath.so: undefined reference to
    `rcu_unregister_thread_mb'

Fix:
    Link the libmultipath with -lurcu

Signed-off-by: Gris Ge <fge@redhat.com>
---
 libmultipath/Makefile | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/libmultipath/Makefile b/libmultipath/Makefile
index b5d54d8..1f5ec25 100644
--- a/libmultipath/Makefile
+++ b/libmultipath/Makefile
@@ -9,7 +9,7 @@ LIBS = $(DEVLIB).$(SONAME)
 
 CFLAGS += -I$(mpathcmddir)
 
-LIBDEPS += -lpthread -ldl -ldevmapper -ludev -L$(mpathcmddir) -lmpathcmd
+LIBDEPS += -lpthread -ldl -ldevmapper -ludev -L$(mpathcmddir) -lmpathcmd -lurcu
 
 ifdef SYSTEMD
 	CFLAGS += -DUSE_SYSTEMD=$(SYSTEMD)
-- 
1.8.3.1

^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [PATCH 2/5] multipath-tools: libmultipath: Removed unused functions.
  2017-01-20 13:38 [PATCH 0/5] [multipath-tools] Misc fixes for JSON API Gris Ge
  2017-01-20 13:38 ` [PATCH 1/5] multipath-tools: libmultipath: fix gcc link failure on rcu_register_thread_mb Gris Ge
@ 2017-01-20 13:38 ` Gris Ge
  2017-01-20 13:39 ` [PATCH 3/5] multipath-tools: New way to limit the IPC command length Gris Ge
                   ` (2 subsequent siblings)
  4 siblings, 0 replies; 10+ messages in thread
From: Gris Ge @ 2017-01-20 13:38 UTC (permalink / raw)
  To: dm-devel; +Cc: Gris Ge

 * Removed these unused functions from libmultipath:
    * write_all()
    * read_all()

Signed-off-by: Gris Ge <fge@redhat.com>
---
 libmultipath/uxsock.c | 60 ---------------------------------------------------
 libmultipath/uxsock.h |  2 --
 2 files changed, 62 deletions(-)

diff --git a/libmultipath/uxsock.c b/libmultipath/uxsock.c
index b158a56..0ca9e50 100644
--- a/libmultipath/uxsock.c
+++ b/libmultipath/uxsock.c
@@ -74,66 +74,6 @@ 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 = send(fd, buf, len, MSG_NOSIGNAL);
-		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)
diff --git a/libmultipath/uxsock.h b/libmultipath/uxsock.h
index c1cf81f..442b564 100644
--- a/libmultipath/uxsock.h
+++ b/libmultipath/uxsock.h
@@ -2,5 +2,3 @@
 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);
-- 
1.8.3.1

^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [PATCH 3/5] multipath-tools: New way to limit the IPC command length.
  2017-01-20 13:38 [PATCH 0/5] [multipath-tools] Misc fixes for JSON API Gris Ge
  2017-01-20 13:38 ` [PATCH 1/5] multipath-tools: libmultipath: fix gcc link failure on rcu_register_thread_mb Gris Ge
  2017-01-20 13:38 ` [PATCH 2/5] multipath-tools: libmultipath: Removed unused functions Gris Ge
@ 2017-01-20 13:39 ` Gris Ge
  2017-02-10 11:36   ` Christophe Varoqui
  2017-01-20 13:39 ` [PATCH 4/5] multipath-tools: Set errno mpath_recv_reply() when failure Gris Ge
  2017-01-20 13:39 ` [PATCH 5/5] multipath-tools: Perform socket client uid check on IPC commands Gris Ge
  4 siblings, 1 reply; 10+ messages in thread
From: Gris Ge @ 2017-01-20 13:39 UTC (permalink / raw)
  To: dm-devel; +Cc: Gris Ge

Problem:

    mpath_recv_reply() from libmpathcmd return -EINVAL on command 'show
    maps json' with 2k paths.  No error will be triggered for command
    `multipathd -k'show maps json` as multipathd is using their own
    implementation on recv_packet() which does not have size limitation.

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 2066784 bytes which trigged the EINVAL error.

Fix:
    * Remove the limitation of MAX_REPLY_LEN in libmpathcmd.

    * New functions `recv_packet_from_client()` in uxsock.h of libmultipath
      which enforce the limitation of buffer size to 512(_MAX_CMD_LEN).

    * Change multipathd socket listener to use
      `recv_packet_from_client()`.

Signed-off-by: Gris Ge <fge@redhat.com>
---
 libmpathcmd/mpath_cmd.c |  2 --
 libmultipath/uxsock.c   | 39 ++++++++++++++++++++++++++++++---------
 libmultipath/uxsock.h   |  9 +++++++++
 multipathd/uxlsnr.c     |  8 +++++---
 4 files changed, 44 insertions(+), 14 deletions(-)

diff --git a/libmpathcmd/mpath_cmd.c b/libmpathcmd/mpath_cmd.c
index d9c5790..7fc9e98 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/libmultipath/uxsock.c b/libmultipath/uxsock.c
index 0ca9e50..492f4b9 100644
--- a/libmultipath/uxsock.c
+++ b/libmultipath/uxsock.c
@@ -24,6 +24,16 @@
 #include "memory.h"
 #include "uxsock.h"
 #include "debug.h"
+
+/*
+ * Code is similar with mpath_recv_reply() with data size limitation
+ * and debug-able malloc.
+ * When limit == 0, it means no limit on data size, used for socket client
+ * to receiving data from multipathd.
+ */
+static int _recv_packet(int fd, char **buf, unsigned int timeout,
+			ssize_t limit);
+
 /*
  * create a unix domain socket and start listening on it
  * return a file descriptor open on the socket
@@ -81,26 +91,37 @@ int send_packet(int fd, const char *buf)
 	return mpath_send_cmd(fd, buf);
 }
 
-/*
- * receive a packet in length prefix format
- */
-int recv_packet(int fd, char **buf, unsigned int timeout)
+static int _recv_packet(int fd, char **buf, unsigned int timeout, ssize_t limit)
 {
-	int err;
-	ssize_t len;
+	int err = 0;
+	ssize_t len = 0;
 
 	*buf = NULL;
 	len = mpath_recv_reply_len(fd, timeout);
 	if (len <= 0)
 		return len;
+	if ((limit > 0) && (len > limit))
+		return -EINVAL;
 	(*buf) = MALLOC(len);
 	if (!*buf)
 		return -ENOMEM;
 	err = mpath_recv_reply_data(fd, *buf, len, timeout);
-	if (err) {
+	if (err != 0) {
 		FREE(*buf);
 		(*buf) = NULL;
-		return err;
 	}
-	return 0;
+	return err;
+}
+
+/*
+ * receive a packet in length prefix format
+ */
+int recv_packet(int fd, char **buf, unsigned int timeout)
+{
+	return _recv_packet(fd, buf, timeout, 0 /* no limit */);
+}
+
+int recv_packet_from_client(int fd, char **buf, unsigned int timeout)
+{
+	return _recv_packet(fd, buf, timeout, _MAX_CMD_LEN);
 }
diff --git a/libmultipath/uxsock.h b/libmultipath/uxsock.h
index 442b564..8e7401d 100644
--- a/libmultipath/uxsock.h
+++ b/libmultipath/uxsock.h
@@ -2,3 +2,12 @@
 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);
+
+#define _MAX_CMD_LEN		512
+
+/*
+ * Used for receiving socket command from untrusted socket client where data
+ * size is restricted to 512(_MAX_CMD_LEN) at most.
+ * Return -EINVAL if data length requested by client exceeded the _MAX_CMD_LEN.
+ */
+int recv_packet_from_client(int fd, char **buf, unsigned int timeout);
diff --git a/multipathd/uxlsnr.c b/multipathd/uxlsnr.c
index dfef03e..6ca62af 100644
--- a/multipathd/uxlsnr.c
+++ b/multipathd/uxlsnr.c
@@ -241,13 +241,15 @@ void * uxsock_listen(uxsock_trigger_fn uxsock_trigger, void * trigger_data)
 				if (clock_gettime(CLOCK_MONOTONIC, &start_time)
 				    != 0)
 					start_time.tv_sec = 0;
-				if (recv_packet(c->fd, &inbuf,
-						uxsock_timeout) != 0) {
+				if (recv_packet_from_client(c->fd, &inbuf,
+							    uxsock_timeout)
+				    != 0) {
 					dead_client(c);
 					continue;
 				}
 				if (!inbuf) {
-					condlog(4, "recv_packet get null request");
+					condlog(4, "recv_packet_from_client "
+						"get null request");
 					continue;
 				}
 				condlog(4, "cli[%d]: Got request [%s]",
-- 
1.8.3.1

^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [PATCH 4/5] multipath-tools: Set errno mpath_recv_reply() when failure
  2017-01-20 13:38 [PATCH 0/5] [multipath-tools] Misc fixes for JSON API Gris Ge
                   ` (2 preceding siblings ...)
  2017-01-20 13:39 ` [PATCH 3/5] multipath-tools: New way to limit the IPC command length Gris Ge
@ 2017-01-20 13:39 ` Gris Ge
  2017-02-10 11:36   ` Christophe Varoqui
  2017-01-20 13:39 ` [PATCH 5/5] multipath-tools: Perform socket client uid check on IPC commands Gris Ge
  4 siblings, 1 reply; 10+ messages in thread
From: Gris Ge @ 2017-01-20 13:39 UTC (permalink / raw)
  To: dm-devel; +Cc: Gris Ge

Enforce what mpath_cmd.h states "-1 on failure (with errno set)" for
mpath_recv_reply() by set errno and return -1 on failure.

Signed-off-by: Gris Ge <fge@redhat.com>
---
 libmpathcmd/mpath_cmd.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/libmpathcmd/mpath_cmd.c b/libmpathcmd/mpath_cmd.c
index 7fc9e98..856e6b4 100644
--- a/libmpathcmd/mpath_cmd.c
+++ b/libmpathcmd/mpath_cmd.c
@@ -141,7 +141,7 @@ int mpath_recv_reply(int fd, char **reply, unsigned int timeout)
 	*reply = NULL;
 	len = mpath_recv_reply_len(fd, timeout);
 	if (len <= 0)
-		return len;
+		return -1;
 	*reply = malloc(len);
 	if (!*reply)
 		return -1;
@@ -149,7 +149,7 @@ int mpath_recv_reply(int fd, char **reply, unsigned int timeout)
 	if (err) {
 		free(*reply);
 		*reply = NULL;
-		return err;
+		return -1;
 	}
 	return 0;
 }
-- 
1.8.3.1

^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [PATCH 5/5] multipath-tools: Perform socket client uid check on IPC commands.
  2017-01-20 13:38 [PATCH 0/5] [multipath-tools] Misc fixes for JSON API Gris Ge
                   ` (3 preceding siblings ...)
  2017-01-20 13:39 ` [PATCH 4/5] multipath-tools: Set errno mpath_recv_reply() when failure Gris Ge
@ 2017-01-20 13:39 ` Gris Ge
  2017-02-10 11:37   ` Christophe Varoqui
  4 siblings, 1 reply; 10+ messages in thread
From: Gris Ge @ 2017-01-20 13:39 UTC (permalink / raw)
  To: dm-devel; +Cc: Gris Ge

Problem:
    A non-root user could send and execute 'shutdown' IPC command to
    multipathd.

Fix:
    Use getsockopt() to find out socket client uid, only query (list or
    show) command are allowed for non-root(uid != 0) socket connection.
    An error message "permission deny: need to be root" will be sent
    otherwise.

Signed-off-by: Gris Ge <fge@redhat.com>
---
 multipathd/main.c   | 12 +++++++++++-
 multipathd/uxlsnr.c | 19 +++++++++++++++++++
 multipathd/uxlsnr.h |  4 +++-
 3 files changed, 33 insertions(+), 2 deletions(-)

diff --git a/multipathd/main.c b/multipathd/main.c
index adc3258..1e64afc 100644
--- a/multipathd/main.c
+++ b/multipathd/main.c
@@ -23,6 +23,7 @@
 #endif
 #include <semaphore.h>
 #include <time.h>
+#include <stdbool.h>
 
 /*
  * libmultipath
@@ -1048,7 +1049,8 @@ map_discovery (struct vectors * vecs)
 }
 
 int
-uxsock_trigger (char * str, char ** reply, int * len, void * trigger_data)
+uxsock_trigger (char * str, char ** reply, int * len, bool is_root,
+		void * trigger_data)
 {
 	struct vectors * vecs;
 	int r;
@@ -1057,6 +1059,14 @@ uxsock_trigger (char * str, char ** reply, int * len, void * trigger_data)
 	*len = 0;
 	vecs = (struct vectors *)trigger_data;
 
+	if ((str != NULL) && (is_root == false) &&
+	    (strncmp(str, "list", strlen("list")) != 0) &&
+	    (strncmp(str, "show", strlen("show")) != 0)) {
+		*reply = STRDUP("permission deny: need to be root");
+		*len = strlen(*reply) + 1;
+		return 1;
+	}
+
 	r = parse_cmd(str, reply, len, vecs, uxsock_timeout / 1000);
 
 	if (r > 0) {
diff --git a/multipathd/uxlsnr.c b/multipathd/uxlsnr.c
index 6ca62af..98ac25a 100644
--- a/multipathd/uxlsnr.c
+++ b/multipathd/uxlsnr.c
@@ -22,6 +22,7 @@
 #include <poll.h>
 #include <sys/time.h>
 #include <signal.h>
+#include <stdbool.h>
 #include "checkers.h"
 #include "memory.h"
 #include "debug.h"
@@ -51,6 +52,23 @@ LIST_HEAD(clients);
 pthread_mutex_t client_lock = PTHREAD_MUTEX_INITIALIZER;
 struct pollfd *polls;
 
+static bool _socket_client_is_root(int fd);
+
+static bool _socket_client_is_root(int fd)
+{
+	socklen_t len = 0;
+	struct ucred uc;
+
+	len = sizeof(struct ucred);
+	if ((fd >= 0) &&
+	    (getsockopt(fd, SOL_SOCKET, SO_PEERCRED, &uc, &len) == 0) &&
+	    (uc.uid == 0))
+			return true;
+
+	/* Treat error as not root client */
+	return false;
+}
+
 /*
  * handle a new client joining
  */
@@ -255,6 +273,7 @@ void * uxsock_listen(uxsock_trigger_fn uxsock_trigger, void * trigger_data)
 				condlog(4, "cli[%d]: Got request [%s]",
 					i, inbuf);
 				uxsock_trigger(inbuf, &reply, &rlen,
+					       _socket_client_is_root(c->fd),
 					       trigger_data);
 				if (reply) {
 					if (send_packet(c->fd,
diff --git a/multipathd/uxlsnr.h b/multipathd/uxlsnr.h
index 4ef47d5..d51a8f9 100644
--- a/multipathd/uxlsnr.h
+++ b/multipathd/uxlsnr.h
@@ -1,7 +1,9 @@
 #ifndef _UXLSNR_H
 #define _UXLSNR_H
 
-typedef int (uxsock_trigger_fn)(char *, char **, int *, void *);
+#include <stdbool.h>
+
+typedef int (uxsock_trigger_fn)(char *, char **, int *, bool, void *);
 
 void * uxsock_listen(uxsock_trigger_fn uxsock_trigger,
 		     void * trigger_data);
-- 
1.8.3.1

^ permalink raw reply related	[flat|nested] 10+ messages in thread

* Re: [PATCH 1/5] multipath-tools: libmultipath: fix gcc link failure on rcu_register_thread_mb.
  2017-01-20 13:38 ` [PATCH 1/5] multipath-tools: libmultipath: fix gcc link failure on rcu_register_thread_mb Gris Ge
@ 2017-02-10 11:33   ` Christophe Varoqui
  0 siblings, 0 replies; 10+ messages in thread
From: Christophe Varoqui @ 2017-02-10 11:33 UTC (permalink / raw)
  To: Gris Ge; +Cc: device-mapper development


[-- Attachment #1.1: Type: text/plain, Size: 1033 bytes --]

Applied.

On Fri, Jan 20, 2017 at 2:38 PM, Gris Ge <fge@redhat.com> wrote:

> Error:
>     ../libmultipath/libmultipath.so: undefined reference to
>     `rcu_register_thread_mb'
>     ../libmultipath/libmultipath.so: undefined reference to
>     `rcu_unregister_thread_mb'
>
> Fix:
>     Link the libmultipath with -lurcu
>
> Signed-off-by: Gris Ge <fge@redhat.com>
> ---
>  libmultipath/Makefile | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/libmultipath/Makefile b/libmultipath/Makefile
> index b5d54d8..1f5ec25 100644
> --- a/libmultipath/Makefile
> +++ b/libmultipath/Makefile
> @@ -9,7 +9,7 @@ LIBS = $(DEVLIB).$(SONAME)
>
>  CFLAGS += -I$(mpathcmddir)
>
> -LIBDEPS += -lpthread -ldl -ldevmapper -ludev -L$(mpathcmddir) -lmpathcmd
> +LIBDEPS += -lpthread -ldl -ldevmapper -ludev -L$(mpathcmddir) -lmpathcmd
> -lurcu
>
>  ifdef SYSTEMD
>         CFLAGS += -DUSE_SYSTEMD=$(SYSTEMD)
> --
> 1.8.3.1
>
> --
> dm-devel mailing list
> dm-devel@redhat.com
> https://www.redhat.com/mailman/listinfo/dm-devel
>

[-- Attachment #1.2: Type: text/html, Size: 1704 bytes --]

[-- Attachment #2: Type: text/plain, Size: 0 bytes --]



^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH 3/5] multipath-tools: New way to limit the IPC command length.
  2017-01-20 13:39 ` [PATCH 3/5] multipath-tools: New way to limit the IPC command length Gris Ge
@ 2017-02-10 11:36   ` Christophe Varoqui
  0 siblings, 0 replies; 10+ messages in thread
From: Christophe Varoqui @ 2017-02-10 11:36 UTC (permalink / raw)
  To: Gris Ge; +Cc: device-mapper development


[-- Attachment #1.1: Type: text/plain, Size: 6003 bytes --]

Applied.

On Fri, Jan 20, 2017 at 2:39 PM, Gris Ge <fge@redhat.com> wrote:

> Problem:
>
>     mpath_recv_reply() from libmpathcmd return -EINVAL on command 'show
>     maps json' with 2k paths.  No error will be triggered for command
>     `multipathd -k'show maps json` as multipathd is using their own
>     implementation on recv_packet() which does not have size limitation.
>
> 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 2066784 bytes which trigged the EINVAL error.
>
> Fix:
>     * Remove the limitation of MAX_REPLY_LEN in libmpathcmd.
>
>     * New functions `recv_packet_from_client()` in uxsock.h of libmultipath
>       which enforce the limitation of buffer size to 512(_MAX_CMD_LEN).
>
>     * Change multipathd socket listener to use
>       `recv_packet_from_client()`.
>
> Signed-off-by: Gris Ge <fge@redhat.com>
> ---
>  libmpathcmd/mpath_cmd.c |  2 --
>  libmultipath/uxsock.c   | 39 ++++++++++++++++++++++++++++++---------
>  libmultipath/uxsock.h   |  9 +++++++++
>  multipathd/uxlsnr.c     |  8 +++++---
>  4 files changed, 44 insertions(+), 14 deletions(-)
>
> diff --git a/libmpathcmd/mpath_cmd.c b/libmpathcmd/mpath_cmd.c
> index d9c5790..7fc9e98 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/libmultipath/uxsock.c b/libmultipath/uxsock.c
> index 0ca9e50..492f4b9 100644
> --- a/libmultipath/uxsock.c
> +++ b/libmultipath/uxsock.c
> @@ -24,6 +24,16 @@
>  #include "memory.h"
>  #include "uxsock.h"
>  #include "debug.h"
> +
> +/*
> + * Code is similar with mpath_recv_reply() with data size limitation
> + * and debug-able malloc.
> + * When limit == 0, it means no limit on data size, used for socket client
> + * to receiving data from multipathd.
> + */
> +static int _recv_packet(int fd, char **buf, unsigned int timeout,
> +                       ssize_t limit);
> +
>  /*
>   * create a unix domain socket and start listening on it
>   * return a file descriptor open on the socket
> @@ -81,26 +91,37 @@ int send_packet(int fd, const char *buf)
>         return mpath_send_cmd(fd, buf);
>  }
>
> -/*
> - * receive a packet in length prefix format
> - */
> -int recv_packet(int fd, char **buf, unsigned int timeout)
> +static int _recv_packet(int fd, char **buf, unsigned int timeout, ssize_t
> limit)
>  {
> -       int err;
> -       ssize_t len;
> +       int err = 0;
> +       ssize_t len = 0;
>
>         *buf = NULL;
>         len = mpath_recv_reply_len(fd, timeout);
>         if (len <= 0)
>                 return len;
> +       if ((limit > 0) && (len > limit))
> +               return -EINVAL;
>         (*buf) = MALLOC(len);
>         if (!*buf)
>                 return -ENOMEM;
>         err = mpath_recv_reply_data(fd, *buf, len, timeout);
> -       if (err) {
> +       if (err != 0) {
>                 FREE(*buf);
>                 (*buf) = NULL;
> -               return err;
>         }
> -       return 0;
> +       return err;
> +}
> +
> +/*
> + * receive a packet in length prefix format
> + */
> +int recv_packet(int fd, char **buf, unsigned int timeout)
> +{
> +       return _recv_packet(fd, buf, timeout, 0 /* no limit */);
> +}
> +
> +int recv_packet_from_client(int fd, char **buf, unsigned int timeout)
> +{
> +       return _recv_packet(fd, buf, timeout, _MAX_CMD_LEN);
>  }
> diff --git a/libmultipath/uxsock.h b/libmultipath/uxsock.h
> index 442b564..8e7401d 100644
> --- a/libmultipath/uxsock.h
> +++ b/libmultipath/uxsock.h
> @@ -2,3 +2,12 @@
>  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);
> +
> +#define _MAX_CMD_LEN           512
> +
> +/*
> + * Used for receiving socket command from untrusted socket client where
> data
> + * size is restricted to 512(_MAX_CMD_LEN) at most.
> + * Return -EINVAL if data length requested by client exceeded the
> _MAX_CMD_LEN.
> + */
> +int recv_packet_from_client(int fd, char **buf, unsigned int timeout);
> diff --git a/multipathd/uxlsnr.c b/multipathd/uxlsnr.c
> index dfef03e..6ca62af 100644
> --- a/multipathd/uxlsnr.c
> +++ b/multipathd/uxlsnr.c
> @@ -241,13 +241,15 @@ void * uxsock_listen(uxsock_trigger_fn
> uxsock_trigger, void * trigger_data)
>                                 if (clock_gettime(CLOCK_MONOTONIC,
> &start_time)
>                                     != 0)
>                                         start_time.tv_sec = 0;
> -                               if (recv_packet(c->fd, &inbuf,
> -                                               uxsock_timeout) != 0) {
> +                               if (recv_packet_from_client(c->fd, &inbuf,
> +                                                           uxsock_timeout)
> +                                   != 0) {
>                                         dead_client(c);
>                                         continue;
>                                 }
>                                 if (!inbuf) {
> -                                       condlog(4, "recv_packet get null
> request");
> +                                       condlog(4,
> "recv_packet_from_client "
> +                                               "get null request");
>                                         continue;
>                                 }
>                                 condlog(4, "cli[%d]: Got request [%s]",
> --
> 1.8.3.1
>
> --
> dm-devel mailing list
> dm-devel@redhat.com
> https://www.redhat.com/mailman/listinfo/dm-devel
>

[-- Attachment #1.2: Type: text/html, Size: 7747 bytes --]

[-- Attachment #2: Type: text/plain, Size: 0 bytes --]



^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH 4/5] multipath-tools: Set errno mpath_recv_reply() when failure
  2017-01-20 13:39 ` [PATCH 4/5] multipath-tools: Set errno mpath_recv_reply() when failure Gris Ge
@ 2017-02-10 11:36   ` Christophe Varoqui
  0 siblings, 0 replies; 10+ messages in thread
From: Christophe Varoqui @ 2017-02-10 11:36 UTC (permalink / raw)
  To: Gris Ge; +Cc: device-mapper development


[-- Attachment #1.1: Type: text/plain, Size: 1231 bytes --]

Applied.

On Fri, Jan 20, 2017 at 2:39 PM, Gris Ge <fge@redhat.com> wrote:

> Enforce what mpath_cmd.h states "-1 on failure (with errno set)" for
> mpath_recv_reply() by set errno and return -1 on failure.
>
> Signed-off-by: Gris Ge <fge@redhat.com>
> ---
>  libmpathcmd/mpath_cmd.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/libmpathcmd/mpath_cmd.c b/libmpathcmd/mpath_cmd.c
> index 7fc9e98..856e6b4 100644
> --- a/libmpathcmd/mpath_cmd.c
> +++ b/libmpathcmd/mpath_cmd.c
> @@ -141,7 +141,7 @@ int mpath_recv_reply(int fd, char **reply, unsigned
> int timeout)
>         *reply = NULL;
>         len = mpath_recv_reply_len(fd, timeout);
>         if (len <= 0)
> -               return len;
> +               return -1;
>         *reply = malloc(len);
>         if (!*reply)
>                 return -1;
> @@ -149,7 +149,7 @@ int mpath_recv_reply(int fd, char **reply, unsigned
> int timeout)
>         if (err) {
>                 free(*reply);
>                 *reply = NULL;
> -               return err;
> +               return -1;
>         }
>         return 0;
>  }
> --
> 1.8.3.1
>
> --
> dm-devel mailing list
> dm-devel@redhat.com
> https://www.redhat.com/mailman/listinfo/dm-devel
>

[-- Attachment #1.2: Type: text/html, Size: 1973 bytes --]

[-- Attachment #2: Type: text/plain, Size: 0 bytes --]



^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH 5/5] multipath-tools: Perform socket client uid check on IPC commands.
  2017-01-20 13:39 ` [PATCH 5/5] multipath-tools: Perform socket client uid check on IPC commands Gris Ge
@ 2017-02-10 11:37   ` Christophe Varoqui
  0 siblings, 0 replies; 10+ messages in thread
From: Christophe Varoqui @ 2017-02-10 11:37 UTC (permalink / raw)
  To: Gris Ge; +Cc: device-mapper development


[-- Attachment #1.1: Type: text/plain, Size: 3907 bytes --]

Applied.

On Fri, Jan 20, 2017 at 2:39 PM, Gris Ge <fge@redhat.com> wrote:

> Problem:
>     A non-root user could send and execute 'shutdown' IPC command to
>     multipathd.
>
> Fix:
>     Use getsockopt() to find out socket client uid, only query (list or
>     show) command are allowed for non-root(uid != 0) socket connection.
>     An error message "permission deny: need to be root" will be sent
>     otherwise.
>
> Signed-off-by: Gris Ge <fge@redhat.com>
> ---
>  multipathd/main.c   | 12 +++++++++++-
>  multipathd/uxlsnr.c | 19 +++++++++++++++++++
>  multipathd/uxlsnr.h |  4 +++-
>  3 files changed, 33 insertions(+), 2 deletions(-)
>
> diff --git a/multipathd/main.c b/multipathd/main.c
> index adc3258..1e64afc 100644
> --- a/multipathd/main.c
> +++ b/multipathd/main.c
> @@ -23,6 +23,7 @@
>  #endif
>  #include <semaphore.h>
>  #include <time.h>
> +#include <stdbool.h>
>
>  /*
>   * libmultipath
> @@ -1048,7 +1049,8 @@ map_discovery (struct vectors * vecs)
>  }
>
>  int
> -uxsock_trigger (char * str, char ** reply, int * len, void * trigger_data)
> +uxsock_trigger (char * str, char ** reply, int * len, bool is_root,
> +               void * trigger_data)
>  {
>         struct vectors * vecs;
>         int r;
> @@ -1057,6 +1059,14 @@ uxsock_trigger (char * str, char ** reply, int *
> len, void * trigger_data)
>         *len = 0;
>         vecs = (struct vectors *)trigger_data;
>
> +       if ((str != NULL) && (is_root == false) &&
> +           (strncmp(str, "list", strlen("list")) != 0) &&
> +           (strncmp(str, "show", strlen("show")) != 0)) {
> +               *reply = STRDUP("permission deny: need to be root");
> +               *len = strlen(*reply) + 1;
> +               return 1;
> +       }
> +
>         r = parse_cmd(str, reply, len, vecs, uxsock_timeout / 1000);
>
>         if (r > 0) {
> diff --git a/multipathd/uxlsnr.c b/multipathd/uxlsnr.c
> index 6ca62af..98ac25a 100644
> --- a/multipathd/uxlsnr.c
> +++ b/multipathd/uxlsnr.c
> @@ -22,6 +22,7 @@
>  #include <poll.h>
>  #include <sys/time.h>
>  #include <signal.h>
> +#include <stdbool.h>
>  #include "checkers.h"
>  #include "memory.h"
>  #include "debug.h"
> @@ -51,6 +52,23 @@ LIST_HEAD(clients);
>  pthread_mutex_t client_lock = PTHREAD_MUTEX_INITIALIZER;
>  struct pollfd *polls;
>
> +static bool _socket_client_is_root(int fd);
> +
> +static bool _socket_client_is_root(int fd)
> +{
> +       socklen_t len = 0;
> +       struct ucred uc;
> +
> +       len = sizeof(struct ucred);
> +       if ((fd >= 0) &&
> +           (getsockopt(fd, SOL_SOCKET, SO_PEERCRED, &uc, &len) == 0) &&
> +           (uc.uid == 0))
> +                       return true;
> +
> +       /* Treat error as not root client */
> +       return false;
> +}
> +
>  /*
>   * handle a new client joining
>   */
> @@ -255,6 +273,7 @@ void * uxsock_listen(uxsock_trigger_fn
> uxsock_trigger, void * trigger_data)
>                                 condlog(4, "cli[%d]: Got request [%s]",
>                                         i, inbuf);
>                                 uxsock_trigger(inbuf, &reply, &rlen,
> +
> _socket_client_is_root(c->fd),
>                                                trigger_data);
>                                 if (reply) {
>                                         if (send_packet(c->fd,
> diff --git a/multipathd/uxlsnr.h b/multipathd/uxlsnr.h
> index 4ef47d5..d51a8f9 100644
> --- a/multipathd/uxlsnr.h
> +++ b/multipathd/uxlsnr.h
> @@ -1,7 +1,9 @@
>  #ifndef _UXLSNR_H
>  #define _UXLSNR_H
>
> -typedef int (uxsock_trigger_fn)(char *, char **, int *, void *);
> +#include <stdbool.h>
> +
> +typedef int (uxsock_trigger_fn)(char *, char **, int *, bool, void *);
>
>  void * uxsock_listen(uxsock_trigger_fn uxsock_trigger,
>                      void * trigger_data);
> --
> 1.8.3.1
>
> --
> dm-devel mailing list
> dm-devel@redhat.com
> https://www.redhat.com/mailman/listinfo/dm-devel
>

[-- Attachment #1.2: Type: text/html, Size: 5390 bytes --]

[-- Attachment #2: Type: text/plain, Size: 0 bytes --]



^ permalink raw reply	[flat|nested] 10+ messages in thread

end of thread, other threads:[~2017-02-10 11:37 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-01-20 13:38 [PATCH 0/5] [multipath-tools] Misc fixes for JSON API Gris Ge
2017-01-20 13:38 ` [PATCH 1/5] multipath-tools: libmultipath: fix gcc link failure on rcu_register_thread_mb Gris Ge
2017-02-10 11:33   ` Christophe Varoqui
2017-01-20 13:38 ` [PATCH 2/5] multipath-tools: libmultipath: Removed unused functions Gris Ge
2017-01-20 13:39 ` [PATCH 3/5] multipath-tools: New way to limit the IPC command length Gris Ge
2017-02-10 11:36   ` Christophe Varoqui
2017-01-20 13:39 ` [PATCH 4/5] multipath-tools: Set errno mpath_recv_reply() when failure Gris Ge
2017-02-10 11:36   ` Christophe Varoqui
2017-01-20 13:39 ` [PATCH 5/5] multipath-tools: Perform socket client uid check on IPC commands Gris Ge
2017-02-10 11:37   ` Christophe Varoqui

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.