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

[sashiko-bot@kernel.org]

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 <alan.maguire@oracle.com>

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.

> diff --git a/kernel/bpf/btf.c b/kernel/bpf/btf.c
> index 3e5890aed2dba..2ac1f1d396608 100644
> --- a/kernel/bpf/btf.c
> +++ b/kernel/bpf/btf.c
[ ... ]
> @@ -8736,10 +8738,82 @@ enum {
>  };
>  
>  #if IS_ENABLED(CONFIG_SYSFS)
> +#if IS_ENABLED(CONFIG_DEBUG_INFO_BTF_INLINE)
> +static struct bin_attribute *vmlinux_inline_attr;
> +#endif
[ ... ]
> +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;
> +}
[ ... ]
> @@ -8755,7 +8829,18 @@ struct bin_attribute *sysfs_btf_add(const char *name, void *data, size_t data_si
>  		err = -ENOMEM;
>  		goto err_free;
>  	}
> -	err = sysfs_create_bin_file(btf_kobj, attr);
> +	if (data_size > 0) {
> +		err = sysfs_create_bin_file(btf_kobj, attr);
> +	} else {
> +		struct kernfs_node *node;
> +
> +		node = __kernfs_create_file(btf_kobj->sd, attr->attr.name,

[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);
> +	}
[ ... ]
> @@ -8775,6 +8860,17 @@ struct bin_attribute *sysfs_btf_add(const char *name, void *data, size_t data_si
>  }
>  #endif
>  
> +#if IS_ENABLED(CONFIG_DEBUG_INFO_BTF_INLINE)
> +static void sysfs_btf_update(struct bin_attribute *attr, void *data, size_t data_size)
> +{
> +	if (!attr)
> +		return;
> +	WRITE_ONCE(attr->private, data);

[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
[ ... ]
> @@ -8872,6 +8968,14 @@ static int btf_module_notify(struct notifier_block *nb, unsigned long op,
>  				err = 0;
>  				goto out;
>  			}
> +			if (strcmp(mod->name, "btf_vmlinux_inline") == 0) {
> +				if (vmlinux_inline_attr)
> +					sysfs_btf_update(vmlinux_inline_attr, data,
> +							 mod->btf_inline_data_size);

[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