Bug 17311 - bug in the code handling ioctl SIOCGIFCONF
Summary: bug in the code handling ioctl SIOCGIFCONF
Status: Closed FIXED
Alias: None
Product: Base System
Classification: Unclassified
Component: kern (show other bugs)
Version: 3.4-STABLE
Hardware: Any Any
: Normal Affects Only Me
Assignee: freebsd-bugs (Nobody)
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2000-03-11 07:20 UTC by patrick
Modified: 2000-06-10 15:31 UTC (History)
0 users

See Also:


Attachments
file.diff (593 bytes, patch)
2000-03-11 07:20 UTC, patrick
no flags Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description patrick 2000-03-11 07:20:01 UTC
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;
}
Comment 1 patrick 2000-03-11 07:39:35 UTC
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;
}
Comment 2 guido freebsd_committer freebsd_triage 2000-04-21 18:54:23 UTC
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 


Comment 3 patrick 2000-06-09 23:17:20 UTC
The bug is fixed.

Patrick.
Comment 4 nrahlstr freebsd_committer freebsd_triage 2000-06-10 15:30:18 UTC
State Changed
From-To: feedback->closed

Originator reports that this is fixed.