Re: [PATCH net 6/7] netfilter: xt_IDLETIMER: allocate timer with kzalloc()

This is an AI-generated review of your patch. The human sending this
email has considered the AI review valid, or at least plausible.
---
netfilter: xt_IDLETIMER: allocate timer with kzalloc()

This switches the struct idletimer_tg allocation in
idletimer_tg_create_v1() from kmalloc_obj() to kzalloc_obj() so the
embedded struct device_attribute does not carry uninitialized function
pointers.  The v0 path already used kzalloc_obj(), so this aligns the
two.

[Severity: Medium]
The change itself looks right, and zeroing does remove the uninitialized
attr.store / attr.store_const pointers, but does the mechanism described in
the commit message actually work?

The message says:

  "When the sysfs file is created, a process with the DAC_OVERRIDE
  capability can bypass the 0444 mode restriction and write to the file.
  This causes dev_attr_store() to execute the uninitialized store pointer,
  leading to arbitrary kernel code execution."

sysfs creates its kernfs root with the extra open permission check:

fs/sysfs/mount.c:sysfs_init() {
	sysfs_root = kernfs_create_root(NULL, KERNFS_ROOT_EXTRA_OPEN_PERM_CHECK,
					NULL);
	...
}

and kernfs_fop_open() rejects the write open before any ->store dispatch:

fs/kernfs/file.c:kernfs_fop_open() {
	...
	if (root->flags & KERNFS_ROOT_EXTRA_OPEN_PERM_CHECK) {
		if ((file->f_mode & FMODE_WRITE) &&
		    (!(inode->i_mode & S_IWUGO) || !has_write))
			goto err_out;
	...
}

idletimer_tg_create_v1() sets attr.attr.mode = 0444, and
sysfs_add_file_mode_ns() installs that mode verbatim via mode & 0777, so
S_IWUGO is clear on the inode and the write open fails with -EACCES
regardless of CAP_DAC_OVERRIDE.

Reaching the garbage pointer seems to require a privileged chmod of the
sysfs file first (permitted by kernfs_iop_setattr()), which is a different
and much narrower trigger than the capability bypass described.

Could the commit message be reworded to describe the actual trigger, given
that the Fixes: tag and the "arbitrary kernel code execution" wording will
drive stable backports?