hp-wmi: allow setting input and output buffer sizes separately

Split buffersize parameter of hp_wmi_perform_query to insize and outsize. Existing callers are changed to use the same value for insize and outsize to avoid any regressions, with the exception of hp_wmi_set_block where the output buffer is unused and therefore outsize is set to 0 (this change is not seen by BIOS code). The maximum input buffer size is kept at 4 bytes as per struct bios_args. Some commands exist that take longer buffers, but they haven't been implemented. The data portion of bios_args can be trivially made dynamically allocated later when such larger buffers become needed. Signed-off-by: Anssi Hannula <anssi.hannula@iki.fi> Signed-off-by: Matthew Garrett <mjg@redhat.com>

hp-wmi: allow setting input and output buffer sizes separately
Split buffersize parameter of hp_wmi_perform_query to insize and outsize. Existing callers are changed to use the same value for insize and outsize to avoid any regressions, with the exception of hp_wmi_set_block where the output buffer is unused and therefore outsize is set to 0 (this change is not seen by BIOS code). The maximum input buffer size is kept at 4 bytes as per struct bios_args. Some commands exist that take longer buffers, but they haven't been implemented. The data portion of bios_args can be trivially made dynamically allocated later when such larger buffers become needed. Signed-off-by: Anssi Hannula <anssi.hannula@iki.fi> Signed-off-by: Matthew Garrett <mjg@redhat.com>
Anssi Hannula · Matthew Garrett
1 parent 25bb067a08
Showing 1 changed file with 39 additions and 24 deletions Side-by-side Diff
drivers/platform/x86/hp-wmi.c
@@ -86,7 +86,6 @@
 struct bios_return {
 	u32 sigpass;
 	u32 return_code;
-	u32 value;
 };
  
 enum hp_return_value {
@@ -136,7 +135,8 @@
  * query:	The commandtype -> What should be queried
  * write:	The command -> 0 read, 1 write, 3 ODM specific
  * buffer:	Buffer used as input and/or output
- * buffersize:	Size of buffer
+ * insize:	Size of input buffer
+ * outsize:	Size of output buffer
  *
  * returns zero on success
  *         an HP WMI query specific error code (which is positive)
  
  
  
  
@@ -147,23 +147,28 @@
  *       size. E.g. Battery info query (0x7) is defined to have 1 byte input
  *       and 128 byte output. The caller would do:
  *       buffer = kzalloc(128, GFP_KERNEL);
- *       ret = hp_wmi_perform_query(0x7, 0, buffer, 128)
+ *       ret = hp_wmi_perform_query(0x7, 0, buffer, 1, 128)
  */
-static int hp_wmi_perform_query(int query, int write, u32 *buffer,
-				int buffersize)
+static int hp_wmi_perform_query(int query, int write, void *buffer,
+				int insize, int outsize)
 {
-	struct bios_return bios_return;
+	struct bios_return *bios_return;
+	int actual_outsize;
 	union acpi_object *obj;
 	struct bios_args args = {
 		.signature = 0x55434553,
 		.command = write ? 0x2 : 0x1,
 		.commandtype = query,
-		.datasize = buffersize,
-		.data = *buffer,
+		.datasize = insize,
+		.data = 0,
 	};
 	struct acpi_buffer input = { sizeof(struct bios_args), &args };
 	struct acpi_buffer output = { ACPI_ALLOCATE_BUFFER, NULL };
  
+	if (WARN_ON(insize > sizeof(args.data)))
+		return -EINVAL;
+	memcpy(&args.data, buffer, insize);
+
 	wmi_evaluate_method(HPWMI_BIOS_GUID, 0, 0x3, &input, &output);
  
 	obj = output.pointer;
  
  
  
  
  
@@ -175,19 +180,26 @@
 		return -EINVAL;
 	}
  
-	bios_return = *((struct bios_return *)obj->buffer.pointer);
+	bios_return = (struct bios_return *)obj->buffer.pointer;
  
-	if (bios_return.return_code) {
-		if (bios_return.return_code != HPWMI_RET_UNKNOWN_CMDTYPE)
+	if (bios_return->return_code) {
+		if (bios_return->return_code != HPWMI_RET_UNKNOWN_CMDTYPE)
 			printk(KERN_WARNING PREFIX "query 0x%x returned "
 						   "error 0x%x\n",
-			       query, bios_return.return_code);
+			       query, bios_return->return_code);
 		kfree(obj);
-		return bios_return.return_code;
+		return bios_return->return_code;
 	}
  
-	memcpy(buffer, &bios_return.value, sizeof(bios_return.value));
+	if (!outsize) {
+		/* ignore output data */
+		kfree(obj);
+		return 0;
+	}
  
+	actual_outsize = min(outsize, (int)(obj->buffer.length - sizeof(*bios_return)));
+	memcpy(buffer, obj->buffer.pointer + sizeof(*bios_return), actual_outsize);
+	memset(buffer + actual_outsize, 0, outsize - actual_outsize);
 	kfree(obj);
 	return 0;
 }
@@ -196,7 +208,7 @@
 {
 	int state = 0;
 	int ret = hp_wmi_perform_query(HPWMI_DISPLAY_QUERY, 0, &state,
-				       sizeof(state));
+				       sizeof(state), sizeof(state));
 	if (ret)
 		return -EINVAL;
 	return state;
@@ -206,7 +218,7 @@
 {
 	int state = 0;
 	int ret = hp_wmi_perform_query(HPWMI_HDDTEMP_QUERY, 0, &state,
-				       sizeof(state));
+				       sizeof(state), sizeof(state));
 	if (ret)
 		return -EINVAL;
 	return state;
@@ -216,7 +228,7 @@
 {
 	int state = 0;
 	int ret = hp_wmi_perform_query(HPWMI_ALS_QUERY, 0, &state,
-				       sizeof(state));
+				       sizeof(state), sizeof(state));
 	if (ret)
 		return -EINVAL;
 	return state;
@@ -226,7 +238,7 @@
 {
 	int state = 0;
 	int ret = hp_wmi_perform_query(HPWMI_HARDWARE_QUERY, 0, &state,
-				       sizeof(state));
+				       sizeof(state), sizeof(state));
  
 	if (ret)
 		return -EINVAL;
@@ -238,7 +250,7 @@
 {
 	int state = 0;
 	int ret = hp_wmi_perform_query(HPWMI_HARDWARE_QUERY, 0, &state,
-				       sizeof(state));
+				       sizeof(state), sizeof(state));
 	if (ret)
 		return ret;
  
@@ -252,7 +264,7 @@
 	int ret;
  
 	ret = hp_wmi_perform_query(HPWMI_WIRELESS_QUERY, 1,
-				   &query, sizeof(query));
+				   &query, sizeof(query), 0);
 	if (ret)
 		return -EINVAL;
 	return 0;
@@ -267,7 +279,8 @@
 	int wireless = 0;
 	int mask;
 	hp_wmi_perform_query(HPWMI_WIRELESS_QUERY, 0,
-			     &wireless, sizeof(wireless));
+			     &wireless, sizeof(wireless),
+			     sizeof(wireless));
 	/* TBD: Pass error */
  
 	mask = 0x200 << (r * 8);
@@ -283,7 +296,8 @@
 	int wireless = 0;
 	int mask;
 	hp_wmi_perform_query(HPWMI_WIRELESS_QUERY, 0,
-			     &wireless, sizeof(wireless));
+			     &wireless, sizeof(wireless),
+			     sizeof(wireless));
 	/* TBD: Pass error */
  
 	mask = 0x800 << (r * 8);
@@ -344,7 +358,7 @@
 {
 	u32 tmp = simple_strtoul(buf, NULL, 10);
 	int ret = hp_wmi_perform_query(HPWMI_ALS_QUERY, 1, &tmp,
-				       sizeof(tmp));
+				       sizeof(tmp), sizeof(tmp));
 	if (ret)
 		return -EINVAL;
  
@@ -417,6 +431,7 @@
 	case HPWMI_BEZEL_BUTTON:
 		ret = hp_wmi_perform_query(HPWMI_HOTKEY_QUERY, 0,
 					   &key_code,
+					   sizeof(key_code),
 					   sizeof(key_code));
 		if (ret)
 			break;
@@ -523,7 +538,7 @@
 	int wireless = 0;
  
 	err = hp_wmi_perform_query(HPWMI_WIRELESS_QUERY, 0, &wireless,
-				   sizeof(wireless));
+				   sizeof(wireless), sizeof(wireless));
 	if (err)
 		return err;
...	...	@@ -86,7 +86,6 @@
86	86	struct bios_return {
87	87	u32 sigpass;
88	88	u32 return_code;
89		- u32 value;
90	89	};
91	90
92	91	enum hp_return_value {
...	...	@@ -136,7 +135,8 @@
136	135	* query: The commandtype -> What should be queried
137	136	* write: The command -> 0 read, 1 write, 3 ODM specific
138	137	* buffer: Buffer used as input and/or output
139		- * buffersize: Size of buffer
	138	+ * insize: Size of input buffer
	139	+ * outsize: Size of output buffer
140	140	*
141	141	* returns zero on success
142	142	* an HP WMI query specific error code (which is positive)
143	143
144	144
145	145
146	146
...	...	@@ -147,23 +147,28 @@
147	147	* size. E.g. Battery info query (0x7) is defined to have 1 byte input
148	148	* and 128 byte output. The caller would do:
149	149	* buffer = kzalloc(128, GFP_KERNEL);
150		- * ret = hp_wmi_perform_query(0x7, 0, buffer, 128)
	150	+ * ret = hp_wmi_perform_query(0x7, 0, buffer, 1, 128)
151	151	*/
152		-static int hp_wmi_perform_query(int query, int write, u32 *buffer,
153		- int buffersize)
	152	+static int hp_wmi_perform_query(int query, int write, void *buffer,
	153	+ int insize, int outsize)
154	154	{
155		- struct bios_return bios_return;
	155	+ struct bios_return *bios_return;
	156	+ int actual_outsize;
156	157	union acpi_object *obj;
157	158	struct bios_args args = {
158	159	.signature = 0x55434553,
159	160	.command = write ? 0x2 : 0x1,
160	161	.commandtype = query,
161		- .datasize = buffersize,
162		- .data = *buffer,
	162	+ .datasize = insize,
	163	+ .data = 0,
163	164	};
164	165	struct acpi_buffer input = { sizeof(struct bios_args), &args };
165	166	struct acpi_buffer output = { ACPI_ALLOCATE_BUFFER, NULL };
166	167
	168	+ if (WARN_ON(insize > sizeof(args.data)))
	169	+ return -EINVAL;
	170	+ memcpy(&args.data, buffer, insize);
	171	+
167	172	wmi_evaluate_method(HPWMI_BIOS_GUID, 0, 0x3, &input, &output);
168	173
169	174	obj = output.pointer;
170	175
171	176
172	177
173	178
174	179
...	...	@@ -175,19 +180,26 @@
175	180	return -EINVAL;
176	181	}
177	182
178		- bios_return = ((struct bios_return )obj->buffer.pointer);
	183	+ bios_return = (struct bios_return *)obj->buffer.pointer;
179	184
180		- if (bios_return.return_code) {
181		- if (bios_return.return_code != HPWMI_RET_UNKNOWN_CMDTYPE)
	185	+ if (bios_return->return_code) {
	186	+ if (bios_return->return_code != HPWMI_RET_UNKNOWN_CMDTYPE)
182	187	printk(KERN_WARNING PREFIX "query 0x%x returned "
183	188	"error 0x%x\n",
184		- query, bios_return.return_code);
	189	+ query, bios_return->return_code);
185	190	kfree(obj);
186		- return bios_return.return_code;
	191	+ return bios_return->return_code;
187	192	}
188	193
189		- memcpy(buffer, &bios_return.value, sizeof(bios_return.value));
	194	+ if (!outsize) {
	195	+ /* ignore output data */
	196	+ kfree(obj);
	197	+ return 0;
	198	+ }
190	199
	200	+ actual_outsize = min(outsize, (int)(obj->buffer.length - sizeof(*bios_return)));
	201	+ memcpy(buffer, obj->buffer.pointer + sizeof(*bios_return), actual_outsize);
	202	+ memset(buffer + actual_outsize, 0, outsize - actual_outsize);
191	203	kfree(obj);
192	204	return 0;
193	205	}
...	...	@@ -196,7 +208,7 @@
196	208	{
197	209	int state = 0;
198	210	int ret = hp_wmi_perform_query(HPWMI_DISPLAY_QUERY, 0, &state,
199		- sizeof(state));
	211	+ sizeof(state), sizeof(state));
200	212	if (ret)
201	213	return -EINVAL;
202	214	return state;
...	...	@@ -206,7 +218,7 @@
206	218	{
207	219	int state = 0;
208	220	int ret = hp_wmi_perform_query(HPWMI_HDDTEMP_QUERY, 0, &state,
209		- sizeof(state));
	221	+ sizeof(state), sizeof(state));
210	222	if (ret)
211	223	return -EINVAL;
212	224	return state;
...	...	@@ -216,7 +228,7 @@
216	228	{
217	229	int state = 0;
218	230	int ret = hp_wmi_perform_query(HPWMI_ALS_QUERY, 0, &state,
219		- sizeof(state));
	231	+ sizeof(state), sizeof(state));
220	232	if (ret)
221	233	return -EINVAL;
222	234	return state;
...	...	@@ -226,7 +238,7 @@
226	238	{
227	239	int state = 0;
228	240	int ret = hp_wmi_perform_query(HPWMI_HARDWARE_QUERY, 0, &state,
229		- sizeof(state));
	241	+ sizeof(state), sizeof(state));
230	242
231	243	if (ret)
232	244	return -EINVAL;
...	...	@@ -238,7 +250,7 @@
238	250	{
239	251	int state = 0;
240	252	int ret = hp_wmi_perform_query(HPWMI_HARDWARE_QUERY, 0, &state,
241		- sizeof(state));
	253	+ sizeof(state), sizeof(state));
242	254	if (ret)
243	255	return ret;
244	256
...	...	@@ -252,7 +264,7 @@
252	264	int ret;
253	265
254	266	ret = hp_wmi_perform_query(HPWMI_WIRELESS_QUERY, 1,
255		- &query, sizeof(query));
	267	+ &query, sizeof(query), 0);
256	268	if (ret)
257	269	return -EINVAL;
258	270	return 0;
...	...	@@ -267,7 +279,8 @@
267	279	int wireless = 0;
268	280	int mask;
269	281	hp_wmi_perform_query(HPWMI_WIRELESS_QUERY, 0,
270		- &wireless, sizeof(wireless));
	282	+ &wireless, sizeof(wireless),
	283	+ sizeof(wireless));
271	284	/* TBD: Pass error */
272	285
273	286	mask = 0x200 << (r * 8);
...	...	@@ -283,7 +296,8 @@
283	296	int wireless = 0;
284	297	int mask;
285	298	hp_wmi_perform_query(HPWMI_WIRELESS_QUERY, 0,
286		- &wireless, sizeof(wireless));
	299	+ &wireless, sizeof(wireless),
	300	+ sizeof(wireless));
287	301	/* TBD: Pass error */
288	302
289	303	mask = 0x800 << (r * 8);
...	...	@@ -344,7 +358,7 @@
344	358	{
345	359	u32 tmp = simple_strtoul(buf, NULL, 10);
346	360	int ret = hp_wmi_perform_query(HPWMI_ALS_QUERY, 1, &tmp,
347		- sizeof(tmp));
	361	+ sizeof(tmp), sizeof(tmp));
348	362	if (ret)
349	363	return -EINVAL;
350	364
...	...	@@ -417,6 +431,7 @@
417	431	case HPWMI_BEZEL_BUTTON:
418	432	ret = hp_wmi_perform_query(HPWMI_HOTKEY_QUERY, 0,
419	433	&key_code,
	434	+ sizeof(key_code),
420	435	sizeof(key_code));
421	436	if (ret)
422	437	break;
...	...	@@ -523,7 +538,7 @@
523	538	int wireless = 0;
524	539
525	540	err = hp_wmi_perform_query(HPWMI_WIRELESS_QUERY, 0, &wireless,
526		- sizeof(wireless));
	541	+ sizeof(wireless), sizeof(wireless));
527	542	if (err)
528	543	return err;
529	544