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