"Daniel P. Berrange" <berrange@xxxxxxxxxx> wrote:
> On Tue, Jan 27, 2009 at 11:28:32AM +0100, Jim Meyering wrote:
>> "Daniel P. Berrange" <berrange@xxxxxxxxxx> wrote:
>> ...
>> > Looking at the whole method again, I think it needs to be re-written to
>> > something closer to this:
>>
>> Ok, I've adapted that.
>> +void virReportSystemErrorFull(virConnectPtr conn,
>> + int domcode,
>> + int theerrno,
>> + const char *filename ATTRIBUTE_UNUSED,
>> + const char *funcname ATTRIBUTE_UNUSED,
>> + size_t linenr ATTRIBUTE_UNUSED,
>> + const char *fmt, ...)
>> +{
>> + char strerror_buf[1024];
>> + char msgDetailBuf[1024];
>> +
>> + const char *errnoDetail = virStrerror(theerrno, strerror_buf,
>> + sizeof(strerror_buf));
>> + const char *msg = virErrorMsg(VIR_ERR_SYSTEM_ERROR, fmt);
>> + const char *msgDetail = NULL;
>>
>> if (fmt) {
>> + va_list args;
>> + int n;
>> +
>> va_start(args, fmt);
>> - vsnprintf(errorMessage, sizeof(errorMessage)-1, fmt, args);
>> + n = vsnprintf(msgDetailBuf, sizeof(msgDetailBuf), fmt, args);
>> va_end(args);
>> - } else {
>> - errorMessage[0] = '\0';
>> +
>> + size_t len = strlen (msgDetailBuf);
>> + if (0 <= n && n + 2 + len < sizeof (msgDetailBuf)) {
>> + char *p = msgDetailBuf + n;
>> + stpcpy (stpcpy (p, ": "), errnoDetail);
>> + msgDetail = msgDetailBuf;
>> + }
>> }
>>
>> - if (virAsprintf(&combined, "%s: %s", errorMessage, theerrnostr) < 0)
>> - combined = theerrnostr; /* OOM, so lets just pass the strerror info as best effort */
>> + if (!msgDetailBuf)
>> + msgDetail = errnoDetail;
>
> This should be if (!msgDetail) - indeed just noticed the compiler
> warns on this
>
> cc1: warnings being treated as errors
> virterror.c: In function 'virReportSystemErrorFull':
> virterror.c:1062: error: the address of 'msgDetailBuf' will always evaluate as 'true'
Good catch.
I've just committed this:
>From 2bb6830c061a580328abf4647a26b9ecc7f7eaa1 Mon Sep 17 00:00:00 2001
From: Jim Meyering <meyering@xxxxxxxxxx>
Date: Tue, 27 Jan 2009 13:25:16 +0100
Subject: [PATCH] virterror.c: don't read beyond end of buffer upon OOM
* src/virterror.c (virReportSystemErrorFull): Fix typo in
my previous change. Patch by Daniel P. Berrange.
---
src/virterror.c | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/src/virterror.c b/src/virterror.c
index a577002..160fea3 100644
--- a/src/virterror.c
+++ b/src/virterror.c
@@ -1059,7 +1059,7 @@ void virReportSystemErrorFull(virConnectPtr conn,
}
}
- if (!msgDetailBuf)
+ if (!msgDetail)
msgDetail = errnoDetail;
virRaiseError(conn, NULL, NULL, domcode, VIR_ERR_SYSTEM_ERROR, VIR_ERR_ERROR,
--
1.6.1.401.gf39d5
--
Libvir-list mailing list
Libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list