ipt_recent: fix race between recent_mt_destroy and proc manipulations
Subsystems (3)
PATCH 1/1 ipt_recent: fix race between recent_mt_destroy and proc manipulations
The thing is that recent_mt_destroy first flushes the entries from table with the recent_table_flush and only *after* this removes the proc file, corresponding to that table.
Thus, if we manage to write to this file the '+XXX' command we will leak some entries. If we manage to write there a 'clean' command we'll race in two recent_table_flush flows, since the recent_mt_destroy calls this outside the recent_lock.
The proper solution as I see it is to remove the proc file first and then go on with flushing the table. This flushing becomes safe w/o the lock, since the table is already inaccessible from the outside.
---
net/ipv4/netfilter/ipt_recent.c +1 -1
@@ -305,10 +305,10 @@ static void recent_mt_destroy(const struct xt_match *match, void *matchinfo)spin_lock_bh (&recent_lock);list_del (&t->list);spin_unlock_bh (&recent_lock); -recent_table_flush (t);#ifdef CONFIG_PROC_FS remove_proc_entry (t->name, proc_dir);#endif +recent_table_flush (t);kfree (t); }mutex_unlock (&recent_mutex);
Patrick McHardy
AppliedQuoted 9 lines · Pavel Emelyanov
The thing is that
recent_mt_destroyfirst flushes the entries from table with therecent_table_flushand only *after* this removes the proc file, corresponding to that table.Thus, if we manage to write to this file the '+XXX' command we will leak some entries. If we manage to write there a 'clean' command we'll race in two
recent_table_flushflows, since therecent_mt_destroycalls this outside therecent_lock.
Quoted 4 lines
The proper solution as I see it is to remove the proc file first and then go on with flushing the table. This flushing becomes safe w/o the lock, since the table is already inaccessible from the outside.
Applied, thanks.