* [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.