In reviewing the StorageAPI patches, I saw that some bogus input could cause a segfault. There's similar code in libvirt.c, so this fixes both. Note that virsh.c does this, too, but it already has the required guard. The src/libvirt.c patch applies to the trunk, but isn't a big deal, since it's in the sample AuthCallback function. The storage_backend.c part applies to the work-in-progress mercurial queue, but does matter, because we have no guarantee what the exec'd command will output. Don't access line[-1] for a zero-length "line" from fgets. A NUL byte at beginning of input, or just after a newline would provoke an invalid buf[-1] access (possible segfault). * src/libvirt.c (virConnectAuthCallbackDefault): * src/storage_backend.c (virStorageBackendRunProgRegex): Signed-off-by: Jim Meyering <meyering@redhat.com> --- src/libvirt.c | 6 ++++-- src/storage_backend.c | 2 +- 2 files changed, 5 insertions(+), 3 deletions(-) diff --git a/src/libvirt.c b/src/libvirt.c index 2972382..331c937 100644 --- a/src/libvirt.c +++ b/src/libvirt.c @@ -74,6 +74,7 @@ static int virConnectAuthCallbackDefault(virConnectCredentialPtr cred, for (i = 0 ; i < ncred ; i++) { char buf[1024]; char *bufptr = buf; + size_t len; if (printf("%s:", cred[i].prompt) < 0) return -1; @@ -92,8 +93,9 @@ static int virConnectAuthCallbackDefault(virConnectCredentialPtr cred, } return -1; } - if (buf[strlen(buf)-1] == '\n') - buf[strlen(buf)-1] = '\0'; + len = strlen(buf); + if (len != 0 && buf[len-1] == '\n') + buf[len-1] = '\0'; break; case VIR_CRED_PASSPHRASE: diff --git a/src/storage_backend.c b/src/storage_backend.c index 32a8d65..58d8642 100644 --- a/src/storage_backend.c +++ b/src/storage_backend.c @@ -338,7 +338,7 @@ int virStorageBackendRunProgRegex(virConnectPtr conn, while (fgets(line, sizeof(line), list) != NULL) { /* Strip trailing newline */ int len = strlen(line); - if (line[len-1] == '\n') + if (len && line[len-1] == '\n') line[len-1] = '\0'; for (i = 0 ; i <= maxReg && i < nregex ; i++) { -- 1.5.4.rc4.1.g1895