[Ocfs2-devel] [PATCH 1/1] OCFS2: don't leave free'd mle attached to hb events

[Wengang Wang]

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.