* [Ocfs2-devel] [PATCH v2] ocfs2: fix possible memory leak in dlm_process_recovery_data
@ 2013-05-02 12:56 Joseph Qi
2013-05-02 17:19 ` Sunil Mushran
0 siblings, 1 reply; 4+ messages in thread
From: Joseph Qi @ 2013-05-02 12:56 UTC (permalink / raw)
To: ocfs2-devel
We found a possible memory leak in dlm_process_recovery_data when doing
code review. In dlm_process_recovery_data, it creates newlock each time,
but don't free when it is bad, and then it will lead to memory leak.
Cc: stable at vger.kernel.org
Signed-off-by: Joseph Qi <joseph.qi@huawei.com>
Reviewed-by: Jie Liu <jeff.liu@oracle.com>
---
fs/ocfs2/dlm/dlmrecovery.c | 4 ++++
1 file changed, 4 insertions(+)
diff --git a/fs/ocfs2/dlm/dlmrecovery.c b/fs/ocfs2/dlm/dlmrecovery.c
index eeac97b..9f08523 100644
--- a/fs/ocfs2/dlm/dlmrecovery.c
+++ b/fs/ocfs2/dlm/dlmrecovery.c
@@ -1974,6 +1974,10 @@ skip_lvb:
res->lockname.len, res->lockname.name, ml->node);
dlm_lockres_set_refmap_bit(dlm, res, ml->node);
added++;
+ } else {
+ /* Free the new lock if it is bad */
+ dlm_lock_put(newlock);
+ newlock = NULL;
}
spin_unlock(&res->spinlock);
}
--
1.7.9.7
-------------- next part --------------
An HTML attachment was scrubbed...
URL: http://oss.oracle.com/pipermail/ocfs2-devel/attachments/20130502/a288dc6b/attachment.html
^ permalink raw reply related [flat|nested] 4+ messages in thread* [Ocfs2-devel] [PATCH v2] ocfs2: fix possible memory leak in dlm_process_recovery_data
2013-05-02 12:56 [Ocfs2-devel] [PATCH v2] ocfs2: fix possible memory leak in dlm_process_recovery_data Joseph Qi
@ 2013-05-02 17:19 ` Sunil Mushran
2013-05-04 1:02 ` Joseph Qi
0 siblings, 1 reply; 4+ messages in thread
From: Sunil Mushran @ 2013-05-02 17:19 UTC (permalink / raw)
To: ocfs2-devel
Do you know under what conditions does it create a new lock when it should
not?
This code should only trigger if the lockres is/was mastered on another
node.
Meaning this node will not know about the newlock. Meaning that code should
never trigger.
1949 if (lock->ml.cookie == ml->cookie) {
This if looks hacky.
If you have a reproducible case, it may be worthwhile to get some traces to
see
under what conditions this happens. Should be straight forward after that.
Thanks
Sunil
On Thu, May 2, 2013 at 5:56 AM, Joseph Qi <joseph.qi@huawei.com> wrote:
> We found a possible memory leak in dlm_process_recovery_data when doing
> code review. In dlm_process_recovery_data, it creates newlock each time,
> but don't free when it is bad, and then it will lead to memory leak.
>
> Cc: stable at vger.kernel.org
> Signed-off-by: Joseph Qi <joseph.qi@huawei.com> <joseph.qi@huawei.com>
> Reviewed-by: Jie Liu <jeff.liu@oracle.com> <jeff.liu@oracle.com>
>
> ---
> fs/ocfs2/dlm/dlmrecovery.c | 4 ++++
> 1 file changed, 4 insertions(+)
>
> diff --git a/fs/ocfs2/dlm/dlmrecovery.c b/fs/ocfs2/dlm/dlmrecovery.c
> index eeac97b..9f08523 100644
> --- a/fs/ocfs2/dlm/dlmrecovery.c
> +++ b/fs/ocfs2/dlm/dlmrecovery.c
> @@ -1974,6 +1974,10 @@ skip_lvb:
> res->lockname.len, res->lockname.name, ml->node);
> dlm_lockres_set_refmap_bit(dlm, res, ml->node);
> added++;
> + } else {
> + /* Free the new lock if it is bad */
> + dlm_lock_put(newlock);
> + newlock = NULL;
> }
> spin_unlock(&res->spinlock);
> }
> --
> 1.7.9.7
>
>
> _______________________________________________
> Ocfs2-devel mailing list
> Ocfs2-devel at oss.oracle.com
> https://oss.oracle.com/mailman/listinfo/ocfs2-devel
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: http://oss.oracle.com/pipermail/ocfs2-devel/attachments/20130502/ba665054/attachment.html
^ permalink raw reply [flat|nested] 4+ messages in thread* [Ocfs2-devel] [PATCH v2] ocfs2: fix possible memory leak in dlm_process_recovery_data
2013-05-02 17:19 ` Sunil Mushran
@ 2013-05-04 1:02 ` Joseph Qi
2013-05-04 1:05 ` Sunil Mushran
0 siblings, 1 reply; 4+ messages in thread
From: Joseph Qi @ 2013-05-04 1:02 UTC (permalink / raw)
To: ocfs2-devel
Please consider the belowing case:
dlm_migrate_all_locks -> dlm_empty_lockres -> dlm_migrate_lockres ->
dlm_send_one_lockres, if we have already sent some locks, say
DLM_MAX_MIGRATABLE_LOCKS for the first time, and then
dlm_send_mig_lockres_msg failed because of network down, it will redo
it. During the redo_bucket, the lockres can be hashed and migrated
again.
On 2013/5/3 1:19, Sunil Mushran wrote:
> Do you know under what conditions does it create a new lock when it
> should not?
>
> This code should only trigger if the lockres is/was mastered on another
> node.
> Meaning this node will not know about the newlock. Meaning that code should
> never trigger.
>
> 1949 if (lock->ml.cookie == ml->cookie) {
> This if looks hacky.
>
> If you have a reproducible case, it may be worthwhile to get some traces
> to see
> under what conditions this happens. Should be straight forward after that.
>
> Thanks
> Sunil
>
>
>
> On Thu, May 2, 2013 at 5:56 AM, Joseph Qi <joseph.qi@huawei.com
> <mailto:joseph.qi@huawei.com>> wrote:
>
> We found a possible memory leak in dlm_process_recovery_data when doing
> code review. In dlm_process_recovery_data, it creates newlock each time,
> but don't free when it is bad, and then it will lead to memory leak.
>
> Cc: stable at vger.kernel.org <mailto:stable@vger.kernel.org>
> Signed-off-by: Joseph Qi <joseph.qi@huawei.com> <mailto:joseph.qi@huawei.com>
> Reviewed-by: Jie Liu <jeff.liu@oracle.com> <mailto:jeff.liu@oracle.com>
>
> ---
> fs/ocfs2/dlm/dlmrecovery.c | 4 ++++
> 1 file changed, 4 insertions(+)
>
> diff --git a/fs/ocfs2/dlm/dlmrecovery.c b/fs/ocfs2/dlm/dlmrecovery.c
> index eeac97b..9f08523 100644
> --- a/fs/ocfs2/dlm/dlmrecovery.c
> +++ b/fs/ocfs2/dlm/dlmrecovery.c
> @@ -1974,6 +1974,10 @@ skip_lvb:
> res->lockname.len, res->lockname.name <http://lockname.name>, ml->node);
> dlm_lockres_set_refmap_bit(dlm, res, ml->node);
> added++;
> + } else {
> + /* Free the new lock if it is bad */
> + dlm_lock_put(newlock);
> + newlock = NULL;
> }
> spin_unlock(&res->spinlock);
> }
> --
> 1.7.9.7
>
>
> _______________________________________________
> Ocfs2-devel mailing list
> Ocfs2-devel at oss.oracle.com <mailto:Ocfs2-devel@oss.oracle.com>
> https://oss.oracle.com/mailman/listinfo/ocfs2-devel
>
>
^ permalink raw reply [flat|nested] 4+ messages in thread* [Ocfs2-devel] [PATCH v2] ocfs2: fix possible memory leak in dlm_process_recovery_data
2013-05-04 1:02 ` Joseph Qi
@ 2013-05-04 1:05 ` Sunil Mushran
0 siblings, 0 replies; 4+ messages in thread
From: Sunil Mushran @ 2013-05-04 1:05 UTC (permalink / raw)
To: ocfs2-devel
Any reason we cannot add the existence check to before the create and thus
skip creation altogether.
On Fri, May 3, 2013 at 6:02 PM, Joseph Qi <joseph.qi@huawei.com> wrote:
> Please consider the belowing case:
> dlm_migrate_all_locks -> dlm_empty_lockres -> dlm_migrate_lockres ->
> dlm_send_one_lockres, if we have already sent some locks, say
> DLM_MAX_MIGRATABLE_LOCKS for the first time, and then
> dlm_send_mig_lockres_msg failed because of network down, it will redo
> it. During the redo_bucket, the lockres can be hashed and migrated
> again.
>
> On 2013/5/3 1:19, Sunil Mushran wrote:
> > Do you know under what conditions does it create a new lock when it
> > should not?
> >
> > This code should only trigger if the lockres is/was mastered on another
> > node.
> > Meaning this node will not know about the newlock. Meaning that code
> should
> > never trigger.
> >
> > 1949 if (lock->ml.cookie == ml->cookie) {
> > This if looks hacky.
> >
> > If you have a reproducible case, it may be worthwhile to get some traces
> > to see
> > under what conditions this happens. Should be straight forward after
> that.
> >
> > Thanks
> > Sunil
> >
> >
> >
> > On Thu, May 2, 2013 at 5:56 AM, Joseph Qi <joseph.qi@huawei.com
> > <mailto:joseph.qi@huawei.com>> wrote:
> >
> > We found a possible memory leak in dlm_process_recovery_data when
> doing
> > code review. In dlm_process_recovery_data, it creates newlock each
> time,
> > but don't free when it is bad, and then it will lead to memory leak.
> >
> > Cc: stable at vger.kernel.org <mailto:stable@vger.kernel.org>
> > Signed-off-by: Joseph Qi <joseph.qi@huawei.com> <mailto:
> joseph.qi at huawei.com>
> > Reviewed-by: Jie Liu <jeff.liu@oracle.com> <mailto:
> jeff.liu at oracle.com>
> >
> > ---
> > fs/ocfs2/dlm/dlmrecovery.c | 4 ++++
> > 1 file changed, 4 insertions(+)
> >
> > diff --git a/fs/ocfs2/dlm/dlmrecovery.c b/fs/ocfs2/dlm/dlmrecovery.c
> > index eeac97b..9f08523 100644
> > --- a/fs/ocfs2/dlm/dlmrecovery.c
> > +++ b/fs/ocfs2/dlm/dlmrecovery.c
> > @@ -1974,6 +1974,10 @@ skip_lvb:
> > res->lockname.len, res->lockname.name <
> http://lockname.name>, ml->node);
> > dlm_lockres_set_refmap_bit(dlm, res, ml->node);
> > added++;
> > + } else {
> > + /* Free the new lock if it is bad */
> > + dlm_lock_put(newlock);
> > + newlock = NULL;
> > }
> > spin_unlock(&res->spinlock);
> > }
> > --
> > 1.7.9.7
> >
> >
> > _______________________________________________
> > Ocfs2-devel mailing list
> > Ocfs2-devel at oss.oracle.com <mailto:Ocfs2-devel@oss.oracle.com>
> > https://oss.oracle.com/mailman/listinfo/ocfs2-devel
> >
> >
>
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: http://oss.oracle.com/pipermail/ocfs2-devel/attachments/20130503/cdfbfd0c/attachment.html
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2013-05-04 1:05 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2013-05-02 12:56 [Ocfs2-devel] [PATCH v2] ocfs2: fix possible memory leak in dlm_process_recovery_data Joseph Qi
2013-05-02 17:19 ` Sunil Mushran
2013-05-04 1:02 ` Joseph Qi
2013-05-04 1:05 ` Sunil Mushran
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.