[PATCH v3 00/10] libmpathpersist fixes and some more

[PATCH v3 00/10] libmpathpersist fixes and some more

[Martin Wilck]

Hi Christophe,

this is v3 of my small set of libmpathpersist fixes. It's based
on your latest push. Sorry again for missing that.

Changes wrt v2:
 - rebased to latest upstream (39ad8dc8)
 - added another gcc 8.1 fix, and another small patch I hadn't
   submitted yet

Changes wrt v1:
 - introduce and use get_unaligned_be64 (Bart van Assche)
 - add test for (get|put)_unaligned_bexx functions

Note: we have both the "unaligned" and the "beXY_to_cpu" APIs
now, for essentially the same purpose. I wonder if we should
settle on a single API instead. I am unsure if beXY_to_cpu() has
alignment issues on some architectures (I don't think there are
issues on our current main target platforms). If yes, I believe that
we'd be on the safe side using the "unaligned" API.
Anyway, that could be done in a follow-up patch.

Thanks,
Martin

Martin Wilck (10):
  libmpathpersist: remove duplicate test in readfullstatus
  libmpathpersist: fix typo in mpath_format_readfullstatus
  libmpathpersist: fix stack overflow in mpath_format_readfullstatus()
  libmultipath: add (get|put)_unaligned_be64
  multipath-tools/tests: add tests for get_unaligned_beXX
  libmpathpersist: fix byte swapping for big endian systems
  (lib)mpathpersist: use O_RDONLY file descriptors
  libmultipath: fix gcc 8.1 "truncated output" warnings
  multipathd: fix buffer size in cli_getprkey()
  libmultipath: avoid error messages from RDAC check

 libmpathpersist/mpath_persist.c  |  8 +--
 libmpathpersist/mpath_pr_ioctl.c | 75 +++++++++----------------
 libmultipath/propsel.c           |  2 +
 libmultipath/sysfs.c             |  4 +-
 libmultipath/unaligned.h         | 16 ++++++
 libmultipath/util.c              |  4 +-
 mpathpersist/main.c              |  2 +-
 multipathd/cli_handlers.c        |  4 +-
 tests/Makefile                   |  2 +-
 tests/unaligned.c                | 96 ++++++++++++++++++++++++++++++++
 10 files changed, 150 insertions(+), 63 deletions(-)
 create mode 100644 tests/unaligned.c

-- 
2.17.1

[PATCH v3 01/10] libmpathpersist: remove duplicate test in readfullstatus

[Martin Wilck]

Signed-off-by: Martin Wilck <mwilck@suse.com>
---
 libmpathpersist/mpath_pr_ioctl.c | 6 ------
 1 file changed, 6 deletions(-)

diff --git a/libmpathpersist/mpath_pr_ioctl.c b/libmpathpersist/mpath_pr_ioctl.c
index 6dd74031..dcdb530d 100644
--- a/libmpathpersist/mpath_pr_ioctl.c
+++ b/libmpathpersist/mpath_pr_ioctl.c
@@ -216,12 +216,6 @@ void mpath_format_readfullstatus(struct prin_resp *pr_buff, int len, int noisy)
 	mpath_reverse_uint32_byteorder(&pr_buff->prin_descriptor.prin_readfd.prgeneration);
 	mpath_reverse_uint32_byteorder(&pr_buff->prin_descriptor.prin_readfd.number_of_descriptor);
 
-	if (0 == pr_buff->prin_descriptor.prin_readfd.number_of_descriptor)
-	{
-		return ;
-	}
-
-
 	if (pr_buff->prin_descriptor.prin_readfd.number_of_descriptor == 0)
 	{
 		condlog(3, "No registration or resrvation found.");
-- 
2.17.1

[PATCH v3 02/10] libmpathpersist: fix typo in mpath_format_readfullstatus

[Martin Wilck]

Signed-off-by: Martin Wilck <mwilck@suse.com>
---
 libmpathpersist/mpath_pr_ioctl.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/libmpathpersist/mpath_pr_ioctl.c b/libmpathpersist/mpath_pr_ioctl.c
index dcdb530d..bcbb9691 100644
--- a/libmpathpersist/mpath_pr_ioctl.c
+++ b/libmpathpersist/mpath_pr_ioctl.c
@@ -218,7 +218,7 @@ void mpath_format_readfullstatus(struct prin_resp *pr_buff, int len, int noisy)
 
 	if (pr_buff->prin_descriptor.prin_readfd.number_of_descriptor == 0)
 	{
-		condlog(3, "No registration or resrvation found.");
+		condlog(3, "No registration or reservation found.");
 		return;
 	}
 
-- 
2.17.1

[PATCH v3 03/10] libmpathpersist: fix stack overflow in mpath_format_readfullstatus()

[Martin Wilck]

Some storage arrays return corrupt data in response to READ FULL STATUS
PRIN commands. This may lead to stack overflow if the values aren't
sanitized.

Signed-off-by: Martin Wilck <mwilck@suse.com>
---
 libmpathpersist/mpath_pr_ioctl.c | 9 +++++++++
 1 file changed, 9 insertions(+)

diff --git a/libmpathpersist/mpath_pr_ioctl.c b/libmpathpersist/mpath_pr_ioctl.c
index bcbb9691..347f21b2 100644
--- a/libmpathpersist/mpath_pr_ioctl.c
+++ b/libmpathpersist/mpath_pr_ioctl.c
@@ -241,6 +241,13 @@ void mpath_format_readfullstatus(struct prin_resp *pr_buff, int len, int noisy)
 		fdesc.rtpi = get_unaligned_be16(&p[18]);
 
 		tid_len_len = get_unaligned_be32(&p[20]);
+		if (tid_len_len + 24 + k >= additional_length) {
+			condlog(0,
+				"%s: corrupt PRIN response: status descriptor end %d exceeds length %d",
+				__func__, tid_len_len + k + 24,
+				additional_length);
+			tid_len_len = additional_length - k - 24;
+		}
 
 		if (tid_len_len > 0)
 			decode_transport_id( &fdesc, &p[24], tid_len_len);
@@ -272,6 +279,8 @@ decode_transport_id(struct prin_fulldescr *fdesc, unsigned char * p, int length)
 			break;
 		case MPATH_PROTOCOL_ID_ISCSI:
 			num = get_unaligned_be16(&p[2]);
+			if (num >= sizeof(fdesc->trnptid.iscsi_name))
+				num = sizeof(fdesc->trnptid.iscsi_name);
 			memcpy(&fdesc->trnptid.iscsi_name, &p[4], num);
 			jump = (((num + 4) < 24) ? 24 : num + 4);
 			break;
-- 
2.17.1

[PATCH v3 04/10] libmultipath: add (get|put)_unaligned_be64

[Martin Wilck]

Signed-off-by: Martin Wilck <mwilck@suse.com>
---
 libmultipath/unaligned.h | 16 ++++++++++++++++
 1 file changed, 16 insertions(+)

diff --git a/libmultipath/unaligned.h b/libmultipath/unaligned.h
index 14ec8b23..68c07742 100644
--- a/libmultipath/unaligned.h
+++ b/libmultipath/unaligned.h
@@ -17,6 +17,14 @@ static inline uint32_t get_unaligned_be32(void *ptr)
 	return p[0] << 24 | p[1] << 16 | p[2] << 8 | p[3];
 }
 
+static inline uint64_t get_unaligned_be64(void *ptr)
+{
+	uint32_t low = get_unaligned_be32(ptr + 4);
+	uint64_t high = get_unaligned_be32(ptr);
+
+	return high << 32 | low;
+}
+
 static inline void put_unaligned_be16(uint16_t val, void *ptr)
 {
 	uint8_t *p = ptr;
@@ -35,4 +43,12 @@ static inline void put_unaligned_be32(uint32_t val, void *ptr)
 	p[3] = val;
 }
 
+static inline void put_unaligned_be64(uint64_t val, void *ptr)
+{
+	uint8_t *p = ptr;
+
+	put_unaligned_be32(val >> 32, p);
+	put_unaligned_be32(val, p + 4);
+}
+
 #endif /* _UNALIGNED_H_ */
-- 
2.17.1

[PATCH v3 05/10] multipath-tools/tests: add tests for get_unaligned_beXX

[Martin Wilck]

Signed-off-by: Martin Wilck <mwilck@suse.com>
---
 tests/Makefile    |  2 +-
 tests/unaligned.c | 96 +++++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 97 insertions(+), 1 deletion(-)
 create mode 100644 tests/unaligned.c

diff --git a/tests/Makefile b/tests/Makefile
index 78755edd..2685a206 100644
--- a/tests/Makefile
+++ b/tests/Makefile
@@ -3,7 +3,7 @@ include ../Makefile.inc
 CFLAGS += $(BIN_CFLAGS) -I$(multipathdir) -I$(mpathcmddir)
 LIBDEPS += -L$(multipathdir) -lmultipath -lcmocka
 
-TESTS := uevent parser util dmevents hwtable
+TESTS := uevent parser util dmevents hwtable unaligned
 
 .SILENT: $(TESTS:%=%.o)
 .PRECIOUS: $(TESTS:%=%-test)
diff --git a/tests/unaligned.c b/tests/unaligned.c
new file mode 100644
index 00000000..7ece1de8
--- /dev/null
+++ b/tests/unaligned.c
@@ -0,0 +1,96 @@
+#include <inttypes.h>
+#include <stdio.h>
+#include <string.h>
+#include <stdbool.h>
+#include <stdarg.h>
+#include <stddef.h>
+#include <setjmp.h>
+#include <stdlib.h>
+#include <cmocka.h>
+#include "unaligned.h"
+
+#define SIZE 16
+static const char memory[8] = {
+	0x01, 0x23, 0x45, 0x67, 0x89, 0xab, 0xcd, 0xef
+};
+
+static const uint64_t intval64 = 0x0123456789abcdef;
+static const uint32_t intval32 = 0x01234567;
+static const uint16_t intval16 = 0x0123;
+
+#include "globals.c"
+
+static int setup(void **state)
+{
+	return posix_memalign(state, 16, 2 * SIZE);
+}
+
+static int teardown(void **state)
+{
+	free(*state);
+	return 0;
+}
+
+
+#define make_test(bits, offset) \
+	static void test_ ## bits ## _ ## offset(void **state)	\
+{								\
+	int len = bits/8;					\
+	uint8_t *c = *state;					\
+	uint8_t *p = *state + SIZE;				\
+	uint64_t u;						\
+								\
+	assert_in_range(len, 1, SIZE);				\
+	assert_in_range(offset + len, 1, SIZE);			\
+	memset(c, 0, 2 * SIZE);					\
+	memcpy(c + offset, memory, len);			\
+								\
+	u = get_unaligned_be##bits(c + offset);			\
+	assert_int_equal(u, intval##bits);			\
+	put_unaligned_be##bits(u, p + offset);			\
+	assert_memory_equal(c + offset, p  + offset, len);	\
+}
+
+make_test(16, 0);
+make_test(16, 1);
+make_test(32, 0);
+make_test(32, 1);
+make_test(32, 2);
+make_test(32, 3);
+make_test(64, 0);
+make_test(64, 1);
+make_test(64, 2);
+make_test(64, 3);
+make_test(64, 4);
+make_test(64, 5);
+make_test(64, 6);
+make_test(64, 7);
+
+int test_unaligned(void)
+{
+	const struct CMUnitTest tests[] = {
+		cmocka_unit_test(test_16_0),
+		cmocka_unit_test(test_16_1),
+		cmocka_unit_test(test_32_0),
+		cmocka_unit_test(test_32_1),
+		cmocka_unit_test(test_32_2),
+		cmocka_unit_test(test_32_3),
+		cmocka_unit_test(test_64_0),
+		cmocka_unit_test(test_64_1),
+		cmocka_unit_test(test_64_2),
+		cmocka_unit_test(test_64_3),
+		cmocka_unit_test(test_64_4),
+		cmocka_unit_test(test_64_5),
+		cmocka_unit_test(test_64_6),
+		cmocka_unit_test(test_64_7),
+	};
+	return cmocka_run_group_tests(tests, setup, teardown);
+}
+
+int main(void)
+{
+	int ret = 0;
+
+	ret += test_unaligned();
+	return ret;
+}
-- 
2.17.1

[PATCH v3 06/10] libmpathpersist: fix byte swapping for big endian systems

[Martin Wilck]

The functions "mpath_reverse_uintXY_byteorder"  were apparently meant to
convert BE integers to native. This leads to wrong results on big-endian
systems. Make these functions use the get_unaligned_beXY API, and rename.

Also, remove the unused endianness test in mpath_pr_ioctl.c.

Signed-off-by: Martin Wilck <mwilck@suse.com>
---
 libmpathpersist/mpath_persist.c  |  8 ++---
 libmpathpersist/mpath_pr_ioctl.c | 54 +++++++++-----------------------
 2 files changed, 16 insertions(+), 46 deletions(-)

diff --git a/libmpathpersist/mpath_persist.c b/libmpathpersist/mpath_persist.c
index 6e9e67f9..435ef4b6 100644
--- a/libmpathpersist/mpath_persist.c
+++ b/libmpathpersist/mpath_persist.c
@@ -20,6 +20,7 @@
 #include <ctype.h>
 #include "propsel.h"
 #include "util.h"
+#include "unaligned.h"
 
 #include "mpath_persist.h"
 #include "mpathpr.h"
@@ -560,12 +561,7 @@ int mpath_prout_reg(struct multipath *mpp,int rq_servact, int rq_scope,
 		}
 		if (!rollback && (thread[i].param.status == MPATH_PR_RESERV_CONFLICT)){
 			rollback = 1;
-			sa_key = 0;
-			for (i = 0; i < 8; ++i){
-				if (i > 0)
-					sa_key <<= 8;
-				sa_key |= paramp->sa_key[i];
-			}
+			sa_key = get_unaligned_be64(&paramp->sa_key[0]);
 			status = MPATH_PR_RESERV_CONFLICT ;
 		}
 		if (!rollback && (status == MPATH_PR_SUCCESS)){
diff --git a/libmpathpersist/mpath_pr_ioctl.c b/libmpathpersist/mpath_pr_ioctl.c
index 347f21b2..8416a3bf 100644
--- a/libmpathpersist/mpath_pr_ioctl.c
+++ b/libmpathpersist/mpath_pr_ioctl.c
@@ -31,8 +31,8 @@ void dumpHex(const char* str, int len, int no_ascii);
 int prout_do_scsi_ioctl( char * dev, int rq_servact, int rq_scope,
 		unsigned int rq_type, struct prout_param_descriptor *paramp, int noisy);
 uint32_t  format_transportids(struct prout_param_descriptor *paramp);
-void mpath_reverse_uint32_byteorder(uint32_t *num);
-void mpath_reverse_uint16_byteorder(uint16_t *num);
+void convert_be32_to_cpu(uint32_t *num);
+void convert_be16_to_cpu(uint16_t *num);
 void decode_transport_id(struct prin_fulldescr *fdesc, unsigned char * p, int length);
 int get_prin_length(int rq_servact);
 int mpath_isLittleEndian(void);
@@ -183,23 +183,23 @@ uint32_t  format_transportids(struct prout_param_descriptor *paramp)
 
 void mpath_format_readkeys( struct prin_resp *pr_buff, int len, int noisy)
 {
-	mpath_reverse_uint32_byteorder(&pr_buff->prin_descriptor.prin_readkeys.prgeneration);
-	mpath_reverse_uint32_byteorder(&pr_buff->prin_descriptor.prin_readkeys.additional_length);
+	convert_be32_to_cpu(&pr_buff->prin_descriptor.prin_readkeys.prgeneration);
+	convert_be32_to_cpu(&pr_buff->prin_descriptor.prin_readkeys.additional_length);
 }
 
 void mpath_format_readresv(struct prin_resp *pr_buff, int len, int noisy)
 {
 
-	mpath_reverse_uint32_byteorder(&pr_buff->prin_descriptor.prin_readkeys.prgeneration);
-	mpath_reverse_uint32_byteorder(&pr_buff->prin_descriptor.prin_readkeys.additional_length);
+	convert_be32_to_cpu(&pr_buff->prin_descriptor.prin_readkeys.prgeneration);
+	convert_be32_to_cpu(&pr_buff->prin_descriptor.prin_readkeys.additional_length);
 
 	return;
 }
 
 void mpath_format_reportcapabilities(struct prin_resp *pr_buff, int len, int noisy)
 {
-	mpath_reverse_uint16_byteorder(&pr_buff->prin_descriptor.prin_readcap.length);
-	mpath_reverse_uint16_byteorder(&pr_buff->prin_descriptor.prin_readcap.pr_type_mask);
+	convert_be16_to_cpu(&pr_buff->prin_descriptor.prin_readcap.length);
+	convert_be16_to_cpu(&pr_buff->prin_descriptor.prin_readcap.pr_type_mask);
 
 	return;
 }
@@ -213,8 +213,8 @@ void mpath_format_readfullstatus(struct prin_resp *pr_buff, int len, int noisy)
 	uint32_t additional_length;
 
 
-	mpath_reverse_uint32_byteorder(&pr_buff->prin_descriptor.prin_readfd.prgeneration);
-	mpath_reverse_uint32_byteorder(&pr_buff->prin_descriptor.prin_readfd.number_of_descriptor);
+	convert_be32_to_cpu(&pr_buff->prin_descriptor.prin_readfd.prgeneration);
+	convert_be32_to_cpu(&pr_buff->prin_descriptor.prin_readfd.number_of_descriptor);
 
 	if (pr_buff->prin_descriptor.prin_readfd.number_of_descriptor == 0)
 	{
@@ -469,40 +469,14 @@ int mpath_translate_response (char * dev, struct sg_io_hdr io_hdr,
 	return MPATH_PR_SUCCESS;
 }
 
-int mpath_isLittleEndian(void)
+void convert_be16_to_cpu(uint16_t *num)
 {
-	int num = 1;
-	if(*(char *)&num == 1)
-	{
-		condlog(4, "Little-Endian");
-	}
-	else
-	{
-		condlog(4, "Big-Endian");
-	}
-	return 0;
-}
-
-void mpath_reverse_uint16_byteorder(uint16_t *num)
-{
-	uint16_t byte0, byte1;
-
-	byte0 = (*num & 0x000000FF) >>  0 ;
-	byte1 = (*num & 0x0000FF00) >>  8 ;
-
-	*num = ((byte0 << 8) | (byte1 << 0));
+	*num = get_unaligned_be16(num);
 }
 
-void mpath_reverse_uint32_byteorder(uint32_t *num)
+void convert_be32_to_cpu(uint32_t *num)
 {
-	uint32_t byte0, byte1, byte2, byte3;
-
-	byte0 = (*num & 0x000000FF) >>  0 ;
-	byte1 = (*num & 0x0000FF00) >>  8 ;
-	byte2 = (*num & 0x00FF0000) >> 16 ;
-	byte3 = (*num & 0xFF000000) >> 24 ;
-
-	*num = ((byte0 << 24) | (byte1 << 16) | (byte2 << 8) | (byte3 << 0));
+	*num = get_unaligned_be32(num);
 }
 
 void
-- 
2.17.1

[PATCH v3 07/10] (lib)mpathpersist: use O_RDONLY file descriptors

[Martin Wilck]

udevd catches close-after-write inotify events and generates "change"
uvents for such devices, which may cause extra unnecessary and unwanted
udev activity.

Therefore use O_RDONLY file descriptors for PRIN and PROUT commands. This
works just as well as O_WRONLY. sg_persist has supported the --readonly option
for years.

Signed-off-by: Martin Wilck <mwilck@suse.com>
---
 libmpathpersist/mpath_pr_ioctl.c | 4 ++--
 mpathpersist/main.c              | 2 +-
 2 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/libmpathpersist/mpath_pr_ioctl.c b/libmpathpersist/mpath_pr_ioctl.c
index 8416a3bf..a222b1e1 100644
--- a/libmpathpersist/mpath_pr_ioctl.c
+++ b/libmpathpersist/mpath_pr_ioctl.c
@@ -52,7 +52,7 @@ int prout_do_scsi_ioctl(char * dev, int rq_servact, int rq_scope,
 	int fd = -1;
 
 	snprintf(devname, FILE_NAME_SIZE, "/dev/%s",dev);
-	fd = open(devname, O_WRONLY);
+	fd = open(devname, O_RDONLY);
 	if(fd < 0){
 		condlog (1, "%s: unable to open device.", dev);
 		return MPATH_PR_FILE_ERROR;
@@ -308,7 +308,7 @@ int prin_do_scsi_ioctl(char * dev, int rq_servact, struct prin_resp * resp, int
 	{MPATH_PRIN_CMD, 0, 0, 0, 0, 0, 0, 0, 0, 0};
 
 	snprintf(devname, FILE_NAME_SIZE, "/dev/%s",dev);
-	fd = open(devname, O_WRONLY);
+	fd = open(devname, O_RDONLY);
 	if(fd < 0){
 		condlog(0, "%s: Unable to open device ", dev);
 		return MPATH_PR_FILE_ERROR;
diff --git a/mpathpersist/main.c b/mpathpersist/main.c
index 5b37f3ae..34caa16c 100644
--- a/mpathpersist/main.c
+++ b/mpathpersist/main.c
@@ -380,7 +380,7 @@ int main (int argc, char * argv[])
 	}
 
 	/* open device */
-	if ((fd = open (device_name, O_WRONLY)) < 0)
+	if ((fd = open (device_name, O_RDONLY)) < 0)
 	{
 		fprintf (stderr, "%s: error opening file (rw) fd=%d\n",
 				device_name, fd);
-- 
2.17.1

[PATCH v3 08/10] libmultipath: fix gcc 8.1 "truncated output" warnings

[Martin Wilck]

gcc 8.1 was warning about a possibly truncated output in snprintf.

Signed-off-by: Martin Wilck <mwilck@suse.com>
---
 libmultipath/sysfs.c | 4 ++--
 libmultipath/util.c  | 4 ++--
 2 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/libmultipath/sysfs.c b/libmultipath/sysfs.c
index 16e0a737..b7dacaad 100644
--- a/libmultipath/sysfs.c
+++ b/libmultipath/sysfs.c
@@ -239,7 +239,7 @@ sysfs_get_size (struct path *pp, unsigned long long * size)
 int sysfs_check_holders(char * check_devt, char * new_devt)
 {
 	unsigned int major, new_minor, table_minor;
-	char path[PATH_SIZE], check_dev[PATH_SIZE];
+	char path[PATH_MAX], check_dev[PATH_SIZE];
 	char * table_name;
 	DIR *dirfd;
 	struct dirent *holder;
@@ -256,7 +256,7 @@ int sysfs_check_holders(char * check_devt, char * new_devt)
 
 	condlog(3, "%s: checking holder", check_dev);
 
-	snprintf(path, PATH_SIZE, "/sys/block/%s/holders", check_dev);
+	snprintf(path, sizeof(path), "/sys/block/%s/holders", check_dev);
 	dirfd = opendir(path);
 	if (dirfd == NULL) {
 		condlog(3, "%s: failed to open directory %s (%d)",
diff --git a/libmultipath/util.c b/libmultipath/util.c
index 8d8fcc81..347af5b0 100644
--- a/libmultipath/util.c
+++ b/libmultipath/util.c
@@ -331,7 +331,7 @@ setup_thread_attr(pthread_attr_t *attr, size_t stacksize, int detached)
 
 int systemd_service_enabled_in(const char *dev, const char *prefix)
 {
-	char path[PATH_SIZE], file[PATH_SIZE], service[PATH_SIZE];
+	char path[PATH_SIZE], file[PATH_MAX], service[PATH_SIZE];
 	DIR *dirfd;
 	struct dirent *d;
 	int found = 0;
@@ -358,7 +358,7 @@ int systemd_service_enabled_in(const char *dev, const char *prefix)
 		p = d->d_name + strlen(d->d_name) - 6;
 		if (strcmp(p, ".wants"))
 			continue;
-		snprintf(file, PATH_SIZE, "%s/%s/%s",
+		snprintf(file, sizeof(file), "%s/%s/%s",
 			 path, d->d_name, service);
 		if (stat(file, &stbuf) == 0) {
 			condlog(3, "%s: found %s", dev, file);
-- 
2.17.1

[PATCH v3 09/10] multipathd: fix buffer size in cli_getprkey()

[Martin Wilck]

gcc 8.1 warned about this.

Signed-off-by: Martin Wilck <mwilck@suse.com>
---
 multipathd/cli_handlers.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/multipathd/cli_handlers.c b/multipathd/cli_handlers.c
index 47830e1c..5682b5c2 100644
--- a/multipathd/cli_handlers.c
+++ b/multipathd/cli_handlers.c
@@ -1449,7 +1449,7 @@ cli_getprkey(void * v, char ** reply, int * len, void * data)
 	if (!mpp)
 		return 1;
 
-	*reply = malloc(20);
+	*reply = malloc(26);
 
 	if (!get_be64(mpp->reservation_key)) {
 		sprintf(*reply, "none\n");
@@ -1458,7 +1458,7 @@ cli_getprkey(void * v, char ** reply, int * len, void * data)
 	}
 	if (mpp->sa_flags & MPATH_F_APTPL_MASK)
 		flagstr = ":aptpl";
-	snprintf(*reply, 20, "0x%" PRIx64 "%s\n",
+	snprintf(*reply, 26, "0x%" PRIx64 "%s\n",
 		 get_be64(mpp->reservation_key), flagstr);
 	(*reply)[19] = '\0';
 	*len = strlen(*reply) + 1;
-- 
2.17.1

[PATCH v3 10/10] libmultipath: avoid error messages from RDAC check

[Martin Wilck]

VPD pages are only valid for SCSI devices, so we should disable
the RDAC detection for non-SCSI devices. Otherwise we'll get
an error about VPD inquiry failed.

Signed-off-by: Martin Wilck <mwilck@suse.com>
---
 libmultipath/propsel.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/libmultipath/propsel.c b/libmultipath/propsel.c
index f0847dba..af3ed622 100644
--- a/libmultipath/propsel.c
+++ b/libmultipath/propsel.c
@@ -466,6 +466,8 @@ check_rdac(struct path * pp)
 	int len;
 	char buff[44];
 
+	if (pp->bus != SYSFS_BUS_SCSI)
+		return 0;
 	len = get_vpd_sgio(pp->fd, 0xC9, buff, 44);
 	if (len <= 0)
 		return 0;
-- 
2.17.1

Re: [PATCH v3 00/10] libmpathpersist fixes and some more

[Benjamin Marzinski]

On Sun, Jun 24, 2018 at 09:09:34PM +0200, Martin Wilck wrote:
> Hi Christophe,
> 
> this is v3 of my small set of libmpathpersist fixes. It's based
> on your latest push. Sorry again for missing that.
> 
> Changes wrt v2:
>  - rebased to latest upstream (39ad8dc8)
>  - added another gcc 8.1 fix, and another small patch I hadn't
>    submitted yet
> 
> Changes wrt v1:
>  - introduce and use get_unaligned_be64 (Bart van Assche)
>  - add test for (get|put)_unaligned_bexx functions
> 
> Note: we have both the "unaligned" and the "beXY_to_cpu" APIs
> now, for essentially the same purpose. I wonder if we should
> settle on a single API instead. I am unsure if beXY_to_cpu() has
> alignment issues on some architectures (I don't think there are
> issues on our current main target platforms). If yes, I believe that
> we'd be on the safe side using the "unaligned" API.
> Anyway, that could be done in a follow-up patch.
> 
> Thanks,
> Martin

ACK for the set.

-Ben

> 
> Martin Wilck (10):
>   libmpathpersist: remove duplicate test in readfullstatus
>   libmpathpersist: fix typo in mpath_format_readfullstatus
>   libmpathpersist: fix stack overflow in mpath_format_readfullstatus()
>   libmultipath: add (get|put)_unaligned_be64
>   multipath-tools/tests: add tests for get_unaligned_beXX
>   libmpathpersist: fix byte swapping for big endian systems
>   (lib)mpathpersist: use O_RDONLY file descriptors
>   libmultipath: fix gcc 8.1 "truncated output" warnings
>   multipathd: fix buffer size in cli_getprkey()
>   libmultipath: avoid error messages from RDAC check
> 
>  libmpathpersist/mpath_persist.c  |  8 +--
>  libmpathpersist/mpath_pr_ioctl.c | 75 +++++++++----------------
>  libmultipath/propsel.c           |  2 +
>  libmultipath/sysfs.c             |  4 +-
>  libmultipath/unaligned.h         | 16 ++++++
>  libmultipath/util.c              |  4 +-
>  mpathpersist/main.c              |  2 +-
>  multipathd/cli_handlers.c        |  4 +-
>  tests/Makefile                   |  2 +-
>  tests/unaligned.c                | 96 ++++++++++++++++++++++++++++++++
>  10 files changed, 150 insertions(+), 63 deletions(-)
>  create mode 100644 tests/unaligned.c
> 
> -- 
> 2.17.1