cluster-devel.redhat.com archive mirror
 help / color / mirror / Atom feed
* [Cluster-devel] [PATCH dlm-next 0/2] fs: dlm: dlm lock from user space fixes
@ 2020-12-11 15:47 Alexander Aring
  2020-12-11 15:47 ` [Cluster-devel] [PATCH dlm-next 1/2] fs: dlm: check for invalid namelen Alexander Aring
  2020-12-11 15:47 ` [Cluster-devel] [PATCH dlm-next 2/2] fs: dlm: transparently align name buffer Alexander Aring
  0 siblings, 2 replies; 3+ messages in thread
From: Alexander Aring @ 2020-12-11 15:47 UTC (permalink / raw)
  To: cluster-devel.redhat.com

Hi,

found some issues with the user space fixes and that we don't have
proper aligned dlm messages. I found these issues while having my
pending patches applied which introduced a "hart" assert for this and
dropping messages on such case. With this assert dlm will probably not
backwards compatible anymore...

so here my question/suggestion:

We should queue these patches to stable?

- Alex

btw: these patches can be applied, I will send PATCHv3 for my pending
patches, I added two other patches.

Alexander Aring (2):
  fs: dlm: check for invalid namelen
  fs: dlm: transparently align name buffer

 fs/dlm/user.c | 10 ++++++++--
 1 file changed, 8 insertions(+), 2 deletions(-)

-- 
2.26.2



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

* [Cluster-devel] [PATCH dlm-next 1/2] fs: dlm: check for invalid namelen
  2020-12-11 15:47 [Cluster-devel] [PATCH dlm-next 0/2] fs: dlm: dlm lock from user space fixes Alexander Aring
@ 2020-12-11 15:47 ` Alexander Aring
  2020-12-11 15:47 ` [Cluster-devel] [PATCH dlm-next 2/2] fs: dlm: transparently align name buffer Alexander Aring
  1 sibling, 0 replies; 3+ messages in thread
From: Alexander Aring @ 2020-12-11 15:47 UTC (permalink / raw)
  To: cluster-devel.redhat.com

This patch adds an additional check inside the dlm locking from user space
functionality that the namelen isn't above the maximum allowed dlm
resource name length.

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

diff --git a/fs/dlm/user.c b/fs/dlm/user.c
index e5cefa90b1ce..9f2f743eeb31 100644
--- a/fs/dlm/user.c
+++ b/fs/dlm/user.c
@@ -241,6 +241,9 @@ static int device_user_lock(struct dlm_user_proc *proc,
 	uint32_t lkid;
 	int error = -ENOMEM;
 
+	if (params->namelen > DLM_RESNAME_MAXLEN)
+		return -EINVAL;
+
 	ls = dlm_find_lockspace_local(proc->lockspace);
 	if (!ls)
 		return -ENOENT;
-- 
2.26.2



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

* [Cluster-devel] [PATCH dlm-next 2/2] fs: dlm: transparently align name buffer
  2020-12-11 15:47 [Cluster-devel] [PATCH dlm-next 0/2] fs: dlm: dlm lock from user space fixes Alexander Aring
  2020-12-11 15:47 ` [Cluster-devel] [PATCH dlm-next 1/2] fs: dlm: check for invalid namelen Alexander Aring
@ 2020-12-11 15:47 ` Alexander Aring
  1 sibling, 0 replies; 3+ messages in thread
From: Alexander Aring @ 2020-12-11 15:47 UTC (permalink / raw)
  To: cluster-devel.redhat.com

This patch adds a zeroed temporary buffer to align the given resource
name of a dlm lock user space request transparently to a 4 byte aligned
buffer. I hit the assert while running the recent pending patch which
contains a 4 byte aligned assert for the dlm header msglength field.
It's important that the allocation and msglength field is 4 byte aligned
as the dlm_header structure is natural aligned to 4 byte values. The
internal dlm allocator can return the next remaining space from a
previously allocated length. The buffer will not be aligned to 4 byte
anymore if this case occurs. At receiving side we cannot parse the next
dlm message because the next dlm header starts at a unaligned address as
well.

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

diff --git a/fs/dlm/user.c b/fs/dlm/user.c
index 9f2f743eeb31..70a5116df9fe 100644
--- a/fs/dlm/user.c
+++ b/fs/dlm/user.c
@@ -236,6 +236,7 @@ void dlm_user_add_ast(struct dlm_lkb *lkb, uint32_t flags, int mode,
 static int device_user_lock(struct dlm_user_proc *proc,
 			    struct dlm_lock_params *params)
 {
+	unsigned char name[DLM_RESNAME_MAXLEN] = {};
 	struct dlm_ls *ls;
 	struct dlm_user_args *ua;
 	uint32_t lkid;
@@ -270,17 +271,19 @@ static int device_user_lock(struct dlm_user_proc *proc,
 				         params->lkid, params->lvb,
 					 (unsigned long) params->timeout);
 	} else if (params->flags & DLM_LKF_ORPHAN) {
+		memcpy(name, params->name, params->namelen);
 		error = dlm_user_adopt_orphan(ls, ua,
 					 params->mode, params->flags,
-					 params->name, params->namelen,
+					 name, ALIGN(params->namelen, 4),
 					 (unsigned long) params->timeout,
 					 &lkid);
 		if (!error)
 			error = lkid;
 	} else {
+		memcpy(name, params->name, params->namelen);
 		error = dlm_user_request(ls, ua,
 					 params->mode, params->flags,
-					 params->name, params->namelen,
+					 name, ALIGN(params->namelen, 4),
 					 (unsigned long) params->timeout);
 		if (!error)
 			error = ua->lksb.sb_lkid;
-- 
2.26.2



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

end of thread, other threads:[~2020-12-11 15:47 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2020-12-11 15:47 [Cluster-devel] [PATCH dlm-next 0/2] fs: dlm: dlm lock from user space fixes Alexander Aring
2020-12-11 15:47 ` [Cluster-devel] [PATCH dlm-next 1/2] fs: dlm: check for invalid namelen Alexander Aring
2020-12-11 15:47 ` [Cluster-devel] [PATCH dlm-next 2/2] fs: dlm: transparently align name buffer Alexander Aring

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).