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);
+}