[linux-morello] Re: [PATCH 04/16] xt_IDLETIMER: remove pointer from uapi struct

On 12/08/2024 16:11, Joshua Lant wrote:

On morello architecture, use of kernel pointers in the uapi structures is not permitted, due to different alignment requirements. Modify these to be unsigned longs.

Signed-off-by: Joshua Lant joshualant@gmail.com

include/uapi/linux/netfilter/xt_IDLETIMER.h | 6 +- net/netfilter/xt_IDLETIMER.c | 132 ++++++++++---------- 2 files changed, 70 insertions(+), 68 deletions(-)

diff --git a/include/uapi/linux/netfilter/xt_IDLETIMER.h b/include/uapi/linux/netfilter/xt_IDLETIMER.h index 7bfb31a66fc9..eef4aa6c6649 100644 --- a/include/uapi/linux/netfilter/xt_IDLETIMER.h +++ b/include/uapi/linux/netfilter/xt_IDLETIMER.h @@ -25,7 +25,8 @@ struct idletimer_tg_info { char label[MAX_IDLETIMER_LABEL_SIZE]; /* for kernel module internal use only */

• struct idletimer_tg *timer __attribute__((aligned(8)));

• /* corresponds to the idletimer_tg struct */
• unsigned long timer __attribute__((aligned(8)));

}; struct idletimer_tg_info_v1 { @@ -37,6 +38,7 @@ struct idletimer_tg_info_v1 { __u8 timer_type; /* for kernel module internal use only */

• struct idletimer_tg *timer __attribute__((aligned(8)));

• /* corresponds to the idletimer_tg struct */
• unsigned long timer __attribute__((aligned(8)));

}; #endif diff --git a/net/netfilter/xt_IDLETIMER.c b/net/netfilter/xt_IDLETIMER.c index db720efa811d..70d48e1d58e8 100644 --- a/net/netfilter/xt_IDLETIMER.c +++ b/net/netfilter/xt_IDLETIMER.c @@ -137,7 +137,7 @@ static int idletimer_tg_create(struct idletimer_tg_info *info) { int ret;

• info->timer = kzalloc(sizeof(*info->timer), GFP_KERNEL);

• info->timer = (unsigned long) kzalloc(sizeof(struct idletimer_tg), GFP_KERNEL); if (!info->timer) { ret = -ENOMEM; goto out;

@@ -148,36 +148,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) {

• ((struct idletimer_tg *)info->timer)->attr.attr.name = kstrdup(info->label, GFP_KERNEL);
• if (!((struct idletimer_tg *)info->timer)->attr.attr.name) { ret = -ENOMEM; goto out_free_timer; }

• info->timer->attr.attr.mode = 0444;
• info->timer->attr.show = idletimer_tg_show;

• ((struct idletimer_tg *)info->timer)->attr.attr.mode = 0444;
• ((struct idletimer_tg *)info->timer)->attr.show = idletimer_tg_show;

• ret = sysfs_create_file(idletimer_tg_kobj, &info->timer->attr.attr);

• ret = sysfs_create_file(idletimer_tg_kobj, &((struct idletimer_tg *)info->timer)->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(&((struct idletimer_tg *)info->timer)->entry, &idletimer_tg_list);

• timer_setup(&info->timer->timer, idletimer_tg_expired, 0);
• info->timer->refcnt = 1;

• timer_setup(&((struct idletimer_tg *)info->timer)->timer, idletimer_tg_expired, 0);
• ((struct idletimer_tg *)info->timer)->refcnt = 1;

• INIT_WORK(&info->timer->work, idletimer_tg_work);

• INIT_WORK(&((struct idletimer_tg *)info->timer)->work, idletimer_tg_work);

• mod_timer(&info->timer->timer,

• mod_timer(&((struct idletimer_tg *)info->timer)->timer, msecs_to_jiffies(info->timeout * 1000) + jiffies);

return 0; out_free_attr:

• kfree(info->timer->attr.attr.name);

• kfree(((struct idletimer_tg *)info->timer)->attr.attr.name);

out_free_timer:

• kfree(info->timer);

• kfree(((struct idletimer_tg *)info->timer));

When there are so many access to the field in the same function, it would be better to use a local variable of the right type, as having so many casts gets pretty unreadable.

Kevin

[...]