Re: [PATCH v2 bpf-next 15/18] btf: Support CONFIG_DEBUG_INFO_BTF_INLINE=m

Thank you for your contribution! Sashiko AI review found 4 potential issue(s) to consider:
- [High] Unsafe lockless call to i_size_write() causes seqcount corruption on 32-bit SMP systems.
- [High] TOCTOU race in sysfs_btf_kernfs_read allows out-of-bounds kernel memory read on module reload.
- [Medium] Reloading the btf_vmlinux_inline module permanently leaks the previously allocated BTF inline data.
- [Medium] Unconditional dereference of btf_kobj->sd can lead to a boot-time kernel panic.
--

commit 79f9afc658508547f1298b89f81b2e99fe704e3c
Author: Alan Maguire [off-list ref]

btf: Support CONFIG_DEBUG_INFO_BTF_INLINE=m
    
Allow vmlinux BTF inline info to be delivered via a loadable
module btf_vmlinux_inline.ko; this reduces the vmlinux binary size.
A kernfs representation is created for vmlinux.inline which loads
the module on demand.

[ ... ]

[ ... ]

+static int sysfs_btf_kernfs_open(struct kernfs_open_file *of)
+{
+	struct bin_attribute *attr = of->kn->priv;
+	size_t data_size;
+	int err;

[ ... ]

+	if (of->file) {
+		struct inode *inode = file_inode(of->file);
+
+		if (inode)
+			i_size_write(inode, data_size);

[Severity: High]
Does this code call i_size_write() locklessly?

On 32-bit SMP systems, calling i_size_write() without holding inode_lock(inode)
can cause seqcount corruption. This can lead to infinite loops in i_size_read()
and a potential denial of service when unprivileged users concurrently open the
sysfs file.

+	}
+	return 0;
+}
+
+static ssize_t sysfs_btf_kernfs_read(struct kernfs_open_file *of, char *buf,
+				     size_t bytes_requested, loff_t offset)
+{
+	struct bin_attribute *attr = of->kn->priv;
+	void *data;
+	size_t data_size;
+
+	data_size = smp_load_acquire(&attr->size);
+	if (offset >= data_size)
+		return 0;
+	if (offset + bytes_requested > data_size)
+		bytes_requested = data_size - offset;
+	data = READ_ONCE(attr->private);
+	memcpy(buf, data + offset, bytes_requested);

[Severity: High]
Can this TOCTOU race lead to an out-of-bounds read?

If a module reload triggers sysfs_btf_update() between the smp_load_acquire()
of attr->size and the READ_ONCE() of attr->private, the new data pointer could
be read while still using the old, potentially larger data_size. This might
allow a local user to extract out-of-bounds kernel memory via the memcpy().

+
+	return bytes_requested;
+}

[ ... ]

[Severity: Medium]
Is it possible for btf_kobj to be NULL here?

If btf_kobj failed to allocate during btf_sysfs_init() and sysfs_btf_add() is
called with data_size == 0 for vmlinux.inline, this unconditional dereference
of btf_kobj->sd could lead to a boot-time kernel panic.

+					    attr->attr.mode, GLOBAL_ROOT_UID,
+					    GLOBAL_ROOT_GID, data_size,
+					    &sysfs_btf_kernfs_ops, attr, NULL, NULL);
+		if (IS_ERR(node))
+			err = PTR_ERR(node);
+	}

[ ... ]

[Severity: Medium]
Does this overwrite attr->private without freeing the previous allocation?

When the btf_vmlinux_inline module is reloaded, this appears to permanently
leak the previously allocated BTF inline data pointer.

+	/* Publish data before its non-zero size makes it readable. */
+	smp_store_release(&attr->size, data_size);
+}
+#endif

[ ... ]

[Severity: Medium]
Does this bypass assigning the data pointer to the module's tracking struct?

Because btf_mod->btf_inline_data is left NULL in this branch, the newly
allocated data won't be cleaned up when the module is unloaded, further
contributing to the memory leak on module reload.

+				else
+					kvfree(data);
+				break;
+			}

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260901165757.801449-1-alan.maguire@oracle.com?part=15