On Mon, Feb 01, 2010 at 10:18:27PM +0100, Jim Meyering wrote:
In src/qemu/qemu_driver.c, coverity reports this:
Event negative_return_fn: Called negative-returning function "lseek(logfile, 0L,
2)"
Event var_assign: NEGATIVE return value of "lseek" assigned to signed
variable "pos"
At conditional (1): "(pos = lseek(logfile, 0L, 2)) < 0" taking true path
2877 if ((pos = lseek(logfile, 0, SEEK_END)) < 0)
2878 VIR_WARN(_("Unable to seek to end of logfile: %s"),
2879 virStrerror(errno, ebuf, sizeof ebuf));
since later in that same function, a negative "pos" may
be used like this:
Event negative_returns: Tracked variable "pos" was passed to a negative sink.
[details]
2930 if (qemudWaitForMonitor(conn, driver, vm, pos) < 0)
2931 goto abort;
2932
which is a legitimate problem, since
qemudWaitForMonitor calls qemudLogReadFD, which calls lseek
with that same "pos" value:
Event neg_sink_parm_call: Parameter "pos" passed to negative sink
"lseek"
560 if (lseek(fd, pos, SEEK_SET) < 0) {
561 virReportSystemError(conn, errno,
562 _("Unable to seek to %lld in %s"),
563 (long long) pos, logfile);
564 close(fd);
565 }
One approach is to detect the negative offset in that final bit
of code and skip the lseek:
>From 0ef617935462c314ed0b44bcaa3dd5bf58ccbc1b Mon Sep 17 00:00:00 2001
From: Jim Meyering <meyering(a)redhat.com>
Date: Mon, 1 Feb 2010 22:17:44 +0100
Subject: [PATCH] avoid a probable EINVAL from lseek
* src/qemu/qemu_driver.c (qemudLogReadFD): Don't pass a negative
offset (from a preceding failed attempt to seek to EOF) to this use
of lseek.
---
src/qemu/qemu_driver.c | 4 ++--
1 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 22593bf..676a27b 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -558,8 +558,8 @@ qemudLogReadFD(virConnectPtr conn, const char* logDir, const char*
name, off_t p
close(fd);
return -1;
}
- if (lseek(fd, pos, SEEK_SET) < 0) {
- virReportSystemError(conn, errno,
+ if (pos < 0 || lseek(fd, pos, SEEK_SET) < 0) {
+ virReportSystemError(conn, pos < 0 ? 0 : errno,
_("Unable to seek to %lld in %s"),
(long long) pos, logfile);
close(fd);
I was wondering if it wasn't simpler to abort earlier on when
pos < 0 was returned from lseek, but after rereading the code I
agree with your patch,
ACK,
Daniel
--
Daniel Veillard | libxml Gnome XML XSLT toolkit
http://xmlsoft.org/
daniel(a)veillard.com | Rpmfind RPM search engine
http://rpmfind.net/
http://veillard.com/ | virtualization library
http://libvirt.org/