d/freebsd-skq
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

* Fix crash due to forgotten value refcouting in ipfw_link_table_values()

Browse Source 
* Fix argument order in rollback_toperation_state()
* Make flush_table() use operation state API to ease checks.
This commit is contained in:
melifaro 2014-09-02 20:46:18 +00:00
parent 416d664184
commit 9677452b6e
2 changed files with 48 additions and 11 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

56
sys/netpfil/ipfw/ip_fw_table.c
View File

@ -140,7 +140,7 @@ rollback_toperation_state(struct ip_fw_chain *ch, void *object)
tcfg = CHAIN_TO_TCFG(ch);
TAILQ_FOREACH(os, &tcfg->state_list, next)
os->func(os, object);
os->func(object, os);
}
void
@ -576,6 +576,8 @@ add_table_entry(struct ip_fw_chain *ch, struct tid_info *ti,
return (error);
}
ta = tc->ta;
/* Fill in tablestate */
ts.ch = ch;
ts.opstate.func = rollback_add_entry;
ts.tc = tc;
@ -1185,12 +1187,26 @@ ipfw_flush_table(struct ip_fw_chain *ch, ip_fw3_opheader *op3,
return (error);
}
static void
restart_flush(void *object, struct op_state *_state)
{
struct tableop_state *ts;
ts = (struct tableop_state *)_state;
if (ts->tc != object)
return;
/* Indicate we've called */
ts->modified = 1;
}
/*
* Flushes given table.
*
* Function create new table instance with the same
* parameters, swaps it with old one and
* flushes state without holding any locks.
* flushes state without holding runtime WLOCK.
*
* Returns 0 on success.
*/
@ -1203,6 +1219,7 @@ flush_table(struct ip_fw_chain *ch, struct tid_info *ti)
struct table_info ti_old, ti_new, *tablestate;
void *astate_old, *astate_new;
char algostate[64], *pstate;
struct tableop_state ts;
int error;
uint16_t kidx;
uint8_t tflags;
@ -1217,13 +1234,18 @@ flush_table(struct ip_fw_chain *ch, struct tid_info *ti)
IPFW_UH_WUNLOCK(ch);
return (ESRCH);
}
restart:
/* Set up swap handler */
memset(&ts, 0, sizeof(ts));
ts.opstate.func = restart_flush;
ts.tc = tc;
ta = tc->ta;
/* Do not flush readonly tables */
if ((ta->flags & TA_FLAG_READONLY) != 0) {
IPFW_UH_WUNLOCK(ch);
return (EACCES);
}
tc->no.refcnt++;
/* Save startup algo parameters */
if (ta->print_config != NULL) {
ta->print_config(tc->astate, KIDX_TO_TI(ch, tc->no.kidx),
@ -1232,24 +1254,39 @@ flush_table(struct ip_fw_chain *ch, struct tid_info *ti)
} else
pstate = NULL;
tflags = tc->tflags;
tc->no.refcnt++;
add_toperation_state(ch, &ts);
IPFW_UH_WUNLOCK(ch);
/*
* Stage 2: allocate new table instance using same algo.
*/
memset(&ti_new, 0, sizeof(struct table_info));
if ((error = ta->init(ch, &astate_new, &ti_new, pstate, tflags)) != 0) {
IPFW_UH_WLOCK(ch);
tc->no.refcnt--;
IPFW_UH_WUNLOCK(ch);
return (error);
}
error = ta->init(ch, &astate_new, &ti_new, pstate, tflags);
/*
* Stage 3: swap old state pointers with newly-allocated ones.
* Decrease refcount.
*/
IPFW_UH_WLOCK(ch);
tc->no.refcnt--;
del_toperation_state(ch, &ts);
if (error != 0) {
IPFW_UH_WUNLOCK(ch);
return (error);
}
/*
* Restart operation if table swap has happened:
* even if algo may be the same, algo init parameters
* may change. Restart operation instead of doing
* complex checks.
*/
if (ts.modified != 0) {
ta->destroy(astate_new, &ti_new);
goto restart;
}
ni = CHAIN_TO_NI(ch);
kidx = tc->no.kidx;
@ -1264,7 +1301,6 @@ flush_table(struct ip_fw_chain *ch, struct tid_info *ti)
tc->astate = astate_new;
tc->ti = ti_new;
tc->count = 0;
tc->no.refcnt--;
/*
* Stage 4: unref values.

3
sys/netpfil/ipfw/ip_fw_table_value.c
View File

@ -531,8 +531,8 @@ ipfw_link_table_values(struct ip_fw_chain *ch, struct tableop_state *ts)
/* Value found. Bump refcount */
ptv->pval->refcnt++;
found++;
ptei->value = ptv->no.kidx;
found++;
}
if (ts->count == found) {
@ -585,6 +585,7 @@ ipfw_link_table_values(struct ip_fw_chain *ch, struct tableop_state *ts)
ptv = (struct table_val_link *)ipfw_objhash_lookup_name(vi, 0,
(char *)&tval);
if (ptv != NULL) {
ptv->pval->refcnt++;
ptei->value = ptv->no.kidx;
continue;
}