[Grub-dev] Yet another invalid workunit entry

[Rainer Blome]

Rainer Blome wrote:
> Looking at the Grub C Client code...
...again, I noticed something funny.

> o The function FetchPage...
>    - contains this loop:
>          do {
>              i = send(sockfd, Q, QueryLen, 0);

If i is negative (for example -1, as is anticipated), Q and QueryLen are 
still moved - in the *reverse* direction :-]:

>              QueryLen -= i;
>              Q += i;
>              if (i == -1) {
>                  perror("send");
>                  Error = SEND_ERROR;
>                  closesocket(sockfd);
>                  // A new socket descriptor... unconnected.
>                  // The send above will never succeed after this branch.
>                  sockfd = NewSocket();
>                  // This continues the while loop - and not
>                  // the enclosing for loop (not shown here),
>                  // which would actually reconnect.
>                  // Consider restructuring the function.
>                  continue;

That "continue" is actually a NOP, since it just falls through to the 
"while".  Assuming that all further "send" calls fail (return -1), and 
that send doesn't try to access *Q unless the socket is connected, the 
loop will iterate roughly 4.5 billion times, until QueryLen overflows 
(wraps around reaching zero), all the while acquiring new sockets and 
releasing them.  Although the loop does finish, this may look like an 
infinite loop to the user.  (Test program below)

>              }
>          } while (QueryLen);
>          // After this loop, the function continues to try
>          // and read data from the socket -
>          // even if the socket is unconnected.

Rainer


/* @file badloop.c
cc -o badloop badloop.c && ./badloop
*/
#include <stdio.h>

static double calls= 0;
static int len_initial= 0;

int send_dummy(char *const q, size_t const len) {
   if (0 == len_initial) { len_initial= len; }
   static int q_reported= 0;
   static int len_reported= 0;
   ++calls;
   if (q < (char*)0) {
     if (!q_reported) {
       q_reported= 1;
       printf("q below zero (calls=%g)\n", calls);
     }
   }
   if (len > len_initial) {
     if (!len_reported) {
       len_reported= 1;
       printf("len exceeded len_initial (calls=%g)\n", calls);
     }
   }
   if (3<=calls) { return -1; }
   printf("q=%p\n", q);
   return 1;
}
void badloop(size_t QueryLen) {
   int i = 0;
   char* Q = 0;
   do {
             i = send_dummy(Q, QueryLen);
             QueryLen -= i;
             Q += i;
             if (i == -1) {
	      /*perror("send");
                 Error = SEND_ERROR;
                 closesocket(sockfd);
                 sockfd = NewSocket();
	      */
                 continue;
             }
         } while (QueryLen);
   printf("badloop: END (calls=%g)\n", calls);
}

int main(void) {
   /* For testing, start close to the max, takes some seconds: */
   badloop(4000000000.0);
   /* realistically, you start low, takes a few minutes: */
   badloop(40);
   return 0;
}