On 2021-10-14T10:25+0300, Denis Pauk wrote:
> +static struct asus_wmi_data *board_sensors;
> +
> +static int __init asus_wmi_dmi_matched(const struct dmi_system_id *d)
> +{
> + board_sensors = d->driver_data;
> + return 0;
> +}
> +
> +#define DMI_EXACT_MATCH_ASUS_BOARD_NAME(name, sensors) \
> + { \
> + .callback = asus_wmi_dmi_matched, \
> + .matches = { \
> + DMI_EXACT_MATCH(DMI_BOARD_VENDOR, \
> + "ASUSTeK COMPUTER INC."), \
> + DMI_EXACT_MATCH(DMI_BOARD_NAME, name), \
> + }, \
> + .driver_data = sensors, \
> + }
> +
> +static const struct dmi_system_id asus_wmi_ec_dmi_table[] = {
> + DMI_EXACT_MATCH_ASUS_BOARD_NAME("PRIME X570-PRO", &sensors_board_PW_X570_P),
> + DMI_EXACT_MATCH_ASUS_BOARD_NAME("Pro WS X570-ACE", &sensors_board_PW_X570_A),
> + DMI_EXACT_MATCH_ASUS_BOARD_NAME("ROG CROSSHAIR VIII DARK HERO", &sensors_board_R_C8DH),
> + DMI_EXACT_MATCH_ASUS_BOARD_NAME("ROG CROSSHAIR VIII FORMULA", &sensors_board_R_C8F),
> + DMI_EXACT_MATCH_ASUS_BOARD_NAME("ROG CROSSHAIR VIII HERO", &sensors_board_R_C8H),
> + DMI_EXACT_MATCH_ASUS_BOARD_NAME("ROG STRIX B550-E GAMING", &sensors_board_RS_B550_E_G),
> + DMI_EXACT_MATCH_ASUS_BOARD_NAME("ROG STRIX X570-E GAMING", &sensors_board_RS_X570_E_G),
> + {}
> +};
> +MODULE_DEVICE_TABLE(dmi, asus_wmi_ec_dmi_table);
> +
> +/*
> + * The next four functions converts to/from BRxx string argument format
> + * The format of the string is as follows:
> + * The string consists of two-byte UTF-16 characters
> + * The value of the very first byte int the string is equal to the total length
> + * of the next string in bytes, thus excluding the first two-byte character
> + * The rest of the string encodes pairs of (bank, index) pairs, where both
> + * values are byte-long (0x00 to 0xFF)
> + * Numbers are encoded as UTF-16 hex values
> + */
> +static void asus_wmi_ec_decode_reply_buffer(const u8 *inp, u8 *out)
> +{
> + unsigned int len = ACPI_MIN(ASUS_WMI_MAX_BUF_LEN, inp[0] / 4);
The general `min()` would be better.
(Or `min3()` when also validating the ACPI buffer, see below)
> + char buffer[ASUS_WMI_MAX_BUF_LEN * 2];
> + const char *pos = buffer;
> + const u8 *data = inp + 2;
> + unsigned int i;
> +
> + utf16s_to_utf8s((wchar_t *)data, len * 2, UTF16_LITTLE_ENDIAN, buffer, len * 2);
> +
> + for (i = 0; i < len; i++, pos += 2)
> + out[i] = (hex_to_bin(pos[0]) << 4) + hex_to_bin(pos[1]);
> +}
> +
> +static int asus_wmi_ec_block_read(u32 method_id, char *query, u8 *out)
> +{
> +#if IS_ENABLED(CONFIG_ACPI_WMI)
> + struct acpi_buffer output = { ACPI_ALLOCATE_BUFFER,
> + NULL };
> + struct acpi_buffer input;
> + union acpi_object *obj;
> + acpi_status status;
> +
> + /* the first byte of the BRxx() argument string has to be the string size */
> + input.length = (acpi_size)query[0] + 2;
> + input.pointer = query;
> + status = wmi_evaluate_method(ASUSWMI_MONITORING_GUID, 0, method_id, &input,
> + &output);
> + if (ACPI_FAILURE(status))
> + return -EIO;
> +
> + obj = output.pointer;
> + if (!obj || obj->type != ACPI_TYPE_BUFFER) {
> + acpi_os_free(obj);
> + return -EIO;
> + }
> + asus_wmi_ec_decode_reply_buffer(obj->buffer.pointer, out);
If this buffer is empty or the length in the first byte is incorrect then
out-of-bound memory will be accessed.
> + acpi_os_free(obj);
> + return 0;
> +#else
> + return -EOPNOTSUPP;
> +#endif
> +}
> +
> +static int asus_wmi_ec_configure_sensor_setup(struct device *dev,
> + struct asus_wmi_sensors *sensor_data)
> +{
> [..]
> +}
> +
> +static int asus_wmi_probe(struct wmi_device *wdev, const void *context)
> +{
> + struct asus_wmi_sensors *sensor_data;
> + struct device *dev = &wdev->dev;
> +
> + if (!dmi_check_system(asus_wmi_ec_dmi_table))
> + return -ENODEV;
Instead of using the callback to assign the static variable `board_sensors` you
could use `dmi_first_match()` here and pass around the result explicitly.
This would remove the need for a static variable and should cut the code down a
bit.
> + sensor_data = devm_kzalloc(dev, sizeof(struct asus_wmi_sensors),
> + GFP_KERNEL);
> + if (!sensor_data)
> + return -ENOMEM;
> +
> + mutex_init(&sensor_data->lock);
> +
> + dev_set_drvdata(dev, sensor_data);
> +
> + /* ec init */
> + return asus_wmi_ec_configure_sensor_setup(dev,
> + sensor_data);
> +}
> +
