On 02/09/2024 17:02, Joshua Lant wrote:
diff --git a/net/netfilter/xt_IDLETIMER.c b/net/netfilter/xt_IDLETIMER.c index db720efa811d..7140e45b65bd 100644 --- a/net/netfilter/xt_IDLETIMER.c +++ b/net/netfilter/xt_IDLETIMER.c @@ -136,8 +136,9 @@ static int idletimer_check_sysfs_name(const char *name, unsigned int size) static int idletimer_tg_create(struct idletimer_tg_info *info) { int ret;
- struct idletimer_tg *timer_local = (struct idletimer_tg *)info->timer;
Such assignment does not make sense in those functions where info->timer is an output, not an input: info->timer is initialised on the next line. As a result timer_local will be set to an irrelevant value.
The most sensible option is probably to replace all references to info->timer with timer_local (including on the next line), and set info->timer to timer_local once the latter is (successfully) initialised.
Also a general nit: local variables are not normally named to indicate they are local. In other words, just "timer" instead of "timer_local" would do just fine.
Kevin
- info->timer = kzalloc(sizeof(*info->timer), GFP_KERNEL);
- info->timer = (__nf_kptr_t) kzalloc(sizeof(struct idletimer_tg), GFP_KERNEL); if (!info->timer) { ret = -ENOMEM; goto out;
@@ -148,36 +149,36 @@ static int idletimer_tg_create(struct idletimer_tg_info *info) goto out_free_timer; sysfs_attr_init(&info->timer->attr.attr);
- info->timer->attr.attr.name = kstrdup(info->label, GFP_KERNEL);
- if (!info->timer->attr.attr.name) {
- (timer_localinfo->timer)->attr.attr.name = kstrdup(info->label, GFP_KERNEL);
- if (!timer_local->attr.attr.name) { ret = -ENOMEM; goto out_free_timer; }
- info->timer->attr.attr.mode = 0444;
- info->timer->attr.show = idletimer_tg_show;
- timer_local->attr.attr.mode = 0444;
- timer_local->attr.show = idletimer_tg_show;
- ret = sysfs_create_file(idletimer_tg_kobj, &info->timer->attr.attr);
- ret = sysfs_create_file(idletimer_tg_kobj, &timer_local->attr.attr); if (ret < 0) { pr_debug("couldn't add file to sysfs"); goto out_free_attr; }
- list_add(&info->timer->entry, &idletimer_tg_list);
- list_add(&timer_local->entry, &idletimer_tg_list);
- timer_setup(&info->timer->timer, idletimer_tg_expired, 0);
- info->timer->refcnt = 1;
- timer_setup(&timer_local->timer, idletimer_tg_expired, 0);
- timer_local->refcnt = 1;
- INIT_WORK(&info->timer->work, idletimer_tg_work);
- INIT_WORK(&timer_local->work, idletimer_tg_work);
- mod_timer(&info->timer->timer,
- mod_timer(&timer_local->timer, msecs_to_jiffies(info->timeout * 1000) + jiffies);
return 0; out_free_attr:
- kfree(info->timer->attr.attr.name);
- kfree(timer_local->attr.attr.name);
out_free_timer:
- kfree(info->timer);
- kfree(timer_local);
out: return ret; }
[...]