| Summary: | crash in winlib if more than 32 bits bpp | ||
|---|---|---|---|
| Product: | SDL | Reporter: | François Gagné <frenchfrog> |
| Component: | video | Assignee: | Ryan C. Gordon <icculus> |
| Status: | RESOLVED FIXED | QA Contact: | Sam Lantinga <slouken> |
| Severity: | critical | ||
| Priority: | P2 | ||
| Version: | 1.2.11 | ||
| Hardware: | x86 | ||
| OS: | Windows (XP) | ||
... Bugzilla bugs out when trying to add the patch ...
-----------
Index: SDL_dibvideo.c
===================================================================
--- SDL_dibvideo.c (revision)
+++ SDL_dibvideo.c (working copy)
@@ -209,7 +209,7 @@
int next_mode;
/* Check to see if we already have this mode */
- if ( bpp < 8 ) { /* Not supported */
+ if ( bpp < 8 || bpp > 32 ) { /* Not supported */
return(0);
}
index = ((bpp+7)/8)-1;
@@ -387,7 +387,7 @@
/* We support any format at any dimension */
SDL_Rect **DIB_ListModes(_THIS, SDL_PixelFormat *format, Uint32 flags)
{
- if ( (flags & SDL_FULLSCREEN) == SDL_FULLSCREEN ) {
+ if ( (flags & SDL_FULLSCREEN) == SDL_FULLSCREEN && format->BitsPerPixel <= 32 ) {
return(SDL_modelist[((format->BitsPerPixel+7)/8)-1]);
} else {
return((SDL_Rect **)-1);
Thanks, this is fixed in subversion revision 3453 I'm assuming the second part of the patch isn't needed if the modes aren't accepted in the first place? To be honest, I don't know the internals of SDL, so I don't know if 'SDL_PixelFormat *format' could come from a request external to SDL and thus have arbitrary value.
In any case, the way the index is calculated '((format->BitsPerPixel+7)/8)-1' is _unsafe_ because if 'format->BitsPerPixel' is bigger then 32 it will access _invalid_ memory since the array has a size of NUM_MODELISTS = 4
As a side note, in the following lines, '32' could have been replaced by '(NUM_MODELISTS * 8)' which I guess it's better because if NUM_MODELISTS is changed the bpp limitations will follow:
+ if ( bpp < 8 || bpp > 32 ) { /* Not supported */
+ if ( (flags & SDL_FULLSCREEN) == SDL_FULLSCREEN && format->BitsPerPixel <= 32 ) {
|
From time to time I receive crash reports from SDL. After some investigation and the help of a user, I think I finally pinpointed the problem. For some unknown reason it seems some Nvidia video cards reports 64 bits per pixel (bpp) in EnumDisplaySettings. This make src/video/windib/SDL_dibvideo.c:DIB_AddMode crash because it cannot handle bits per pixel over 32. I did a patch to limit bpp to 32 bits and it solved the problem for the user. Another way around that bug would be to bump src/video/windib/SDL_dibvideo.h:NUM_MODELISTS to 8 #define NUM_MODELISTS 8 But I wasn't sure SDL could handle bbp higher than 32. ------------- Video card of the user: Card name: NVIDIA GeForce Go 6800 Ultra Manufacturer: NVIDIA Chip type: GeForce Go 6800 Ultra DAC type: Integrated RAMDAC Device Key: Enum\PCI\VEN_10DE&DEV_00C9&SUBSYS_019C1028&REV_A2 Display Memory: 256.0 MB Current Mode: 1920 x 1200 (32 bit) (60Hz) Monitor: (Standardmonitor) Monitor Max Res: Driver Name: nv4_disp.dll Driver Version: 6.14.0011.6375 (English) DDI Version: 9 (or higher) Driver Attributes: Final Retail Driver Date/Size: 10/5/2007 04:44:00, 5783424 bytes WHQL Logo'd: No WHQL Date Stamp: None VDD: Nicht zutreffend Mini VDD: nv4_mini.sys Mini VDD Date: 10/5/2007 04:44:00, 6854464 bytes Device Identifier: {D7B71E3E-4389-11CF-7F6B-972103C2CB35} Vendor ID: 0x10DE Device ID: 0x00C9 SubSys ID: 0x019C1028 Revision ID: 0x00A2 Revision ID: 0x00A2