When invoked with a small buffer size, the ioctl SIOCGIFCONF may return more information than can fit in the provided buffer. This problem has been partially fixed in FreeBSD 4.0 (if.c rev 1.85). It does not write outside the boundary of the provided buffer but the returned buffer size is incorrect. This in turn can lead to problems in the calling code. This only happens if the system has interfaces for which sa->sa_len is greater than sizeof(*sa), which is the case with most non-IPv4 addresses... This problem has been noted in PR kern/12996 and PR kern/14457 without a good fix yet. Fix: Fix to apply against the HEAD of the CVS Fix to apply against the RELENG_3 branch of the CVS This PR should close PR kern/12996 and kern/14457 Patrick Bihan-Faou, Herve Masson MindStep Corporation--UGW9GnnkTiuE7ucr43uB4RQm8itWniIDf0v4iysRZgdQOWhd Content-Type: text/plain; name="file.diff" Content-Transfer-Encoding: 7bit Content-Disposition: attachment; filename="file.diff" Index: if.c =================================================================== RCS file: /cvs/freebsd/src/sys/net/if.c,v retrieving revision 1.85 diff -u -r1.85 if.c --- if.c 2000/02/28 19:30:25 1.85 +++ if.c 2000/03/10 23:29:50 @@ -1086,11 +1086,9 @@ sizeof (ifr)); ifrp++; } else { - if (space < sa->sa_len - sizeof(*sa)) + if (space < sizeof(ifr) + sa->sa_len - sizeof(*sa)) break; space -= sa->sa_len - sizeof(*sa); - if (space < sizeof (ifr)) - break; error = copyout((caddr_t)&ifr, (caddr_t)ifrp, sizeof (ifr.ifr_name)); if (error == 0) How-To-Repeat: The following piece of code will show the problem. #include <errno.h> #include <sys/types.h> #include <sys/param.h> #include <sys/time.h> #include <sys/ioctl.h> #include <sys/socket.h> #include <net/if.h> #include <netinet/in.h> #include <sys/sockio.h> #define VERBOSE_CHECK check55(char *start,char *end) { int startoff=-1,endoff=0; int off=0,c=0; #ifdef VERBOSE_CHECK printf("%03d\t",off); #endif for(;start<end;start++,off++) { if(*start != 0x55) { if(startoff<0) { startoff=off; } endoff=off; } #ifdef VERBOSE_CHECK if(++c>=33) { printf("\n%03d\t",off); c=1; } printf("%02x ",*(unsigned char*)start); #endif } printf("\n"); if(startoff>=0) { printf(" ** buffer changed from %d to %d => %d bytes modified **\n",startoff,endoff, endoff - startoff + 1); } } main() { struct ifconf ifc; char *x; struct ifreq *ifr; struct sockaddr_in *sin; int len,ret; int s; char buf[1024]; #define END_TEST 300 if ((s = socket(AF_INET,SOCK_STREAM,0)) == -1) return -1; for (len=1;len<=END_TEST;len++) { ifc.ifc_buf = buf; ifc.ifc_len = len; memset(buf,0x55,sizeof(buf)); printf("\n"); printf("[Try with len=%d]\n",len); if ((ret=ioctl(s,SIOCGIFCONF,&ifc)) < 0) { printf(" => ioctl failed (returned %d, errno=%d)\n",ret,errno); } printf(" => ioctl succeeded, pretends it wrote %d bytes\n",ifc.ifc_len); printf("\n"); check55(buf,buf+sizeof(buf)); printf("\n"); } return 0; }
The following test code will provide better report on the version of the bug present in the system. #include <errno.h> #include <sys/types.h> #include <sys/param.h> #include <sys/time.h> #include <sys/ioctl.h> #include <sys/socket.h> #include <net/if.h> #include <netinet/in.h> #include <sys/sockio.h> #define VERBOSE_CHECK int check55(char *start,char *end,int mod) { int startoff=-1,endoff=0; int off=0,c=0; int ret = 0; #ifdef VERBOSE_CHECK printf("%03d\t",off); #endif for(;start<end;start++,off++) { if(*start != 0x55) { if(startoff<0) { startoff=off; } endoff=off; } #ifdef VERBOSE_CHECK if(++c>=33) { printf("\n%03d\t",off); c=1; } printf("%02x ",*(unsigned char*)start); #endif } printf("\n"); if(startoff>=0) { printf(" ** buffer changed from %d to %d => %d bytes modified **\n",startoff,endoff, endoff - startoff + 1); if (mod != (endoff-startoff+1)) ret = 1; } return ret; } main() { struct ifconf ifc; char *x; struct ifreq *ifr; struct sockaddr_in *sin; int len,ret; int s; char buf[1024]; int bug=0; #define END_TEST 300 if ((s = socket(AF_INET,SOCK_STREAM,0)) == -1) return -1; for (len=1;len<=END_TEST;len++) { ifc.ifc_buf = buf; ifc.ifc_len = len; memset(buf,0x55,sizeof(buf)); printf("\n"); printf("[Try with len=%d]\n",len); if ((ret=ioctl(s,SIOCGIFCONF,&ifc)) < 0) { printf(" => ioctl failed (returned %d, errno=%d)\n",ret,errno); } printf(" => ioctl succeeded, pretends it wrote %d bytes\n",ifc.ifc_len); if ((ifc.ifc_len > len) && bug < 1) { bug = 1; } printf("\n"); if(check55(buf,buf+sizeof(buf),ifc.ifc_len) && bug > 0) { bug = 2; } printf("\n"); } switch (bug) { case 0: printf("\n\n*** Implementation OK (FIXED) *** \n\n"); break; case 1: printf("\n\n*** Implementation returns incorrect ifc.ifc_len, but buffer OK (HEAD) ***\n\n"); break; case 2: printf("\n\n*** Implementation corrupts buffer (RELENG_3) ***\n\n"); break; default: printf("\n\n*** Huh ??? %d ***", bug); break; } return bug; }
State Changed From-To: open->feedback Please chck if rev. 1.86 of if.c (in 5.0-current) or rev 1.85.2.1 (4.-stable) fixes it
The bug is fixed. Patrick.
State Changed From-To: feedback->closed Originator reports that this is fixed.