| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <CAJZ5v0jSk-NrovSq5En6qOureQXFbCsJLChorbgYXQdYfa0m3w@mail.gmail.com>
Date: Tue, 24 Jun 2025 15:46:58 +0200
From: "Rafael J. Wysocki" <rafael@...nel.org>
To: Brahmajit Das <listout@...tout.xyz>
Cc: linux-hardening@...r.kernel.org, linux-kernel@...r.kernel.org,
linux-acpi@...r.kernel.org, rafael@...nel.org, lenb@...nel.org,
lv.zheng@...el.com, kees@...nel.org, rui.zhang@...el.com, len.brown@...el.com
Subject: Re: [PATCH] ACPI / sysfs: Replace deprecated and unsafe functions
with sysfs_emit
On Tue, Jun 24, 2025 at 3:38 PM Brahmajit Das <listout@...tout.xyz> wrote:
>
> acpi/sysfs.c has many instances of unsafe or deprecated functions such
> as sprintf, strcpy. This patch relaces them with sysfs_emit to safer
"replaces"
> alternavtive and better following of kernel API.
"alternative"
1. Have you tested all of the affected interfaces and verified that
they still work as expected after the changes?
2. While the replaced functions are unsafe in principle, is the usage
of them in any places affected by this patch actually unsafe?
> Signed-off-by: Brahmajit Das <listout@...tout.xyz>
> ---
> drivers/acpi/sysfs.c | 87 ++++++++++++++++++++++----------------------
> 1 file changed, 43 insertions(+), 44 deletions(-)
>
> diff --git a/drivers/acpi/sysfs.c b/drivers/acpi/sysfs.c
> index a48ebbf768f9..c3bb7af79fcb 100644
> --- a/drivers/acpi/sysfs.c
> +++ b/drivers/acpi/sysfs.c
> @@ -94,23 +94,23 @@ static int param_get_debug_layer(char *buffer, const struct kernel_param *kp)
> int result = 0;
> int i;
>
> - result = sprintf(buffer, "%-25s\tHex SET\n", "Description");
> + result = sysfs_emit(buffer, "%-25s\tHex SET\n", "Description");
>
> for (i = 0; i < ARRAY_SIZE(acpi_debug_layers); i++) {
> - result += sprintf(buffer + result, "%-25s\t0x%08lX [%c]\n",
> + result += sysfs_emit(buffer + result, "%-25s\t0x%08lX [%c]\n",
> acpi_debug_layers[i].name,
> acpi_debug_layers[i].value,
> (acpi_dbg_layer & acpi_debug_layers[i].value)
> ? '*' : ' ');
> }
> result +=
> - sprintf(buffer + result, "%-25s\t0x%08X [%c]\n", "ACPI_ALL_DRIVERS",
> + sysfs_emit(buffer + result, "%-25s\t0x%08X [%c]\n", "ACPI_ALL_DRIVERS",
> ACPI_ALL_DRIVERS,
> (acpi_dbg_layer & ACPI_ALL_DRIVERS) ==
> ACPI_ALL_DRIVERS ? '*' : (acpi_dbg_layer & ACPI_ALL_DRIVERS)
> == 0 ? ' ' : '-');
> result +=
> - sprintf(buffer + result,
> + sysfs_emit(buffer + result,
> "--\ndebug_layer = 0x%08X ( * = enabled)\n",
> acpi_dbg_layer);
>
> @@ -122,17 +122,17 @@ static int param_get_debug_level(char *buffer, const struct kernel_param *kp)
> int result = 0;
> int i;
>
> - result = sprintf(buffer, "%-25s\tHex SET\n", "Description");
> + result = sysfs_emit(buffer, "%-25s\tHex SET\n", "Description");
>
> for (i = 0; i < ARRAY_SIZE(acpi_debug_levels); i++) {
> - result += sprintf(buffer + result, "%-25s\t0x%08lX [%c]\n",
> + result += sysfs_emit(buffer + result, "%-25s\t0x%08lX [%c]\n",
> acpi_debug_levels[i].name,
> acpi_debug_levels[i].value,
> (acpi_dbg_level & acpi_debug_levels[i].value)
> ? '*' : ' ');
> }
> result +=
> - sprintf(buffer + result, "--\ndebug_level = 0x%08X (* = enabled)\n",
> + sysfs_emit(buffer + result, "--\ndebug_level = 0x%08X (* = enabled)\n",
> acpi_dbg_level);
>
> return result;
> @@ -181,11 +181,11 @@ static int param_set_trace_method_name(const char *val,
>
> /* This is a hack. We can't kmalloc in early boot. */
> if (is_abs_path)
> - strcpy(trace_method_name, val);
> + sysfs_emit(trace_method_name, "%s", val);
> else {
> - trace_method_name[0] = '\\';
> - strcpy(trace_method_name+1, val);
> + sysfs_emit(trace_method_name, "\%s", val);
> }
> + pr_info("tracepoint: %s", trace_method_name);
>
> /* Restore the original tracer state */
> (void)acpi_debug_trace(trace_method_name,
> @@ -255,13 +255,13 @@ static int param_set_trace_state(const char *val,
> static int param_get_trace_state(char *buffer, const struct kernel_param *kp)
> {
> if (!(acpi_gbl_trace_flags & ACPI_TRACE_ENABLED))
> - return sprintf(buffer, "disable\n");
> + return sysfs_emit(buffer, "disable\n");
> if (!acpi_gbl_trace_method_name)
> - return sprintf(buffer, "enable\n");
> + return sysfs_emit(buffer, "enable\n");
> if (acpi_gbl_trace_flags & ACPI_TRACE_ONESHOT)
> - return sprintf(buffer, "method-once\n");
> + return sysfs_emit(buffer, "method-once\n");
> else
> - return sprintf(buffer, "method\n");
> + return sysfs_emit(buffer, "method\n");
> }
>
> module_param_call(trace_state, param_set_trace_state, param_get_trace_state,
> @@ -282,7 +282,7 @@ static int param_get_acpica_version(char *buffer,
> {
> int result;
>
> - result = sprintf(buffer, "%x\n", ACPI_CA_VERSION);
> + result = sysfs_emit(buffer, "%x\n", ACPI_CA_VERSION);
>
> return result;
> }
> @@ -366,9 +366,8 @@ static int acpi_table_attr_init(struct kobject *tables_obj,
> if (table_attr->instance > 1 || (table_attr->instance == 1 &&
> !acpi_get_table
> (table_header->signature, 2, &header))) {
> - snprintf(instance_str, sizeof(instance_str), "%u",
> - table_attr->instance);
> - strcat(table_attr->filename, instance_str);
> + sysfs_emit(instance_str, "%u%s", table_attr->instance,
> + table_attr->filename);
> }
>
> table_attr->attr.size = table_header->length;
> @@ -687,7 +686,7 @@ static ssize_t counter_show(struct kobject *kobj,
> acpi_irq_not_handled;
> all_counters[num_gpes + ACPI_NUM_FIXED_EVENTS + COUNT_GPE].count =
> acpi_gpe_count;
> - size = sprintf(buf, "%8u", all_counters[index].count);
> + size = sysfs_emit(buf, "%8u", all_counters[index].count);
>
> /* "gpe_all" or "sci" */
> if (index >= num_gpes + ACPI_NUM_FIXED_EVENTS)
> @@ -698,29 +697,29 @@ static ssize_t counter_show(struct kobject *kobj,
> goto end;
>
> if (status & ACPI_EVENT_FLAG_ENABLE_SET)
> - size += sprintf(buf + size, " EN");
> + size += sysfs_emit(buf + size, " EN");
> else
> - size += sprintf(buf + size, " ");
> + size += sysfs_emit(buf + size, " ");
> if (status & ACPI_EVENT_FLAG_STATUS_SET)
> - size += sprintf(buf + size, " STS");
> + size += sysfs_emit(buf + size, " STS");
> else
> - size += sprintf(buf + size, " ");
> + size += sysfs_emit(buf + size, " ");
>
> if (!(status & ACPI_EVENT_FLAG_HAS_HANDLER))
> - size += sprintf(buf + size, " invalid ");
> + size += sysfs_emit(buf + size, " invalid ");
> else if (status & ACPI_EVENT_FLAG_ENABLED)
> - size += sprintf(buf + size, " enabled ");
> + size += sysfs_emit(buf + size, " enabled ");
> else if (status & ACPI_EVENT_FLAG_WAKE_ENABLED)
> - size += sprintf(buf + size, " wake_enabled");
> + size += sysfs_emit(buf + size, " wake_enabled");
> else
> - size += sprintf(buf + size, " disabled ");
> + size += sysfs_emit(buf + size, " disabled ");
> if (status & ACPI_EVENT_FLAG_MASKED)
> - size += sprintf(buf + size, " masked ");
> + size += sysfs_emit(buf + size, " masked ");
> else
> - size += sprintf(buf + size, " unmasked");
> + size += sysfs_emit(buf + size, " unmasked");
>
> end:
> - size += sprintf(buf + size, "\n");
> + size += sysfs_emit(buf + size, "\n");
> return result ? result : size;
> }
>
> @@ -885,27 +884,27 @@ void acpi_irq_stats_init(void)
> char *name;
>
> if (i < num_gpes)
> - sprintf(buffer, "gpe%02X", i);
> + sysfs_emit(buffer, "gpe%02X", i);
> else if (i == num_gpes + ACPI_EVENT_PMTIMER)
> - sprintf(buffer, "ff_pmtimer");
> + sysfs_emit(buffer, "ff_pmtimer");
> else if (i == num_gpes + ACPI_EVENT_GLOBAL)
> - sprintf(buffer, "ff_gbl_lock");
> + sysfs_emit(buffer, "ff_gbl_lock");
> else if (i == num_gpes + ACPI_EVENT_POWER_BUTTON)
> - sprintf(buffer, "ff_pwr_btn");
> + sysfs_emit(buffer, "ff_pwr_btn");
> else if (i == num_gpes + ACPI_EVENT_SLEEP_BUTTON)
> - sprintf(buffer, "ff_slp_btn");
> + sysfs_emit(buffer, "ff_slp_btn");
> else if (i == num_gpes + ACPI_EVENT_RTC)
> - sprintf(buffer, "ff_rt_clk");
> + sysfs_emit(buffer, "ff_rt_clk");
> else if (i == num_gpes + ACPI_NUM_FIXED_EVENTS + COUNT_GPE)
> - sprintf(buffer, "gpe_all");
> + sysfs_emit(buffer, "gpe_all");
> else if (i == num_gpes + ACPI_NUM_FIXED_EVENTS + COUNT_SCI)
> - sprintf(buffer, "sci");
> + sysfs_emit(buffer, "sci");
> else if (i == num_gpes + ACPI_NUM_FIXED_EVENTS + COUNT_SCI_NOT)
> - sprintf(buffer, "sci_not");
> + sysfs_emit(buffer, "sci_not");
> else if (i == num_gpes + ACPI_NUM_FIXED_EVENTS + COUNT_ERROR)
> - sprintf(buffer, "error");
> + sysfs_emit(buffer, "error");
> else
> - sprintf(buffer, "bug%02X", i);
> + sysfs_emit(buffer, "bug%02X", i);
>
> name = kstrdup(buffer, GFP_KERNEL);
> if (name == NULL)
> @@ -937,7 +936,7 @@ static void __exit interrupt_stats_exit(void)
>
> static ssize_t pm_profile_show(struct kobject *kobj, struct kobj_attribute *attr, char *buf)
> {
> - return sprintf(buf, "%d\n", acpi_gbl_FADT.preferred_profile);
> + return sysfs_emit(buf, "%d\n", acpi_gbl_FADT.preferred_profile);
> }
>
> static const struct kobj_attribute pm_profile_attr = __ATTR_RO(pm_profile);
> @@ -946,7 +945,7 @@ static ssize_t enabled_show(struct kobject *kobj, struct kobj_attribute *attr, c
> {
> struct acpi_hotplug_profile *hotplug = to_acpi_hotplug_profile(kobj);
>
> - return sprintf(buf, "%d\n", hotplug->enabled);
> + return sysfs_emit(buf, "%d\n", hotplug->enabled);
> }
>
> static ssize_t enabled_store(struct kobject *kobj, struct kobj_attribute *attr,
> @@ -1000,7 +999,7 @@ void acpi_sysfs_add_hotplug_profile(struct acpi_hotplug_profile *hotplug,
> static ssize_t force_remove_show(struct kobject *kobj,
> struct kobj_attribute *attr, char *buf)
> {
> - return sprintf(buf, "%d\n", 0);
> + return sysfs_emit(buf, "%d\n", 0);
> }
>
> static ssize_t force_remove_store(struct kobject *kobj,
> --
> 2.50.0
>
>
Powered by blists - more mailing lists