Andrew Beekhof wrote:
Coverity started complaining about a bounds overrun... can someone check
this patch is ok?
Index: heartbeat/heartbeat.c
===================================================================
RCS file: /home/cvs/linux-ha/linux-ha/heartbeat/heartbeat.c,v
retrieving revision 1.506
diff -u -r1.506 heartbeat.c
--- heartbeat/heartbeat.c 26 Apr 2006 03:42:07 -0000 1.506
+++ heartbeat/heartbeat.c 11 May 2006 09:22:43 -0000
@@ -5975,8 +5975,8 @@
longclock_t last_rexmit;
size_t len;
- if (msgslot < 0) {
- msgslot = MAXMSGHIST;
+ if (msgslot < 0 || msgslot >= MAXMSGHIST) {
+ msgslot = MAXMSGHIST-1;
}
if (hist->msgq[msgslot] == NULL) {
continue;
This looks like a real bug. One that could in theory cause a crash.
I think this patch might cause it to loop infinitely if
firstslot == MAXMSGHIST-1.
Let's see...
We decrement and decrement, and decrement, then the index goes negative,
and we then reset it to MAXMSGHIST-1.
We go through one of the continue statements, and decrement msgslot
again. It is now MAXMSGHIST-2. We can now loop forever...
So, I don't think that this is quite right yet...
This can happen if the sequence number we're being asked to rexmit isn't
found at all (it's too old, or bogus or whatever).
Here's a similar patch, but one that moves one of the tests outside the
loop, and which hopefully doesn't loop...
Index: heartbeat.c
===================================================================
RCS file: /home/cvs/linux-ha/linux-ha/heartbeat/heartbeat.c,v
retrieving revision 1.506
diff -u -r1.506 heartbeat.c
--- heartbeat.c 26 Apr 2006 03:42:07 -0000 1.506
+++ heartbeat.c 11 May 2006 12:49:22 -0000
@@ -5911,10 +5911,16 @@
struct node_info* fromnode = NULL;
if (fromnodename == NULL){
- cl_log(LOG_ERR, "process_rexmit: "
- "from node not found in the message");
+ cl_log(LOG_ERR, "process_rexmit"
+ ": from node not found in the message");
return;
}
+ if (firstslot >= MAXMSGHIST) {
+ cl_log(LOG_ERR, "process_rexmit"
+ ": firstslot out of range [%d]"
+ , );
+ hist->lastmsg = firstslot = MAXMSGHIST-1;
+ }
fromnode = lookup_tables(fromnodename, NULL);
if (fromnode == NULL){
@@ -5976,7 +5982,11 @@
size_t len;
if (msgslot < 0) {
- msgslot = MAXMSGHIST;
+ if (firstslot == MAXMSGHIST-1) {
+ /* We've wrapped around */
+ break;
+ }
+ msgslot = MAXMSGHIST-1;
}
if (hist->msgq[msgslot] == NULL) {
continue;
--
Alan Robertson <[EMAIL PROTECTED]>
"Openness is the foundation and preservative of friendship... Let me
claim from you at all times your undisguised opinions." - William
Wilberforce
_______________________________________________________
Linux-HA-Dev: [email protected]
http://lists.linux-ha.org/mailman/listinfo/linux-ha-dev
Home Page: http://linux-ha.org/
