SpartanJ / SOIL2

SOIL2 is a tiny C library used primarily for uploading textures into OpenGL.
MIT No Attribution
370 stars 75 forks source link

SOIL_save_screenshot heap corruption #19

Closed SpartanJ closed 6 years ago

SpartanJ commented 6 years ago

Original report by Anonymous.


Hi,

I've found the glReadPixels call in the SOIL_save_screenshot function occasionally corrupts the heap (depending on odd screen dimensions).

I believe its due to the GL_PACK_ALIGNMENT not being set. My fix is to surround the glReadPixels call with the following:

GLuint nOrgAlignment = 0;
glGetIntegerv(GL_PACK_ALIGNMENT, &nOrgAlignment);

// Ensure proper alignment
if(nOrgAlignment != 1)
    glPixelStorei(GL_PACK_ALIGNMENT, 1);

glReadPixels (x, y, width, height, GL_RGB, GL_UNSIGNED_BYTE, pixel_data);

// Restore original alignment
if (nOrgAlignment != 1)
    glPixelStorei(GL_PACK_ALIGNMENT, nOrgAlignment);
SpartanJ commented 6 years ago

Original comment by Martín Lucas Golini (Bitbucket: SpartanJ, GitHub: SpartanJ).


Sounds you're right! I'll take a look at this after work.

Thanks for reporting it!

SpartanJ commented 6 years ago

Original comment by Terry Russell (Bitbucket: xsensordev, ).


Certainly, no problem.

SpartanJ commented 6 years ago

Original comment by Martín Lucas Golini (Bitbucket: SpartanJ, GitHub: SpartanJ).


Fixed issue #19. Thanks Terry Russell!

SpartanJ commented 6 years ago

Original comment by Terry Russell (Bitbucket: xsensordev, ).


Glad to help. Though I notice you've selected the GL_UNPACK_ALIGNMENT instead of the GL_PACK_ALIGNMENT. Changing the GL_PACK_ALIGNMENT to 1 (from 4) was definitely the change I needed to prevent a heap corruption. I tested setting only the GL_UNPACK_ALIGNMENT and was still corrupting the heap when my window width was an odd value due to user re-sizing (ie: 807x423). This was with the latest nvidia drivers on Windows 10.

Perhaps both should be set to 1?

-Terry

SpartanJ commented 6 years ago

Original comment by Martín Lucas Golini (Bitbucket: SpartanJ, GitHub: SpartanJ).


That's what happens when you try to do something while tired ( and didn't even tested ). It's PACK aligment. Sorry!

PD: Commited the real fix.

SpartanJ commented 6 years ago

Original comment by Terry Russell (Bitbucket: xsensordev, ).


Looks good. Thanks for maintaining this useful library!