Buffer overflow fix

SDL Development
1

Okay, this isn’t really a fix so much as a cleanup, but seeing comments
like “/* Caution: potential buffer overflow! */” drives me nuts, so I
fixed it.

Attached is a patch to clean up such a warning in the Linux SDL CD-ROM code.

To apply:

cd /where/i/installed/the/cvs/version/of/SDL12
patch -p1 < SDL-linux-cdrom-overflow-RYAN1.diff

–ryan.

-------------- next part --------------
— SDL12/src/cdrom/linux/SDL_syscdrom.c-virgin Mon Aug 6 23:03:16 2001
+++ SDL12/src/cdrom/linux/SDL_syscdrom.c Mon Aug 6 23:04:18 2001
@@ -181,10 +181,21 @@

mntfp = setmntent(mtab, "r");
if ( mntfp != NULL ) {

  •   char *tmp, mnt_type[32], mnt_dev[1024];

  •   char *tmp;

  •   char *mnt_type;

  •   char *mnt_dev;

  while ( (mntent=getmntent(mntfp)) != NULL ) {

  •   	/* Warning, possible buffer overflow.. */

  •   	mnt_type = malloc(strlen(mntent->mnt_type) + 1);

  •   	if (mnt_type == NULL)

  •   		continue;  /* maybe you'll get lucky next time. */+

  •   	mnt_dev = malloc(strlen(mntent->mnt_fsname) + 1);

  •   	if (mnt_dev == NULL) {

  •   		free(mnt_type);

  •   		continue;

  •   	}

  •   	strcpy(mnt_type, mntent->mnt_type);
  	strcpy(mnt_dev, mntent->mnt_fsname);

@@ -216,6 +227,8 @@
AddDrive(mnt_dev, &stbuf);
}
}

  •   	free(mnt_dev);

  •   	free(mnt_type);
  }
  endmntent(mntfp);
    }
2

“Ryan C. Gordon” wrote

Okay, this isn’t really a fix so much as a cleanup, but seeing
comments like “/* Caution: potential buffer overflow! */” drives
me nuts, so I fixed it.

Attached is a patch to clean up such a warning in the Linux SDL
CD-ROM code.

heh, now the comment has been changed to
/* maybe you’ll get lucky next time. */

instead of a potential buffer overflow there’s now a
potential infinite loop. to the effect of “while(!malloc(x))”

actually, i don’t know if that’s really a problem. in my
experience, the OS always kills an app when it runs out of
memory. (but that may not always be the case?)

3

heh, now the comment has been changed to
/* maybe you’ll get lucky next time. */

instead of a potential buffer overflow there’s now a
potential infinite loop. to the effect of “while(!malloc(x))”

No there isn’t. If it can’t allocate memory, it goes on to the next entry
in the mount table (which is, according to “man setmntent”) stored in a
preallocated, static buffer. If at that moment, there is miraculously 10
bytes of memory available to allocate a string like “/dev/cdrom”, then
processing continues, otherwise, you’ll iterate through the entire mount
table without processing anything, and continue on.

…granted, if you’re THAT low on memory, you aren’t going to get much
further (I don’t imagine the likely upcoming call to SDL_SetVideoMode()
would succeed), but the exercise what to remove a buffer overflow, and
it’s always good to handle malloc failures gracefully in each subsystem.

actually, i don’t know if that’s really a problem. in my
experience, the OS always kills an app when it runs out of
memory. (but that may not always be the case?)

The OS will return NULL in malloc, in which case most careful applications
will gracefully exit, and the not-so-careful ones will write to memory
address 0x00000000, and segfault.

If you’ve never tried this, under Linux your box is generally completely
unusable by the time malloc actually returns NULL, since you’re swapping
like mad. (There are exceptions: if you try to malloc a couple gigs of
memory in one call, generally you’ll get a NULL without any swapping, or
if you’ve got no swapswap defined, etc.)

–ryan.

4

No there isn’t. If it can’t allocate memory, it goes on to the next entry
in the mount table (which is, according to “man setmntent”) stored in a
preallocated, static buffer. If at that moment, there is miraculously 10
bytes of memory available to allocate a string like “/dev/cdrom”, then
processing continues, otherwise, you’ll iterate through the entire mount
table without processing anything, and continue on.

Oh my lord, what was I thinking when I sent this? My apologies for my
awful grammar; run that through your ZeroWing filter if any of it doesn’t
make sense.

–ryan.

5

Don’t worry; I don’t think anyone got seriously injured! :wink:

//David Olofson — Programmer, Reologica Instruments AB

.- M A I A -------------------------------------------------.
| Multimedia Application Integration Architecture |
| A Free/Open Source Plugin API for Professional Multimedia |
----------------------> http://www.linuxaudiodev.com/maia -' .- David Olofson -------------------------------------------. | Audio Hacker - Open Source Advocate - Singer - Songwriter |--------------------------------------> david at linuxdj.com -'On Tuesday 07 August 2001 21:16, Ryan C. Gordon wrote:

No there isn’t. If it can’t allocate memory, it goes on to the next
entry in the mount table (which is, according to “man setmntent”)
stored in a preallocated, static buffer. If at that moment, there is
miraculously 10 bytes of memory available to allocate a string like
"/dev/cdrom", then processing continues, otherwise, you’ll iterate
through the entire mount table without processing anything, and
continue on.

Oh my lord, what was I thinking when I sent this? My apologies for my
awful grammar; run that through your ZeroWing filter if any of it
doesn’t make sense.

6

Okay, this isn’t really a fix so much as a cleanup, but seeing comments
like “/* Caution: potential buffer overflow! */” drives me nuts, so I
fixed it.

Attached is a patch to clean up such a warning in the Linux SDL CD-ROM code.

Cool, it’s in CVS.

Thanks!
-Sam Lantinga, Software Engineer, Blizzard Entertainment