* [Ocfs2-devel] [PATCH 1/1] OCFS2: don't leave free'd mle attached to hb events
@ 2009-12-07 14:52 wengang wang
2009-12-07 21:42 ` Sunil Mushran
0 siblings, 1 reply; 5+ messages in thread
From: wengang wang @ 2009-12-07 14:52 UTC (permalink / raw)
To: ocfs2-devel
don't leave free'd mle attached to hb events.
in dlm_add_migration_mle() the mle is attched to "heartbeat events" anyway no
matter there is an existing mle with same name(returns -EEXIST).
dlm_migrate_lockres() calls dlm_add_migration_mle(). in case the later function
returning -EEXIST, dlm_migrate_lockres() frees the (new) mle without detaching
it from "hb events". so that later "hb events" related operations could improperly
operate against wrong mle objects or against an invalid memory address.
the patch fixes above problem. it marks the mle as "added" just after
dlm_add_migration_mle() is called.
Signed-off-by: Wengang Wang <wen.gang.wang@oracle.com>
diff --git a/fs/ocfs2/dlm/dlmmaster.c b/fs/ocfs2/dlm/dlmmaster.c
index 83bcaf2..0df80e9 100644
--- a/fs/ocfs2/dlm/dlmmaster.c
+++ b/fs/ocfs2/dlm/dlmmaster.c
@@ -2498,11 +2498,12 @@ static int dlm_migrate_lockres(struct dlm_ctxt *dlm,
spin_unlock(&dlm->master_lock);
spin_unlock(&dlm->spinlock);
+ mle_added = 1;
+
if (ret == -EEXIST) {
mlog(0, "another process is already migrating it\n");
goto fail;
}
- mle_added = 1;
/*
* set the MIGRATING flag and flush asts
^ permalink raw reply related [flat|nested] 5+ messages in thread* [Ocfs2-devel] [PATCH 1/1] OCFS2: don't leave free'd mle attached to hb events 2009-12-07 14:52 [Ocfs2-devel] [PATCH 1/1] OCFS2: don't leave free'd mle attached to hb events wengang wang @ 2009-12-07 21:42 ` Sunil Mushran 2009-12-08 1:43 ` Wengang Wang 0 siblings, 1 reply; 5+ messages in thread From: Sunil Mushran @ 2009-12-07 21:42 UTC (permalink / raw) To: ocfs2-devel NAK wengang wang wrote: > don't leave free'd mle attached to hb events. > > in dlm_add_migration_mle() the mle is attched to "heartbeat events" anyway no > matter there is an existing mle with same name(returns -EEXIST). > dlm_migrate_lockres() calls dlm_add_migration_mle(). in case the later function > returning -EEXIST, dlm_migrate_lockres() frees the (new) mle without detaching > it from "hb events". so that later "hb events" related operations could improperly > operate against wrong mle objects or against an invalid memory address. The mle is attached to hb events in dlm_init_mle() which is not called if it returns -EEXIST. When it returns -EEXIST, oldmle is set to the existing mle and its refcounting is handled correctly. mle is not touched and thus only needs to be freed. > the patch fixes above problem. it marks the mle as "added" just after > dlm_add_migration_mle() is called. > > Signed-off-by: Wengang Wang <wen.gang.wang@oracle.com> > > diff --git a/fs/ocfs2/dlm/dlmmaster.c b/fs/ocfs2/dlm/dlmmaster.c > index 83bcaf2..0df80e9 100644 > --- a/fs/ocfs2/dlm/dlmmaster.c > +++ b/fs/ocfs2/dlm/dlmmaster.c > @@ -2498,11 +2498,12 @@ static int dlm_migrate_lockres(struct dlm_ctxt *dlm, > spin_unlock(&dlm->master_lock); > spin_unlock(&dlm->spinlock); > > + mle_added = 1; > + > if (ret == -EEXIST) { > mlog(0, "another process is already migrating it\n"); > goto fail; > } > - mle_added = 1; > > /* > * set the MIGRATING flag and flush asts > ^ permalink raw reply [flat|nested] 5+ messages in thread
* [Ocfs2-devel] [PATCH 1/1] OCFS2: don't leave free'd mle attached to hb events 2009-12-07 21:42 ` Sunil Mushran @ 2009-12-08 1:43 ` Wengang Wang 2009-12-08 1:57 ` Sunil Mushran 0 siblings, 1 reply; 5+ messages in thread From: Wengang Wang @ 2009-12-08 1:43 UTC (permalink / raw) To: ocfs2-devel Hi Sunil, Sunil Mushran wrote: > NAK > > wengang wang wrote: >> don't leave free'd mle attached to hb events. >> in dlm_add_migration_mle() the mle is attched to "heartbeat >> events" anyway no >> matter there is an existing mle with same name(returns -EEXIST). >> dlm_migrate_lockres() calls dlm_add_migration_mle(). in case the later >> function >> returning -EEXIST, dlm_migrate_lockres() frees the (new) mle without >> detaching >> it from "hb events". so that later "hb events" related operations >> could improperly >> operate against wrong mle objects or against an invalid memory address. > > The mle is attached to hb events in dlm_init_mle() which is not called > if it returns -EEXIST. When it returns -EEXIST, oldmle is set to the > existing mle and its refcounting is handled correctly. mle is not touched > and thus only needs to be freed. > Maybe I am wrong. but are you sure dlm_init_mle() is not called when it returns -EEXIST? in code, it doesn't return immediately after setting ret with -EEXIST, but continue to call dlm_init_mle(); and then mle->new_master = new_master; Maybe I am wrong somewhere? simplified code pasted here: 3098 static int dlm_add_migration_mle(struct dlm_ctxt *dlm, 3099 struct dlm_lock_resource *res, 3100 struct dlm_master_list_entry *mle, 3101 struct dlm_master_list_entry **oldmle, 3102 const char *name, unsigned int namelen, 3103 u8 new_master, u8 master) 3104 { 3116 found = dlm_find_mle(dlm, oldmle, (char *)name, namelen); 3117 if (found) { 3118 struct dlm_master_list_entry *tmp = *oldmle; 3120 if (tmp->type == DLM_MLE_MIGRATION) { 3121 if (master == dlm->node_num) { 3126 ret = -EEXIST; 3127 } else { 3136 BUG(); 3137 } 3138 } else { .... 3151 } 3153 } 3154 3156 dlm_init_mle(mle, DLM_MLE_MIGRATION, dlm, res, name, namelen); 3157 mle->new_master = new_master; ...... 3163 __dlm_insert_mle(dlm, mle); 3164 3165 return ret; 3166 } regards, wengang. ^ permalink raw reply [flat|nested] 5+ messages in thread
* [Ocfs2-devel] [PATCH 1/1] OCFS2: don't leave free'd mle attached to hb events 2009-12-08 1:43 ` Wengang Wang @ 2009-12-08 1:57 ` Sunil Mushran 2010-02-02 12:23 ` Wengang Wang 0 siblings, 1 reply; 5+ messages in thread From: Sunil Mushran @ 2009-12-08 1:57 UTC (permalink / raw) To: ocfs2-devel oops... I read it incorrectly. But that does not sound right. Let me think about it. Wengang Wang wrote: > Hi Sunil, > > Sunil Mushran wrote: >> NAK >> >> wengang wang wrote: >>> don't leave free'd mle attached to hb events. >>> in dlm_add_migration_mle() the mle is attched to "heartbeat >>> events" anyway no >>> matter there is an existing mle with same name(returns -EEXIST). >>> dlm_migrate_lockres() calls dlm_add_migration_mle(). in case the >>> later function >>> returning -EEXIST, dlm_migrate_lockres() frees the (new) mle without >>> detaching >>> it from "hb events". so that later "hb events" related operations >>> could improperly >>> operate against wrong mle objects or against an invalid memory address. >> >> The mle is attached to hb events in dlm_init_mle() which is not called >> if it returns -EEXIST. When it returns -EEXIST, oldmle is set to the >> existing mle and its refcounting is handled correctly. mle is not >> touched >> and thus only needs to be freed. >> > > Maybe I am wrong. but are you sure dlm_init_mle() is not called when > it returns -EEXIST? > in code, it doesn't return immediately after setting ret with -EEXIST, > but continue to call > dlm_init_mle(); > and then > mle->new_master = new_master; > > Maybe I am wrong somewhere? > simplified code pasted here: > > > 3098 static int dlm_add_migration_mle(struct dlm_ctxt *dlm, > 3099 struct dlm_lock_resource *res, > 3100 struct dlm_master_list_entry *mle, > 3101 struct dlm_master_list_entry > **oldmle, > 3102 const char *name, unsigned int > namelen, > 3103 u8 new_master, u8 master) > 3104 { > 3116 found = dlm_find_mle(dlm, oldmle, (char *)name, namelen); > 3117 if (found) { > 3118 struct dlm_master_list_entry *tmp = *oldmle; > 3120 if (tmp->type == DLM_MLE_MIGRATION) { > 3121 if (master == dlm->node_num) { > 3126 ret = -EEXIST; > 3127 } else { > 3136 BUG(); > 3137 } > 3138 } else { > .... > 3151 } > 3153 } > 3154 > 3156 dlm_init_mle(mle, DLM_MLE_MIGRATION, dlm, res, name, > namelen); > 3157 mle->new_master = new_master; > ...... > 3163 __dlm_insert_mle(dlm, mle); > 3164 > 3165 return ret; > 3166 } > > > regards, > wengang. ^ permalink raw reply [flat|nested] 5+ messages in thread
* [Ocfs2-devel] [PATCH 1/1] OCFS2: don't leave free'd mle attached to hb events 2009-12-08 1:57 ` Sunil Mushran @ 2010-02-02 12:23 ` Wengang Wang 0 siblings, 0 replies; 5+ messages in thread From: Wengang Wang @ 2010-02-02 12:23 UTC (permalink / raw) To: ocfs2-devel Just a reminder:) On 09-12-07 17:57, Sunil Mushran wrote: > oops... I read it incorrectly. But that does not sound right. > Let me think about it. > > Wengang Wang wrote: >> Hi Sunil, >> >> Sunil Mushran wrote: >>> NAK >>> >>> wengang wang wrote: >>>> don't leave free'd mle attached to hb events. >>>> in dlm_add_migration_mle() the mle is attched to "heartbeat >>>> events" anyway no >>>> matter there is an existing mle with same name(returns -EEXIST). >>>> dlm_migrate_lockres() calls dlm_add_migration_mle(). in case the >>>> later function >>>> returning -EEXIST, dlm_migrate_lockres() frees the (new) mle >>>> without detaching >>>> it from "hb events". so that later "hb events" related operations >>>> could improperly >>>> operate against wrong mle objects or against an invalid memory address. >>> ^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2010-02-02 12:23 UTC | newest] Thread overview: 5+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2009-12-07 14:52 [Ocfs2-devel] [PATCH 1/1] OCFS2: don't leave free'd mle attached to hb events wengang wang 2009-12-07 21:42 ` Sunil Mushran 2009-12-08 1:43 ` Wengang Wang 2009-12-08 1:57 ` Sunil Mushran 2010-02-02 12:23 ` Wengang Wang
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.