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