Re: [PATCH 4/6] configfs: Constify configfs_bin_attribute

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:

New issues:
- [Low] The configfs documentation was not updated to reflect the new `const` signature of `ct_bin_attrs` in `struct config_item_type`.

Pre-existing issues:
- [High] acpi_table_aml_write lacks validation for minimum payload size and NULL data, leading to a NULL pointer dereference or heap out-of-bounds read.
- [High] tsm_report_inblob_write passes a NULL pointer to memcpy when handling a zero-length write.
--

--- Patch [4]: [PATCH 4/6] configfs: Constify configfs_bin_attribute ---
commit 8e0989136bf4a701a307f8ac9e6579f1dbe58c67
Author: Thomas Weißschuh <linux@weissschuh.net>

configfs: Constify configfs_bin_attribute

The configfs_bin_attribute structures defined by driver are never
modified. Make them const.

As there are only two users of these attributes, adapt them in the same
commit to avoid a phased transition.

> diff --git a/drivers/acpi/acpi_configfs.c b/drivers/acpi/acpi_configfs.c
> index 12ffec7958031..6071699c71655 100644
> --- a/drivers/acpi/acpi_configfs.c
> +++ b/drivers/acpi/acpi_configfs.c
> @@ -91,7 +91,7 @@ static ssize_t acpi_table_aml_read(struct config_item *cfg,
>  
>  CONFIGFS_BIN_ATTR(acpi_table_, aml, NULL, MAX_ACPI_TABLE_SIZE);
>  
> -static struct configfs_bin_attribute *acpi_table_bin_attrs[] = {
> +static const struct configfs_bin_attribute *const acpi_table_bin_attrs[] = {
>  	&acpi_table_attr_aml,

[Severity: High]
This isn't a bug introduced by this patch, but there is a pre-existing issue
in acpi_table_aml_write() which is associated with this attribute.

If a user with write access to the configfs aml attribute performs a 0-byte
write (e.g., write(fd, buf, 0)), configfs_bin_write_iter() sets
write_in_progress but leaves bin_buffer as NULL. On file close,
acpi_table_aml_write() is invoked with data = NULL and size = 0.

The function then immediately dereferences header->length:

	struct acpi_table_header *header = data;
	...
	if (header->length != size) {

This causes a NULL pointer dereference. Additionally, writing a crafted 8-byte
payload can bypass the header->length != size check (if the payload's length
field is 8), leading to an undersized allocation and out-of-bounds read.

Does acpi_table_aml_write() need validation for minimum payload size and NULL
data?

>  	NULL,
>  };

[ ... ]

> diff --git a/drivers/virt/coco/guest/report.c b/drivers/virt/coco/guest/report.c
> index 96e89ddf49896..ad400fbe53f01 100644
> --- a/drivers/virt/coco/guest/report.c
> +++ b/drivers/virt/coco/guest/report.c
> @@ -356,7 +356,7 @@ static struct configfs_attribute *tsm_report_attrs[] = {
>  	NULL,
>  };
>  
> -static struct configfs_bin_attribute *tsm_report_bin_attrs[] = {
> +static const struct configfs_bin_attribute *const tsm_report_bin_attrs[] = {
>  	[TSM_REPORT_INBLOB] = &tsm_report_attr_inblob,

[Severity: High]
This isn't a bug introduced by this patch, but there is a pre-existing issue
in tsm_report_inblob_write() which is associated with this attribute.

A zero-length write to the inblob attribute sets write_in_progress in
configfs without allocating bin_buffer. Upon file release,
tsm_report_inblob_write() is called with buf = NULL and count = 0.

The function then executes:

	memcpy(report->desc.inblob, buf, count);

which resolves to memcpy(..., NULL, 0). This is strictly undefined behavior
in C and can trigger a UBSAN trap, panicking the kernel if panic_on_warn is
enabled.

Does tsm_report_inblob_write() need to check for a zero-length write and NULL
buffer before calling memcpy()?

>  	[TSM_REPORT_OUTBLOB] = &tsm_report_attr_outblob,
>  	[TSM_REPORT_AUXBLOB] = &tsm_report_attr_auxblob,

[ ... ]

> diff --git a/include/linux/configfs.h b/include/linux/configfs.h
> index 5d3fc8822a1d3..eff2fb22ab700 100644
> --- a/include/linux/configfs.h
> +++ b/include/linux/configfs.h
> @@ -67,7 +67,7 @@ struct config_item_type {
>  	const struct configfs_item_operations	*ct_item_ops;
>  	const struct configfs_group_operations	*ct_group_ops;
>  	struct configfs_attribute		**ct_attrs;
> -	struct configfs_bin_attribute		**ct_bin_attrs;
> +	const struct configfs_bin_attribute	*const *ct_bin_attrs;
>  };

[Severity: Low]
Does Documentation/filesystems/configfs.rst need to be updated to reflect
the new const signature of ct_bin_attrs in struct config_item_type? It
currently documents the old non-const pointer array type.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260716-configfs-const-base-v1-0-c545a4053cb5@weissschuh.net?part=4