Bug#764386: libefivar0: Segmentation fault in vars_get_variable() (vars.c:165)
Jan, I passed your patch upstream. Peter Jones is looking at it. It'd probably help him to know more details about what system you're on, though, like what make/model this is and probably an strace if you can. --Jared -- Jared Domínguez Server OS Engineering Dell | Enterprise Solutions Group -- To UNSUBSCRIBE, email to debian-bugs-rc-requ...@lists.debian.org with a subject of unsubscribe. Trouble? Contact listmas...@lists.debian.org
Bug#764386: libefivar0: Segmentation fault in vars_get_variable() (vars.c:165)
Actually, never mind. Peter broke out your patch and applied it upstream. I've just uploaded efivar 0.15-2. Please verify that it works correctly on your system. On Mon, Nov 03, 2014 at 09:58:20AM -0600, Dominguez, Jared wrote: Jan, I passed your patch upstream. Peter Jones is looking at it. It'd probably help him to know more details about what system you're on, though, like what make/model this is and probably an strace if you can. --Jared -- Jared Domínguez Server OS Engineering Dell | Enterprise Solutions Group -- Jared Domínguez Server OS Engineering Dell | Enterprise Solutions Group -- To UNSUBSCRIBE, email to debian-bugs-rc-requ...@lists.debian.org with a subject of unsubscribe. Trouble? Contact listmas...@lists.debian.org
Bug#764386: libefivar0: Segmentation fault in vars_get_variable() (vars.c:165)
On Wed, Oct 29, 2014 at 12:11:18PM -0500, D. Jared Dominguez wrote:
Can you confirm whether this is still an issue for you? New versions
of libefivar0 are in testing and unstable. Please test against them.
The segmentation fault still occurs with libefivar0 version 0.15-1 and
efibootmgr version 0.11.0-1.
I had some time for a detailed examination of read_file() this weekend and
it looked worse than I had hoped. The attached patch tries to fix the
following problems:
- It returns 0 on read errors and memory allocation failures, although
the code calling it probably expects a return value of -1 and a failure
reason in errno.
- The type of filesize doesn't match the type of size and bufsize.
- Partial read handling seems to be seriously broken. If I'm not mistaken,
buffer overflows are possible.
- There's no integer overflow check before increasing the buffer size.
The only part of that patch that I could test was the handling of read
errors. With the patch, efibootmgr doesn't crash anymore but reports
efibootmgr: efibootmgr: Input/output error
--
Jan
diff -ru efivar-0.15.orig/src/util.h efivar-0.15/src/util.h
--- efivar-0.15.orig/src/util.h 2014-10-15 15:48:49.0 +0200
+++ efivar-0.15/src/util.h 2014-11-03 00:45:18.271719150 +0100
@@ -31,46 +31,58 @@
{
uint8_t *p;
size_t size = 4096;
- int s = 0, filesize = 0;
+ int s = 0;
uint8_t *newbuf;
if (!(newbuf = calloc(size, sizeof (uint8_t
return -1;
*buf = newbuf;
+ *bufsize = 0;
do {
- p = *buf + filesize;
- s = read(fd, p, 4096 - s);
+ p = *buf + *bufsize;
+ /* size - *bufsize shouldn't exceed SSIZE_MAX because we're
+ only allocating 4096 bytes at a time. */
+ s = read(fd, p, size - *bufsize);
if (s 0 errno == EAGAIN) {
continue;
} else if (s 0) {
+ int saved_errno = errno;
free(*buf);
*buf = NULL;
*bufsize = 0;
- break;
+ errno = saved_errno;
+ return -1;
}
- filesize += s;
+ *bufsize += s;
/* only exit for empty reads */
if (s == 0) {
break;
- } else if (s == 4096) {
+ }
+ if (*bufsize = size) {
+ if (size (size_t)-1 - 4096)
+ {
+free(*buf);
+*buf = NULL;
+*bufsize = 0;
+errno = ENOMEM;
+return -1;
+ }
newbuf = realloc(*buf, size + 4096);
if (newbuf == NULL) {
+int saved_errno = errno;
free(*buf);
*buf = NULL;
*bufsize = 0;
+errno = saved_errno;
return -1;
}
*buf = newbuf;
memset(*buf + size, '\0', 4096);
- size += s;
- s = 0;
- } else {
- size += s;
+ size += 4096;
}
} while (1);
- *bufsize = filesize;
return 0;
}
Bug#764386: libefivar0: Segmentation fault in vars_get_variable() (vars.c:165)
On Mon, Oct 06, 2014 at 06:14:59PM -0500, Jan Echternach wrote: Package: libefivar0 Version: 0.12-1 Severity: critical Justification: breaks the whole system Upgrading libefivar0 from version 0.10-5 to 0.12-1 causes a segmentation fault when running efibootmgr without arguments (I tried it with both efibootmgr 0.7.0-2 and 0.9.0-1). I'm not quite sure if severity critical is justified, but I think a broken efibootmgr is at least potentially able to break the whole system. gdb pointed to libefivar.so.0 which has no debugging symbols, so I built my own and that one crashes in vars.c line 165 with var == NULL. The last two lines in an strace log before the crash are open(/sys/firmware/efi/vars/Boot0005-8be4[...]/raw_var, O_RDONLY) = 3 read(3, [...], 4096) = -1 EIO (Input/output error) (Sorry, no copypaste, just readtype; the system in question has only very limited network connectivity at the moment and I'm sending this report from a different system.) var is apparently returned from a call to read_file() a few lines above. The source code history shows that read_fd() has recently been replaced by read_file(), but they behave differently after read errors. In particular, read_file() resets the buffer to NULL whereas read_fd() didn't. Jan, Can you confirm whether this is still an issue for you? New versions of libefivar0 are in testing and unstable. Please test against them. --Jared -- Jared Domínguez Server OS Engineering Dell | Enterprise Solutions Group -- To UNSUBSCRIBE, email to debian-bugs-rc-requ...@lists.debian.org with a subject of unsubscribe. Trouble? Contact listmas...@lists.debian.org
Bug#764386: libefivar0: Segmentation fault in vars_get_variable() (vars.c:165)
Package: libefivar0 Version: 0.12-1 Severity: critical Justification: breaks the whole system Upgrading libefivar0 from version 0.10-5 to 0.12-1 causes a segmentation fault when running efibootmgr without arguments (I tried it with both efibootmgr 0.7.0-2 and 0.9.0-1). I'm not quite sure if severity critical is justified, but I think a broken efibootmgr is at least potentially able to break the whole system. gdb pointed to libefivar.so.0 which has no debugging symbols, so I built my own and that one crashes in vars.c line 165 with var == NULL. The last two lines in an strace log before the crash are open(/sys/firmware/efi/vars/Boot0005-8be4[...]/raw_var, O_RDONLY) = 3 read(3, [...], 4096) = -1 EIO (Input/output error) (Sorry, no copypaste, just readtype; the system in question has only very limited network connectivity at the moment and I'm sending this report from a different system.) var is apparently returned from a call to read_file() a few lines above. The source code history shows that read_fd() has recently been replaced by read_file(), but they behave differently after read errors. In particular, read_file() resets the buffer to NULL whereas read_fd() didn't. -- To UNSUBSCRIBE, email to debian-bugs-rc-requ...@lists.debian.org with a subject of unsubscribe. Trouble? Contact listmas...@lists.debian.org
