* [PATCH] ACPI / sysfs: Replace deprecated and unsafe functions with sysfs_emit
@ 2025-06-24 13:37 Brahmajit Das
2025-06-24 13:46 ` Rafael J. Wysocki
2025-06-26 2:51 ` kernel test robot
0 siblings, 2 replies; 6+ messages in thread
From: Brahmajit Das @ 2025-06-24 13:37 UTC (permalink / raw)
To: linux-hardening, linux-kernel, linux-acpi
Cc: rafael, lenb, lv.zheng, kees, rui.zhang, len.brown
acpi/sysfs.c has many instances of unsafe or deprecated functions such
as sprintf, strcpy. This patch relaces them with sysfs_emit to safer
alternavtive and better following of kernel API.
Signed-off-by: Brahmajit Das <listout@listout.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
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH] ACPI / sysfs: Replace deprecated and unsafe functions with sysfs_emit
2025-06-24 13:37 [PATCH] ACPI / sysfs: Replace deprecated and unsafe functions with sysfs_emit Brahmajit Das
@ 2025-06-24 13:46 ` Rafael J. Wysocki
2025-06-24 13:56 ` Brahmajit Das
2025-06-24 14:16 ` Brahmajit Das
2025-06-26 2:51 ` kernel test robot
1 sibling, 2 replies; 6+ messages in thread
From: Rafael J. Wysocki @ 2025-06-24 13:46 UTC (permalink / raw)
To: Brahmajit Das
Cc: linux-hardening, linux-kernel, linux-acpi, rafael, lenb, lv.zheng,
kees, rui.zhang, len.brown
On Tue, Jun 24, 2025 at 3:38 PM Brahmajit Das <listout@listout.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@listout.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
>
>
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] ACPI / sysfs: Replace deprecated and unsafe functions with sysfs_emit
2025-06-24 13:46 ` Rafael J. Wysocki
@ 2025-06-24 13:56 ` Brahmajit Das
2025-06-24 14:16 ` Brahmajit Das
1 sibling, 0 replies; 6+ messages in thread
From: Brahmajit Das @ 2025-06-24 13:56 UTC (permalink / raw)
To: Rafael J. Wysocki
Cc: linux-hardening, linux-kernel, linux-acpi, lenb, lv.zheng, kees,
rui.zhang, len.brown
On 24.06.2025 15:46, Rafael J. Wysocki wrote:
> On Tue, Jun 24, 2025 at 3:38 PM Brahmajit Das <listout@listout.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?
>
I meant to send this as RFC. Sorry.
But I guess the questions still holds.
--
Regards,
listout
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] ACPI / sysfs: Replace deprecated and unsafe functions with sysfs_emit
2025-06-24 13:46 ` Rafael J. Wysocki
2025-06-24 13:56 ` Brahmajit Das
@ 2025-06-24 14:16 ` Brahmajit Das
1 sibling, 0 replies; 6+ messages in thread
From: Brahmajit Das @ 2025-06-24 14:16 UTC (permalink / raw)
To: Rafael J. Wysocki
Cc: linux-hardening, linux-kernel, linux-acpi, lenb, lv.zheng, kees,
rui.zhang, len.brown
On 24.06.2025 15:46, Rafael J. Wysocki wrote:
> On Tue, Jun 24, 2025 at 3:38 PM Brahmajit Das <listout@listout.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?
>
The previous patch's idea came while I was working to remove strcpy from
acpi/sysfs.c. But I guess this is not a good way of sending patch and me
being a new comer didn't help that I didn't completely tested the patch
before sending, even it was meant for RFC.
I vaguely remember a tread by GHK where he asked to leave out old code
and only work on new code that you've tested. So I'll follow that for
now until I've learnt testing my changes properly.
And again sorry.
I'm working on a patch that replaces deprecated strcpy (which according
to kernel docs) with sysfs_emit. And looks like:
diff --git a/drivers/acpi/sysfs.c b/drivers/acpi/sysfs.c
index a48ebbf768f9..7ce90998ab97 100644
--- a/drivers/acpi/sysfs.c
+++ b/drivers/acpi/sysfs.c
@@ -181,10 +181,9 @@ 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);
}
/* Restore the original tracer state */
I guess I'll keep this, instead of replacing every instance of sprint
with sysfs_emit blindly.
--
Regards,
listout
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH] ACPI / sysfs: Replace deprecated and unsafe functions with sysfs_emit
2025-06-24 13:37 [PATCH] ACPI / sysfs: Replace deprecated and unsafe functions with sysfs_emit Brahmajit Das
2025-06-24 13:46 ` Rafael J. Wysocki
@ 2025-06-26 2:51 ` kernel test robot
2025-06-26 3:33 ` Brahmajit Das
1 sibling, 1 reply; 6+ messages in thread
From: kernel test robot @ 2025-06-26 2:51 UTC (permalink / raw)
To: Brahmajit Das
Cc: oe-lkp, lkp, linux-acpi, linux-hardening, linux-kernel, rafael,
lenb, lv.zheng, kees, rui.zhang, len.brown, oliver.sang
Hello,
kernel test robot noticed "WARNING:at_fs/sysfs/file.c:#sysfs_emit" on:
commit: 74191212ddb1a82ed54ddd75fcd820f3df79abef ("[PATCH] ACPI / sysfs: Replace deprecated and unsafe functions with sysfs_emit")
url: https://github.com/intel-lab-lkp/linux/commits/Brahmajit-Das/ACPI-sysfs-Replace-deprecated-and-unsafe-functions-with-sysfs_emit/20250624-213919
base: https://git.kernel.org/cgit/linux/kernel/git/rafael/linux-pm.git linux-next
patch link: https://lore.kernel.org/all/20250624133739.25215-1-listout@listout.xyz/
patch subject: [PATCH] ACPI / sysfs: Replace deprecated and unsafe functions with sysfs_emit
in testcase: boot
config: x86_64-rhel-9.4
compiler: gcc-12
test machine: qemu-system-x86_64 -enable-kvm -cpu SandyBridge -smp 2 -m 16G
(please refer to attached dmesg/kmsg for entire log/backtrace)
+----------------------------------------+------------+------------+
| | 3cd1e195f0 | 74191212dd |
+----------------------------------------+------------+------------+
| WARNING:at_fs/sysfs/file.c:#sysfs_emit | 0 | 18 |
| RIP:sysfs_emit | 0 | 18 |
+----------------------------------------+------------+------------+
If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <oliver.sang@intel.com>
| Closes: https://lore.kernel.org/oe-lkp/202506261036.895ef959-lkp@intel.com
[ 0.410439][ T1] ------------[ cut here ]------------
[ 0.410995][ T1] invalid sysfs_emit: buf:(____ptrval____)
[ 0.411411][ T1] WARNING: CPU: 0 PID: 1 at fs/sysfs/file.c:767 sysfs_emit (fs/sysfs/file.c:767)
[ 0.412230][ T1] Modules linked in:
[ 0.412660][ T1] CPU: 0 UID: 0 PID: 1 Comm: swapper/0 Not tainted 6.16.0-rc3-00031-g74191212ddb1 #1 VOLUNTARY
[ 0.413504][ T1] Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS 1.16.2-debian-1.16.2-1 04/01/2014
[ 0.414499][ T1] RIP: 0010:sysfs_emit (fs/sysfs/file.c:767)
[ 0.415012][ T1] Code: 10 e8 d5 8e b4 00 48 8b 54 24 18 65 48 2b 15 40 f5 69 02 75 1b c9 e9 d0 7c b6 00 48 89 fe 48 c7 c7 d9 be b6 ae e8 71 86 b3 ff <0f> 0b 31 c0 eb d6 e8 a6 88 b5 00 66 66 2e 0f 1f 84 00 00 00 00 00
All code
========
0: 10 e8 adc %ch,%al
2: d5 (bad)
3: 8e b4 00 48 8b 54 24 mov 0x24548b48(%rax,%rax,1),%?
a: 18 65 48 sbb %ah,0x48(%rbp)
d: 2b 15 40 f5 69 02 sub 0x269f540(%rip),%edx # 0x269f553
13: 75 1b jne 0x30
15: c9 leave
16: e9 d0 7c b6 00 jmp 0xb67ceb
1b: 48 89 fe mov %rdi,%rsi
1e: 48 c7 c7 d9 be b6 ae mov $0xffffffffaeb6bed9,%rdi
25: e8 71 86 b3 ff call 0xffffffffffb3869b
2a:* 0f 0b ud2 <-- trapping instruction
2c: 31 c0 xor %eax,%eax
2e: eb d6 jmp 0x6
30: e8 a6 88 b5 00 call 0xb588db
35: 66 66 2e 0f 1f 84 00 data16 cs nopw 0x0(%rax,%rax,1)
3c: 00 00 00 00
Code starting with the faulting instruction
===========================================
0: 0f 0b ud2
2: 31 c0 xor %eax,%eax
4: eb d6 jmp 0xffffffffffffffdc
6: e8 a6 88 b5 00 call 0xb588b1
b: 66 66 2e 0f 1f 84 00 data16 cs nopw 0x0(%rax,%rax,1)
12: 00 00 00 00
[ 0.415674][ T1] RSP: 0000:ffffcdc1c0013cb8 EFLAGS: 00010282
[ 0.416230][ T1] RAX: 0000000000000000 RBX: 0000000000000000 RCX: c0000000ffff7fff
[ 0.417231][ T1] RDX: 0000000000000000 RSI: 00000000ffff7fff RDI: ffffffffaee63ba0
[ 0.418233][ T1] RBP: ffffcdc1c0013d08 R08: 0000000000000000 R09: 0000000000000003
[ 0.419233][ T1] R10: ffffcdc1c0013b58 R11: ffffffffaf1e3be8 R12: ffff8c28408940a0
[ 0.420232][ T1] R13: ffffffffadb37cd0 R14: 00000000000004e5 R15: 0000000000000000
[ 0.421234][ T1] FS: 0000000000000000(0000) GS:ffff8c2bbfdce000(0000) knlGS:0000000000000000
[ 0.422237][ T1] CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
[ 0.423179][ T1] CR2: ffff8c2b7ffff000 CR3: 000000010f024000 CR4: 00000000000406f0
[ 0.423631][ T1] DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000
[ 0.424609][ T1] DR3: 0000000000000000 DR6: 00000000fffe0ff0 DR7: 0000000000000400
[ 0.425613][ T1] Call Trace:
[ 0.426168][ T1] <TASK>
[ 0.426431][ T1] acpi_irq_stats_init (drivers/acpi/sysfs.c:887)
[ 0.427234][ T1] ? acpi_os_signal_semaphore (drivers/acpi/osl.c:1320)
[ 0.428079][ T1] ? acpi_ut_release_mutex (drivers/acpi/acpica/utmutex.c:329)
[ 0.428478][ T1] acpi_os_install_interrupt_handler (drivers/acpi/osl.c:568)
[ 0.429235][ T1] ? __pfx_acpi_init (drivers/acpi/bus.c:1440)
[ 0.429990][ T1] acpi_ev_install_xrupt_handlers (drivers/acpi/acpica/evevent.c:95)
[ 0.430500][ T1] ? __pfx_acpi_init (drivers/acpi/bus.c:1440)
[ 0.431233][ T1] acpi_bus_init (drivers/acpi/bus.c:1362)
[ 0.431971][ T1] ? acpi_ut_release_mutex (drivers/acpi/acpica/utmutex.c:329)
[ 0.432485][ T1] ? acpi_install_address_space_handler_internal+0x64/0xb0
[ 0.433605][ T1] ? __pfx_acpi_init (drivers/acpi/bus.c:1440)
[ 0.434234][ T1] ? __pfx_acpi_init (drivers/acpi/bus.c:1440)
[ 0.434989][ T1] acpi_init (drivers/acpi/bus.c:1456)
[ 0.435382][ T1] ? __pfx_scan_for_dmi_ipmi (drivers/char/ipmi/ipmi_dmi.c:215)
[ 0.435948][ T1] ? __pfx_acpi_init (drivers/acpi/bus.c:1440)
[ 0.436380][ T1] do_one_initcall (init/main.c:1274)
[ 0.436875][ T1] do_initcalls (init/main.c:1335 init/main.c:1352)
[ 0.437233][ T1] kernel_init_freeable (init/main.c:1588)
[ 0.437770][ T1] ? __pfx_kernel_init (init/main.c:1466)
[ 0.438231][ T1] kernel_init (init/main.c:1476)
[ 0.438714][ T1] ret_from_fork (arch/x86/kernel/process.c:154)
[ 0.439203][ T1] ? __pfx_kernel_init (init/main.c:1466)
[ 0.439386][ T1] ret_from_fork_asm (arch/x86/entry/entry_64.S:258)
[ 0.439885][ T1] </TASK>
[ 0.440230][ T1] ---[ end trace 0000000000000000 ]---
The kernel config and materials to reproduce are available at:
https://download.01.org/0day-ci/archive/20250626/202506261036.895ef959-lkp@intel.com
--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] ACPI / sysfs: Replace deprecated and unsafe functions with sysfs_emit
2025-06-26 2:51 ` kernel test robot
@ 2025-06-26 3:33 ` Brahmajit Das
0 siblings, 0 replies; 6+ messages in thread
From: Brahmajit Das @ 2025-06-26 3:33 UTC (permalink / raw)
To: kernel test robot
Cc: oe-lkp, lkp, linux-acpi, linux-hardening, linux-kernel, rafael,
lenb, lv.zheng, kees, rui.zhang, len.brown
On 26.06.2025 10:51, kernel test robot wrote:
>
>
> Hello,
>
> kernel test robot noticed "WARNING:at_fs/sysfs/file.c:#sysfs_emit" on:
>
> commit: 74191212ddb1a82ed54ddd75fcd820f3df79abef ("[PATCH] ACPI / sysfs: Replace deprecated and unsafe functions with sysfs_emit")
> url: https://github.com/intel-lab-lkp/linux/commits/Brahmajit-Das/ACPI-sysfs-Replace-deprecated-and-unsafe-functions-with-sysfs_emit/20250624-213919
> base: https://git.kernel.org/cgit/linux/kernel/git/rafael/linux-pm.git linux-next
> patch link: https://lore.kernel.org/all/20250624133739.25215-1-listout@listout.xyz/
> patch subject: [PATCH] ACPI / sysfs: Replace deprecated and unsafe functions with sysfs_emit
>
Please do not use this patch.
--
Regards,
listout
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2025-06-26 3:33 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-06-24 13:37 [PATCH] ACPI / sysfs: Replace deprecated and unsafe functions with sysfs_emit Brahmajit Das
2025-06-24 13:46 ` Rafael J. Wysocki
2025-06-24 13:56 ` Brahmajit Das
2025-06-24 14:16 ` Brahmajit Das
2025-06-26 2:51 ` kernel test robot
2025-06-26 3:33 ` Brahmajit Das
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).