[Cluster-devel] [PATCH] dlm: fixed memory leaks after failed ls_remove_names allocation
If allocation fails on last elements of array need to free already
allocated elements.
Fixes 789924ba635f ("dlm: fix race between remove and lookup")
Cc: sta...@kernel.org # 3.6
Signed-off-by: Vasily Averin
---
fs/dlm/lockspace.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/fs/dlm/lockspace.c b/fs/dlm/lockspace.c
index 5ba94be006ee..f99e110a0af8 100644
--- a/fs/dlm/lockspace.c
+++ b/fs/dlm/lockspace.c
@@ -532,7 +532,7 @@ static int new_lockspace(const char *name, const char
*cluster,
ls->ls_remove_names[i] = kzalloc(DLM_RESNAME_MAXLEN+1,
GFP_KERNEL);
if (!ls->ls_remove_names[i])
- goto out_rsbtbl;
+ goto out_remove_names;
}
idr_init(&ls->ls_lkbidr);
@@ -680,6 +680,7 @@ static int new_lockspace(const char *name, const char
*cluster,
kfree(ls->ls_recover_buf);
out_lkbidr:
idr_destroy(&ls->ls_lkbidr);
+ out_remove_names:
for (i = 0; i < DLM_REMOVE_NAMES_MAX; i++) {
if (ls->ls_remove_names[i])
kfree(ls->ls_remove_names[i]);
--
2.17.1
[Cluster-devel] lost idr_destroy for ls_recover_idr in release_lockspace() ?
Dear David,
I've noticed that release_lockspace() lacks idr_destroy(&ls->ls_recover_idr),
though it is called on rollback in new_lockspace().
It seems for me it is not critical, and should not lead to any leaks,
however could you please re-check it?
Thank you,
Vasily Averin
[Cluster-devel] [PATCH 3/3] dlm: memory leaks on error path in dlm_user_request()
According to comment in dlm_user_request() ua should be freed
in dlm_free_lkb() after successful attach to lkb.
However ua is attached to lkb not in set_lock_args() but later,
inside request_lock().
Fixes 597d0cae0f99 ("[DLM] dlm: user locks")
Cc: sta...@kernel.org # 2.6.19
Signed-off-by: Vasily Averin
---
fs/dlm/lock.c | 14 +++---
1 file changed, 7 insertions(+), 7 deletions(-)
diff --git a/fs/dlm/lock.c b/fs/dlm/lock.c
index 03d767b94f7b..a928ba008d7d 100644
--- a/fs/dlm/lock.c
+++ b/fs/dlm/lock.c
@@ -5795,20 +5795,20 @@ int dlm_user_request(struct dlm_ls *ls, struct
dlm_user_args *ua,
goto out;
}
}
-
- /* After ua is attached to lkb it will be freed by dlm_free_lkb().
- When DLM_IFL_USER is set, the dlm knows that this is a userspace
- lock and that lkb_astparam is the dlm_user_args structure. */
-
error = set_lock_args(mode, &ua->lksb, flags, namelen, timeout_cs,
fake_astfn, ua, fake_bastfn, &args);
- lkb->lkb_flags |= DLM_IFL_USER;
-
if (error) {
+ kfree(ua->lksb.sb_lvbptr);
+ ua->lksb.sb_lvbptr = NULL;
+ kfree(ua);
__put_lkb(ls, lkb);
goto out;
}
+ /* After ua is attached to lkb it will be freed by dlm_free_lkb().
+ When DLM_IFL_USER is set, the dlm knows that this is a userspace
+ lock and that lkb_astparam is the dlm_user_args structure. */
+ lkb->lkb_flags |= DLM_IFL_USER;
error = request_lock(ls, lkb, name, namelen, &args);
switch (error) {
--
2.17.1
[Cluster-devel] [PATCH v2] dlm: fixed memory leaks after failed ls_remove_names allocation
If allocation fails on last elements of array need to free already
allocated elements.
v2: just move existing out_rsbtbl label to right place
Fixes 789924ba635f ("dlm: fix race between remove and lookup")
Cc: sta...@kernel.org # 3.6
Signed-off-by: Vasily Averin
---
fs/dlm/lockspace.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/fs/dlm/lockspace.c b/fs/dlm/lockspace.c
index 5ba94be006ee..6a1529e478f3 100644
--- a/fs/dlm/lockspace.c
+++ b/fs/dlm/lockspace.c
@@ -680,11 +680,11 @@ static int new_lockspace(const char *name, const char
*cluster,
kfree(ls->ls_recover_buf);
out_lkbidr:
idr_destroy(&ls->ls_lkbidr);
+ out_rsbtbl:
for (i = 0; i < DLM_REMOVE_NAMES_MAX; i++) {
if (ls->ls_remove_names[i])
kfree(ls->ls_remove_names[i]);
}
- out_rsbtbl:
vfree(ls->ls_rsbtbl);
out_lsfree:
if (do_unreg)
--
2.17.1
[Cluster-devel] [PATCH 2/3] dlm: lost put_lkb on error path in receive_convert() and receive_unlock()
Fixes 6d40c4a708e0 ("dlm: improve error and debug messages")
Cc: sta...@kernel.org # 3.5
Signed-off-by: Vasily Averin
---
fs/dlm/lock.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/fs/dlm/lock.c b/fs/dlm/lock.c
index 2cb125cc21c9..03d767b94f7b 100644
--- a/fs/dlm/lock.c
+++ b/fs/dlm/lock.c
@@ -4180,6 +4180,7 @@ static int receive_convert(struct dlm_ls *ls, struct
dlm_message *ms)
(unsigned long long)lkb->lkb_recover_seq,
ms->m_header.h_nodeid, ms->m_lkid);
error = -ENOENT;
+ dlm_put_lkb(lkb);
goto fail;
}
@@ -4233,6 +4234,7 @@ static int receive_unlock(struct dlm_ls *ls, struct
dlm_message *ms)
lkb->lkb_id, lkb->lkb_remid,
ms->m_header.h_nodeid, ms->m_lkid);
error = -ENOENT;
+ dlm_put_lkb(lkb);
goto fail;
}
--
2.17.1
[Cluster-devel] [PATCH 1/3] dlm: possible memory leak on error path in create_lkb()
Fixes 3d6aa675fff9 ("dlm: keep lkbs in idr")
Cc: sta...@kernel.org # 3.1
Signed-off-by: Vasily Averin
---
fs/dlm/lock.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/fs/dlm/lock.c b/fs/dlm/lock.c
index cc91963683de..2cb125cc21c9 100644
--- a/fs/dlm/lock.c
+++ b/fs/dlm/lock.c
@@ -1209,6 +1209,7 @@ static int create_lkb(struct dlm_ls *ls, struct dlm_lkb
**lkb_ret)
if (rv < 0) {
log_error(ls, "create_lkb idr error %d", rv);
+ dlm_free_lkb(lkb);
return rv;
}
--
2.17.1
[Cluster-devel] [PATCH 0/1] dlm: seq_file .next functions should increase position index
In Aug 2018 NeilBrown noticed
commit 1f4aace60b0e ("fs/seq_file.c: simplify seq_file iteration code and
interface")
"Some ->next functions do not increment *pos when they return NULL...
Note that such ->next functions are buggy and should be fixed.
A simple demonstration is
dd if=/proc/swaps bs=1000 skip=1
Choose any block size larger than the size of /proc/swaps. This will
always show the whole last line of /proc/swaps"
Described problem is still actual. If you make lseek into middle of last output
line
following read will output end of last line and whole last line once again.
$ dd if=/proc/swaps bs=1 # usual output
FilenameTypeSizeUsedPriority
/dev/dm-0 partition 4194812 97536 -2
104+0 records in
104+0 records out
104 bytes copied
$ dd if=/proc/swaps bs=40 skip=1# last line was generated twice
dd: /proc/swaps: cannot skip to specified offset
v/dm-0 partition 4194812 97536 -2
/dev/dm-0 partition 4194812 97536 -2
3+1 records in
3+1 records out
131 bytes copied
There are lot of other affected files, I've found 30+ including
/proc/net/ip_tables_matches and /proc/sysvipc/*
Following patch fixes the problem in dlm-related file
https://bugzilla.kernel.org/show_bug.cgi?id=206283
Vasily Averin (1):
table_seq_next should increase position index
fs/dlm/debug_fs.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
--
1.8.3.1
[Cluster-devel] [PATCH 1/1] table_seq_next should increase position index
if seq_file .next fuction does not change position index,
read after some lseek can generate unexpected output.
https://bugzilla.kernel.org/show_bug.cgi?id=206283
Signed-off-by: Vasily Averin
---
fs/dlm/debug_fs.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/fs/dlm/debug_fs.c b/fs/dlm/debug_fs.c
index d6bbccb..c1eda93 100644
--- a/fs/dlm/debug_fs.c
+++ b/fs/dlm/debug_fs.c
@@ -523,7 +523,7 @@ static void *table_seq_next(struct seq_file *seq, void
*iter_ptr, loff_t *pos)
ri->rsb = r;
spin_unlock(&ls->ls_rsbtbl[bucket].lock);
dlm_put_rsb(rp);
- ++*pos;
+ ++(*pos);
return ri;
}
spin_unlock(&ls->ls_rsbtbl[bucket].lock);
@@ -542,6 +542,7 @@ static void *table_seq_next(struct seq_file *seq, void
*iter_ptr, loff_t *pos)
if (bucket >= ls->ls_rsbtbl_size) {
kfree(ri);
+ ++(*pos);
return NULL;
}
tree = toss ? &ls->ls_rsbtbl[bucket].toss :
&ls->ls_rsbtbl[bucket].keep;
--
1.8.3.1
Re: [Cluster-devel] [PATCH 1/1] table_seq_next should increase position index
Could I get any comments?
Unfortunately I do not have enough experience with dlm and cannot verify the
patch locally
Usually you can observe following related problems:
- read after lseek beyond end of file, described by NeilBrown in
commit 1f4aace60b0e ("fs/seq_file.c: simplify seq_file iteration code and
interface")
"dd if= bs=1000 skip=1" will incorrectly generate whole last
line
- read after lseek on into middle of last line will output expected rest of
last line but then repeat whole last line once again.
- If .show() function generates multi-line output following bash script will
never finish.
$ q=;while read -r r;do echo "$((++q)) $r";done < AFFECTED_FILE
Thank you,
Vasily Averin
On 1/24/20 9:03 AM, Vasily Averin wrote:
> if seq_file .next fuction does not change position index,
> read after some lseek can generate unexpected output.
>
> https://bugzilla.kernel.org/show_bug.cgi?id=206283
> Signed-off-by: Vasily Averin
> ---
> fs/dlm/debug_fs.c | 3 ++-
> 1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/fs/dlm/debug_fs.c b/fs/dlm/debug_fs.c
> index d6bbccb..c1eda93 100644
> --- a/fs/dlm/debug_fs.c
> +++ b/fs/dlm/debug_fs.c
> @@ -523,7 +523,7 @@ static void *table_seq_next(struct seq_file *seq, void
> *iter_ptr, loff_t *pos)
> ri->rsb = r;
> spin_unlock(&ls->ls_rsbtbl[bucket].lock);
> dlm_put_rsb(rp);
> - ++*pos;
> + ++(*pos);
> return ri;
> }
> spin_unlock(&ls->ls_rsbtbl[bucket].lock);
> @@ -542,6 +542,7 @@ static void *table_seq_next(struct seq_file *seq, void
> *iter_ptr, loff_t *pos)
>
> if (bucket >= ls->ls_rsbtbl_size) {
> kfree(ri);
> + ++(*pos);
> return NULL;
> }
> tree = toss ? &ls->ls_rsbtbl[bucket].toss :
> &ls->ls_rsbtbl[bucket].keep;
>
