[Cluster-devel] [PATCHv2 dlm/next 0/3] fs: dlm: node address configuration fixes

[Cluster-devel] [PATCHv2 dlm/next 0/3] fs: dlm: node address configuration fixes

[Alexander Aring]

Hi,

I got reports of:

kernel: dlm: TCP protocol can't handle multi-homed hosts, try SCTP

there was some reconfiguration involed and my guess it that we
left some dlm_local_addr array pointers in some dangled state. The
first patch should fix the observed behaviour. The other patches is
something what I thought can also happen, it just checks if we
setup a node address twice, if this is the case we return -EEXIST.

This is based on dlm/next with the previous submitted patch series.

- Alex

changes since v2:
 - remove deadlock situation in 3/3
 - fix grammar in 1/3 commit msg

Alexander Aring (3):
  fs: dlm: cleanup dlm_local_addr properly
  fs: dlm: constify addr_compare
  fs: dlm: check on existing node address

 fs/dlm/lowcomms.c | 30 ++++++++++++++++++++++++++++--
 1 file changed, 28 insertions(+), 2 deletions(-)

-- 
2.26.2

[Cluster-devel] [PATCHv2 dlm/next 1/3] fs: dlm: cleanup dlm_local_addr properly

[Alexander Aring]

This patch assigns the dlm_local_addr entries to NULL after we freeing
the memory of the entry. This is required because the user can changed some
settings with less addresses than before and a NULL check on start
functionality will check on a dangled pointer.

Signed-off-by: Alexander Aring <aahringo@redhat.com>
---
 fs/dlm/lowcomms.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/fs/dlm/lowcomms.c b/fs/dlm/lowcomms.c
index 11e5e92148fda..9973293bfddcd 100644
--- a/fs/dlm/lowcomms.c
+++ b/fs/dlm/lowcomms.c
@@ -1248,8 +1248,10 @@ static void deinit_local(void)
 {
 	int i;
 
-	for (i = 0; i < dlm_local_count; i++)
+	for (i = 0; i < dlm_local_count; i++) {
 		kfree(dlm_local_addr[i]);
+		dlm_local_addr[i] = NULL;
+	}
 }
 
 /* Initialise SCTP socket and bind to all interfaces
-- 
2.26.2

[Cluster-devel] [PATCHv2 dlm/next 2/3] fs: dlm: constify addr_compare

[Alexander Aring]

This patch just constify some function parameter which should be have a
read access only.

Signed-off-by: Alexander Aring <aahringo@redhat.com>
---
 fs/dlm/lowcomms.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/fs/dlm/lowcomms.c b/fs/dlm/lowcomms.c
index 9973293bfddcd..c23d794e82910 100644
--- a/fs/dlm/lowcomms.c
+++ b/fs/dlm/lowcomms.c
@@ -270,7 +270,8 @@ static struct dlm_node_addr *find_node_addr(int nodeid)
 	return NULL;
 }
 
-static int addr_compare(struct sockaddr_storage *x, struct sockaddr_storage *y)
+static int addr_compare(const struct sockaddr_storage *x,
+			const struct sockaddr_storage *y)
 {
 	switch (x->ss_family) {
 	case AF_INET: {
-- 
2.26.2

[Cluster-devel] [PATCHv2 dlm/next 3/3] fs: dlm: check on existing node address

[Alexander Aring]

This patch checks if we add twice the same address to a per node address
array. This should never be the case and we report -EEXIST to the user
space.

Signed-off-by: Alexander Aring <aahringo@redhat.com>
---
 fs/dlm/lowcomms.c | 23 +++++++++++++++++++++++
 1 file changed, 23 insertions(+)

diff --git a/fs/dlm/lowcomms.c b/fs/dlm/lowcomms.c
index c23d794e82910..8122eb5f2afb8 100644
--- a/fs/dlm/lowcomms.c
+++ b/fs/dlm/lowcomms.c
@@ -370,10 +370,25 @@ static int addr_to_nodeid(struct sockaddr_storage *addr, int *nodeid)
 	return rv;
 }
 
+/* caller need to held dlm_node_addrs_spin lock */
+static bool dlm_lowcomms_na_has_addr(const struct dlm_node_addr *na,
+				     const struct sockaddr_storage *addr)
+{
+	int i;
+
+	for (i = 0; i < na->addr_count; i++) {
+		if (addr_compare(na->addr[i], addr))
+			return true;
+	}
+
+	return false;
+}
+
 int dlm_lowcomms_addr(int nodeid, struct sockaddr_storage *addr, int len)
 {
 	struct sockaddr_storage *new_addr;
 	struct dlm_node_addr *new_node, *na;
+	bool ret;
 
 	new_node = kzalloc(sizeof(struct dlm_node_addr), GFP_NOFS);
 	if (!new_node)
@@ -398,6 +413,14 @@ int dlm_lowcomms_addr(int nodeid, struct sockaddr_storage *addr, int len)
 		return 0;
 	}
 
+	ret = dlm_lowcomms_na_has_addr(na, addr);
+	if (ret) {
+		spin_unlock(&dlm_node_addrs_spin);
+		kfree(new_addr);
+		kfree(new_node);
+		return -EEXIST;
+	}
+
 	if (na->addr_count >= DLM_MAX_ADDR_COUNT) {
 		spin_unlock(&dlm_node_addrs_spin);
 		kfree(new_addr);
-- 
2.26.2