efivars: Improve variable validation

Ben Hutchings pointed out that the validation in efivars was inadequate - most obviously, an entry with size 0 would server as a DoS against the kernel. Improve this based on his suggestions. Signed-off-by: Matthew Garrett <mjg@redhat.com> Cc: stable@vger.kernel.org Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org>

efivars: Improve variable validation
Ben Hutchings pointed out that the validation in efivars was inadequate - most obviously, an entry with size 0 would server as a DoS against the kernel. Improve this based on his suggestions. Signed-off-by: Matthew Garrett <mjg@redhat.com> Cc: stable@vger.kernel.org Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org>
Matthew Garrett · Linus Torvalds
1 parent fec6c20b57
Showing 1 changed file with 30 additions and 16 deletions Side-by-side Diff
drivers/firmware/efivars.c
@@ -192,19 +192,22 @@
 }
  
 static bool
-validate_device_path(struct efi_variable *var, int match, u8 *buffer, int len)
+validate_device_path(struct efi_variable *var, int match, u8 *buffer,
+		     unsigned long len)
 {
 	struct efi_generic_dev_path *node;
 	int offset = 0;
  
 	node = (struct efi_generic_dev_path *)buffer;
  
-	while (offset < len) {
+	if (len < sizeof(*node))
+		return false;
+
+	while (offset <= len - sizeof(*node) &&
+	       node->length >= sizeof(*node) &&
+		node->length <= len - offset) {
 		offset += node->length;
  
-		if (offset > len)
-			return false;
-
 		if ((node->type == EFI_DEV_END_PATH ||
 		     node->type == EFI_DEV_END_PATH2) &&
 		    node->sub_type == EFI_DEV_END_ENTIRE)
@@ -222,7 +225,8 @@
 }
  
 static bool
-validate_boot_order(struct efi_variable *var, int match, u8 *buffer, int len)
+validate_boot_order(struct efi_variable *var, int match, u8 *buffer,
+		    unsigned long len)
 {
 	/* An array of 16-bit integers */
 	if ((len % 2) != 0)
  
  
  
  
  
  
@@ -232,28 +236,36 @@
 }
  
 static bool
-validate_load_option(struct efi_variable *var, int match, u8 *buffer, int len)
+validate_load_option(struct efi_variable *var, int match, u8 *buffer,
+		     unsigned long len)
 {
 	u16 filepathlength;
-	int i, desclength = 0;
+	int i, desclength = 0, namelen;
  
+	namelen = utf16_strnlen(var->VariableName, sizeof(var->VariableName));
+
 	/* Either "Boot" or "Driver" followed by four digits of hex */
 	for (i = match; i < match+4; i++) {
-		if (hex_to_bin(var->VariableName[i] & 0xff) < 0)
+		if (var->VariableName[i] > 127 ||
+		    hex_to_bin(var->VariableName[i] & 0xff) < 0)
 			return true;
 	}
  
-	/* A valid entry must be at least 6 bytes */
-	if (len < 6)
+	/* Reject it if there's 4 digits of hex and then further content */
+	if (namelen > match + 4)
 		return false;
  
+	/* A valid entry must be at least 8 bytes */
+	if (len < 8)
+		return false;
+
 	filepathlength = buffer[4] | buffer[5] << 8;
  
 	/*
 	 * There's no stored length for the description, so it has to be
 	 * found by hand
 	 */
-	desclength = utf16_strsize((efi_char16_t *)(buffer + 6), len) + 2;
+	desclength = utf16_strsize((efi_char16_t *)(buffer + 6), len - 6) + 2;
  
 	/* Each boot entry must have a descriptor */
 	if (!desclength)
@@ -275,7 +287,8 @@
 }
  
 static bool
-validate_uint16(struct efi_variable *var, int match, u8 *buffer, int len)
+validate_uint16(struct efi_variable *var, int match, u8 *buffer,
+		unsigned long len)
 {
 	/* A single 16-bit integer */
 	if (len != 2)
@@ -285,7 +298,8 @@
 }
  
 static bool
-validate_ascii_string(struct efi_variable *var, int match, u8 *buffer, int len)
+validate_ascii_string(struct efi_variable *var, int match, u8 *buffer,
+		      unsigned long len)
 {
 	int i;
  
@@ -303,7 +317,7 @@
 struct variable_validate {
 	char *name;
 	bool (*validate)(struct efi_variable *var, int match, u8 *data,
-			 int len);
+			 unsigned long len);
 };
  
 static const struct variable_validate variable_validate[] = {
@@ -325,7 +339,7 @@
 };
  
 static bool
-validate_var(struct efi_variable *var, u8 *data, int len)
+validate_var(struct efi_variable *var, u8 *data, unsigned long len)
 {
 	int i;
 	u16 *unicode_name = var->VariableName;
...	...	@@ -192,19 +192,22 @@
192	192	}
193	193
194	194	static bool
195		-validate_device_path(struct efi_variable var, int match, u8 buffer, int len)
	195	+validate_device_path(struct efi_variable var, int match, u8 buffer,
	196	+ unsigned long len)
196	197	{
197	198	struct efi_generic_dev_path *node;
198	199	int offset = 0;
199	200
200	201	node = (struct efi_generic_dev_path *)buffer;
201	202
202		- while (offset < len) {
	203	+ if (len < sizeof(*node))
	204	+ return false;
	205	+
	206	+ while (offset <= len - sizeof(*node) &&
	207	+ node->length >= sizeof(*node) &&
	208	+ node->length <= len - offset) {
203	209	offset += node->length;
204	210
205		- if (offset > len)
206		- return false;
207		-
208	211	if ((node->type == EFI_DEV_END_PATH \|\|
209	212	node->type == EFI_DEV_END_PATH2) &&
210	213	node->sub_type == EFI_DEV_END_ENTIRE)
...	...	@@ -222,7 +225,8 @@
222	225	}
223	226
224	227	static bool
225		-validate_boot_order(struct efi_variable var, int match, u8 buffer, int len)
	228	+validate_boot_order(struct efi_variable var, int match, u8 buffer,
	229	+ unsigned long len)
226	230	{
227	231	/* An array of 16-bit integers */
228	232	if ((len % 2) != 0)
229	233
230	234
231	235
232	236
233	237
234	238
...	...	@@ -232,28 +236,36 @@
232	236	}
233	237
234	238	static bool
235		-validate_load_option(struct efi_variable var, int match, u8 buffer, int len)
	239	+validate_load_option(struct efi_variable var, int match, u8 buffer,
	240	+ unsigned long len)
236	241	{
237	242	u16 filepathlength;
238		- int i, desclength = 0;
	243	+ int i, desclength = 0, namelen;
239	244
	245	+ namelen = utf16_strnlen(var->VariableName, sizeof(var->VariableName));
	246	+
240	247	/* Either "Boot" or "Driver" followed by four digits of hex */
241	248	for (i = match; i < match+4; i++) {
242		- if (hex_to_bin(var->VariableName[i] & 0xff) < 0)
	249	+ if (var->VariableName[i] > 127 \|\|
	250	+ hex_to_bin(var->VariableName[i] & 0xff) < 0)
243	251	return true;
244	252	}
245	253
246		- /* A valid entry must be at least 6 bytes */
247		- if (len < 6)
	254	+ /* Reject it if there's 4 digits of hex and then further content */
	255	+ if (namelen > match + 4)
248	256	return false;
249	257
	258	+ /* A valid entry must be at least 8 bytes */
	259	+ if (len < 8)
	260	+ return false;
	261	+
250	262	filepathlength = buffer[4] \| buffer[5] << 8;
251	263
252	264	/*
253	265	* There's no stored length for the description, so it has to be
254	266	* found by hand
255	267	*/
256		- desclength = utf16_strsize((efi_char16_t *)(buffer + 6), len) + 2;
	268	+ desclength = utf16_strsize((efi_char16_t *)(buffer + 6), len - 6) + 2;
257	269
258	270	/* Each boot entry must have a descriptor */
259	271	if (!desclength)
...	...	@@ -275,7 +287,8 @@
275	287	}
276	288
277	289	static bool
278		-validate_uint16(struct efi_variable var, int match, u8 buffer, int len)
	290	+validate_uint16(struct efi_variable var, int match, u8 buffer,
	291	+ unsigned long len)
279	292	{
280	293	/* A single 16-bit integer */
281	294	if (len != 2)
...	...	@@ -285,7 +298,8 @@
285	298	}
286	299
287	300	static bool
288		-validate_ascii_string(struct efi_variable var, int match, u8 buffer, int len)
	301	+validate_ascii_string(struct efi_variable var, int match, u8 buffer,
	302	+ unsigned long len)
289	303	{
290	304	int i;
291	305
...	...	@@ -303,7 +317,7 @@
303	317	struct variable_validate {
304	318	char *name;
305	319	bool (validate)(struct efi_variable var, int match, u8 *data,
306		- int len);
	320	+ unsigned long len);
307	321	};
308	322
309	323	static const struct variable_validate variable_validate[] = {
...	...	@@ -325,7 +339,7 @@
325	339	};
326	340
327	341	static bool
328		-validate_var(struct efi_variable var, u8 data, int len)
	342	+validate_var(struct efi_variable var, u8 data, unsigned long len)
329	343	{
330	344	int i;
331	345	u16 *unicode_name = var->VariableName;