On 2021-10-15T08:58+0300, Denis Pauk wrote:
> +/*
> + * 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, u32 length)
> +{
> + char buffer[ASUSWMI_MAX_BUF_LEN * 2];
> + const char *pos = buffer;
> + const u8 *data = inp + 2;
> + unsigned int i;
> + u32 len;
> +
> + len = min3((u32)ASUSWMI_MAX_BUF_LEN, (length - 2) / 4, (u32)inp[0] / 4);
This will still access out of bounds memory when length == 0.
> +
> + 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_find_sensor_index(const struct asus_wmi_ec_info *ec,
> + enum hwmon_sensor_types type, int channel)
> +{
> + int i;
> +
> + for (i = 0; i < ec->nr_sensors; i++) {
> + if (known_ec_sensors[ec->sensors[i].info_index].type == type) {
> + if (channel == 0)
> + return i;
> +
> + channel--;
> + }
> + }
> + return -EINVAL;
> +}
> +
> +static int asus_wmi_ec_get_cached_value_or_update(int sensor_index,
> + struct asus_wmi_sensors *state,
> + u32 *value)
In both drivers there is function like this where the caller is responsible
for halding the mutex. Maybe the mutex could be handled by the function
directly.
> +{
> + int ret;
> +
> + if (time_after(jiffies, state->ec.last_updated + HZ)) {
> + ret = asus_wmi_ec_block_read(ASUSWMI_METHODID_BREC,
> + state->ec.read_arg,
> + state->ec.read_buffer);
> + if (ret)
> + return ret;
> +
> + asus_wmi_ec_update_ec_sensors(&state->ec);
> + state->ec.last_updated = jiffies;
> + }
> +
> + *value = state->ec.sensors[sensor_index].cached_value;
> + return 0;
> +}
> +static umode_t asus_wmi_ec_hwmon_is_visible(const void *drvdata,
> + enum hwmon_sensor_types type, u32 attr,
> + int channel)
> +{
> + int index;
> + const struct asus_wmi_sensors *sensor_data = drvdata;
> +
> + index = asus_wmi_ec_find_sensor_index(&sensor_data->ec, type, channel);
> +
> + return index == 0xff ? 0 : 0444;
Should this not check for index < 0?
> +}
> +static int asus_wmi_probe(struct wmi_device *wdev, const void *context)
> +{
> + struct asus_wmi_sensors *sensor_data;
> + struct asus_wmi_data *board_sensors;
> + const enum known_ec_sensor *bsi;
> + const struct dmi_system_id *dmi_id;
> + struct device *dev = &wdev->dev;
> +
> + dmi_id = dmi_first_match(asus_wmi_ec_dmi_table);
> + if (!dmi_id)
> + return -ENODEV;
> +
> + board_sensors = dmi_id->driver_data;
> + if (!board_sensors)
> + return -ENODEV;
> +
> + bsi = board_sensors->known_board_sensors;
> +
> + sensor_data = devm_kzalloc(dev, sizeof(struct asus_wmi_sensors),
> + GFP_KERNEL);
sizeof(*sensor_data);
> + 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, bsi);
> +}
