Just wondering whether there's still hope for this issue to get fixed.
I haven't added the LED extension to iptables yet, so if there's no
interest in fixing this, we can remove the LED target again.
Sorry again it has taken me so long to get around to this, but hopefully the
attached patch is acceptable. It fixes the issue for me, however I would just
like to confirm one thing will work as expected.
I am relying on the xt_tgchk_param passed to the .checkentry function being
zero'd out the first time. One of the members is a pointer which becomes
valid when the LED trigger is created, and I'm assuming if that pointer is
NULL then the LED trigger hasn't yet been created. Providing that iptables
sets this field to NULL before the first call, and leaves it unchanged when
replacing rules/tables then it will work fine.
The patch is against the latest git version of nf-next-2.6.
Thanks,
Adam.
Add reference counting to the netfilter LED target, to fix errors when
multiple rules point to the same target ("LED trigger already exists").
Signed-off-by: Adam Nielsen <a.nielsen@xxxxxxxxxxx>
---
diff --git a/net/netfilter/xt_LED.c b/net/netfilter/xt_LED.c
index 8ff7843..1877d6b 100644
--- a/net/netfilter/xt_LED.c
+++ b/net/netfilter/xt_LED.c
@@ -37,6 +37,7 @@ MODULE_DESCRIPTION("Xtables: trigger LED devices on packet match");
* points to.
*/
struct xt_led_info_internal {
+ atomic_t refcount;
struct led_trigger netfilter_led_trigger;
struct timer_list timer;
};
@@ -83,43 +84,52 @@ static void led_timeout_callback(unsigned long data)
static bool led_tg_check(const struct xt_tgchk_param *par)
{
struct xt_led_info *ledinfo = par->targinfo;
- struct xt_led_info_internal *ledinternal;
+ struct xt_led_info_internal *ledinternal = ledinfo->internal_data;
int err;
- if (ledinfo->id[0] == '\0') {
- printk(KERN_ERR KBUILD_MODNAME ": No 'id' parameter given.\n");
- return false;
- }
-
- ledinternal = kzalloc(sizeof(struct xt_led_info_internal), GFP_KERNEL);
- if (!ledinternal) {
- printk(KERN_CRIT KBUILD_MODNAME ": out of memory\n");
- return false;
- }
-
- ledinternal->netfilter_led_trigger.name = ledinfo->id;
-
- err = led_trigger_register(&ledinternal->netfilter_led_trigger);
- if (err) {
- printk(KERN_CRIT KBUILD_MODNAME
- ": led_trigger_register() failed\n");
- if (err == -EEXIST)
- printk(KERN_ERR KBUILD_MODNAME
- ": Trigger name is already in use.\n");
- goto exit_alloc;
+ if (ledinternal) {
+ /* Rule already exists */
+ atomic_inc(&ledinternal->refcount);
+ } else {
+ /* New rule, check everything is OK */
+ if (ledinfo->id[0] == '\0') {
+ printk(KERN_ERR KBUILD_MODNAME ": No 'id' parameter given.\n");
+ return false;
+ }
+
+ ledinternal = kzalloc(sizeof(struct xt_led_info_internal), GFP_KERNEL);
+ if (!ledinternal) {
+ printk(KERN_CRIT KBUILD_MODNAME ": out of memory\n");
+ return false;
+ }
+
+ ledinternal->netfilter_led_trigger.name = ledinfo->id;
+
+ err = led_trigger_register(&ledinternal->netfilter_led_trigger);
+ if (err) {
+ printk(KERN_CRIT KBUILD_MODNAME
+ ": led_trigger_register() failed\n");
+ if (err == -EEXIST)
+ printk(KERN_ERR KBUILD_MODNAME
+ ": Trigger name is already in use.\n");
+ goto exit_alloc;
+ }
+
+ /* See if we need to set up a timer */
+ if (ledinfo->delay > 0)
+ setup_timer(&ledinternal->timer, led_timeout_callback,
+ (unsigned long)ledinfo);
+
+ atomic_set(&ledinternal->refcount, 1);
+
+ ledinfo->internal_data = ledinternal;
}
- /* See if we need to set up a timer */
- if (ledinfo->delay > 0)
- setup_timer(&ledinternal->timer, led_timeout_callback,
- (unsigned long)ledinfo);
-
- ledinfo->internal_data = ledinternal;
-
return true;
exit_alloc:
kfree(ledinternal);
+ ledinfo->internal_data = NULL;
return false;
}
@@ -129,11 +139,14 @@ static void led_tg_destroy(const struct xt_tgdtor_param *par)
const struct xt_led_info *ledinfo = par->targinfo;
struct xt_led_info_internal *ledinternal = ledinfo->internal_data;
- if (ledinfo->delay > 0)
- del_timer_sync(&ledinternal->timer);
+ if (atomic_dec_and_test(&ledinternal->refcount)) {
+ /* No more rules use this LED trigger, so remove it */
+ if (ledinfo->delay > 0)
+ del_timer_sync(&ledinternal->timer);
- led_trigger_unregister(&ledinternal->netfilter_led_trigger);
- kfree(ledinternal);
+ led_trigger_unregister(&ledinternal->netfilter_led_trigger);
+ kfree(ledinternal);
+ }
}
static struct xt_target led_tg_reg __read_mostly = {
