linux-kernel - acpi_string with const char * losing casts

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

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [day] [month] [year] [list]

Message-ID: <3e9bc1431278847ad72fc32960b3687da3f01909.camel@perches.com>
Date:   Sun, 06 Jan 2019 13:52:37 -0800
From:   Joe Perches <joe@...ches.com>
To:     Robert Moore <robert.moore@...el.com>,
        Erik Schmauss <erik.schmauss@...el.com>,
        "Rafael J. Wysocki" <rafael.j.wysocki@...el.com>,
        Len Brown <lenb@...nel.org>
Cc:     linux-acpi@...r.kernel.org, devel@...ica.org,
        linux-kernel <linux-kernel@...r.kernel.org>
Subject: acpi_string with const char * losing casts

include/acpi/actypes.h:typedef char *acpi_string;       /* Null terminated ASCII string */

It seems there are cases where a function with an acpi_string
argument should instead use const acpi_string.

Some acpi function callers cast const char * to non-const to
avoid compiler warnings.

It would be good to use const acpi_string where appropriate in
the generic acpi_<foo> functions instead.

Also acpi_<foo> seems to use acpi_string and char * somewhat
interchangeably.

It would also be good to standardize on one style.

For instance intel-hid.c has:

   static const char *intel_hid_dsm_fn_to_method[INTEL_HID_DSM_FN_MAX] = {
   	NULL,
   	"BTNL",
   	"HDMM",
   	"HDSM",
   	"HDEM",
   	"BTNS",
   	"BTNE",
   	"HEBC",
   	"VGBS",
   	"HEBC"
   };

   [...]

   static bool intel_hid_execute_method(acpi_handle handle,
   				     enum intel_hid_dsm_fn_codes fn_index,
   				     unsigned long long arg)
   {
   	union acpi_object *obj, argv4, req;
   	acpi_status status;
   	char *method_name;

   	if (fn_index <= INTEL_HID_DSM_FN_INVALID ||
   	    fn_index >= INTEL_HID_DSM_FN_MAX)
   		return false;

   	method_name = (char *)intel_hid_dsm_fn_to_method[fn_index];

   [...]

where the const is cast away because

   skip_dsm_exec:
   	status = acpi_execute_simple_method(handle, method_name, arg);

here acpi_execute_simple_method takes a non-const acpi_string method_name.

   include/acpi/acpi_bus.h:acpi_status acpi_execute_simple_method(acpi_handle handle, char *method,
   include/acpi/acpi_bus.h-                                       u64 arg);

and the implementation in drivers/acpi/utils.c does not modify method

   acpi_status acpi_execute_simple_method(acpi_handle handle, char *method,
   				       u64 arg)
   {
   	union acpi_object obj = { .type = ACPI_TYPE_INTEGER };
   	struct acpi_object_list arg_list = { .count = 1, .pointer = &obj, };

   	obj.integer.value = arg;

   	return acpi_evaluate_object(handle, method, &arg_list, NULL);
   }
   EXPORT_SYMBOL(acpi_execute_simple_method);

and acpi_evaluate_object does not modify method either
but may assign it to another const char *relative_pathname

   acpi_status
   acpi_evaluate_object(acpi_handle handle,
   		     acpi_string pathname,
   		     struct acpi_object_list *external_params,
   		     struct acpi_buffer *return_buffer)

   {
   	[...]

   	info->relative_pathname = pathname;