Coverity has detected a long list of small defects in the snmp_core.cc code.
Lots of alignment, sizeof(), and memory uninitialized.
They all stem from a few uses of "sizeof(name)" where name is a pointer
to an OID object rather than the object itself. This code is present as
far back as squid-2.6 and probably a lot further.
I believe it has not been causing obvious problems due to the
sizeof(oid*) [4 or 8 octets] on a pointer being larger than the
sizeof(oid) [1 or 2 octets] on the object stored into the allocate memory.
Is anyone able to test the attached patch please?
Amos
=== modified file 'src/snmp_core.cc'
--- src/snmp_core.cc 2013-01-27 17:35:07 +0000
+++ src/snmp_core.cc 2013-01-27 23:48:59 +0000
@@ -705,8 +705,8 @@
{
oid *instance = NULL;
if (*len <= current->len) {
- instance = (oid *)xmalloc(sizeof(name) * (*len + 1));
- memcpy(instance, name, (sizeof(name) * *len));
+ instance = (oid *)xmalloc(sizeof(*name) * (*len + 1));
+ memcpy(instance, name, sizeof(*name) * (*len));
instance[*len] = 0;
*len += 1;
}
@@ -722,8 +722,8 @@
int index[TIME_INDEX_LEN] = {TIME_INDEX};
if (*len <= current->len) {
- instance = (oid *)xmalloc(sizeof(name) * (*len + 1));
- memcpy(instance, name, (sizeof(name) * *len));
+ instance = (oid *)xmalloc(sizeof(*name) * (*len + 1));
+ memcpy(instance, name, sizeof(*name) * (*len));
instance[*len] = *index;
*len += 1;
} else {
@@ -733,8 +733,8 @@
++loop;
if (loop < (TIME_INDEX_LEN - 1)) {
- instance = (oid *)xmalloc(sizeof(name) * (*len));
- memcpy(instance, name, (sizeof(name) * *len));
+ instance = (oid *)xmalloc(sizeof(*name) * (*len));
+ memcpy(instance, name, sizeof(*name) * (*len));
instance[*len - 1] = index[++loop];
}
}
@@ -761,8 +761,8 @@
instance = client_Inst(current->name, len, current, Fn);
} else if (*len <= current->len) {
debugs(49, 6, "snmp peer_Inst: *len <= current->len ???");
- instance = (oid *)xmalloc(sizeof(name) * ( *len + 1));
- memcpy(instance, name, (sizeof(name) * *len));
+ instance = (oid *)xmalloc(sizeof(*name) * ( *len + 1));
+ memcpy(instance, name, sizeof(*name) * (*len));
instance[*len] = 1 ;
*len += 1;
} else {
@@ -773,8 +773,8 @@
if (peers) {
debugs(49, 6, "snmp peer_Inst: Encode peer #" << i);
- instance = (oid *)xmalloc(sizeof(name) * (current->len + 1 ));
- memcpy(instance, name, (sizeof(name) * current->len ));
+ instance = (oid *)xmalloc(sizeof(*name) * (current->len + 1 ));
+ memcpy(instance, name, (sizeof(*name) * current->len ));
instance[current->len] = no + 1 ; // i.e. the next index on
cache_peeer table.
} else {
debugs(49, 6, "snmp peer_Inst: We have " << i << " peers. Can't
find #" << no);
@@ -808,8 +808,8 @@
debugs(49, 6, HERE << "len" << *len << ", current-len" << current->len
<< ", addr=" << laddr << ", size=" << size);
- instance = (oid *)xmalloc(sizeof(name) * (*len + size ));
- memcpy(instance, name, (sizeof(name) * (*len)));
+ instance = (oid *)xmalloc(sizeof(*name) * (*len + size ));
+ memcpy(instance, name, (sizeof(*name) * (*len)));
if ( !laddr.IsAnyAddr() ) {
addr2oid(laddr, &instance[ *len]); // the addr
@@ -832,8 +832,8 @@
debugs(49, 6, HERE << "len" << *len << ", current-len" <<
current->len << ", addr=" << laddr << ", newshift=" << newshift);
- instance = (oid *)xmalloc(sizeof(name) * (current->len +
newshift));
- memcpy(instance, name, (sizeof(name) * (current->len)));
+ instance = (oid *)xmalloc(sizeof(*name) * (current->len +
newshift));
+ memcpy(instance, name, (sizeof(*name) * (current->len)));
addr2oid(laddr, &instance[current->len]); // the addr.
*len = current->len + newshift ;
}
