Opened 16 years ago
Closed 16 years ago
#2075 closed enhancement (fixed)
[PATCH] Add VSISafeMul2, etc. functions to detect overflows
Reported by: | Even Rouault | Owned by: | Even Rouault |
---|---|---|---|
Priority: | normal | Milestone: | 1.6.0 |
Component: | default | Version: | svn-trunk |
Severity: | normal | Keywords: | overflow memory allocation |
Cc: | warmerdam, Mateusz Łoskot |
Description
In GDAL, we frequently do things like VSIMalloc(nXSize * nYSize * sizeof(float)). If nXSize and nYSize are big enough and well choosen, the result can be a moderate value, and VSIMalloc can return a non-NULL pointer. However, the memory buffer will probably be overrun in later operations.
The first patch gdal_svn_trunk_vsi_safe_mul.patch adds new API to do safe multiplications with overflow checking. The second patch uses these new API in gdal/gcore. The third patch uses these new API in gdal/frmts. Note that it only uses these new API at places where VSIMalloc/VSICalloc are already used. Drivers using CPLMalloc/CPLCalloc would crash on big allocations and should be modified to use VSIMalloc/VSICalloc, but that's a much bigger effort.
Attachments (3)
Change History (11)
by , 16 years ago
Attachment: | gdal_svn_trunk_use_vsi_safe_mul_in_gcore.patch added |
---|
comment:1 by , 16 years ago
root@r0:~# cat malloc_test.cpp #include <stdio.h> #include <stdlib.h>
int main() {
printf("%p\n", malloc(0)); return 0;
} root@r0:~# ./malloc_test 0x804a008
So if you use VSIMalloc(VSISafeMul2(x, y)), VSISafeMul2 will return 0, VSIMalloc will allocate 0 bytes and the code that used it will think everything is ok.
VSISafeSizetCastToInt has hardcoded int size + returns correct int value on error.
comment:2 by , 16 years ago
gdal_svn_trunk_vsi_safe_mul.patch updated so that VSIMalloc/Calloc/Realloc return NULL when size is 0.
by , 16 years ago
Attachment: | gdal_svn_trunk_vsi_safe_mul.patch added |
---|
by , 16 years ago
Attachment: | gdal_svn_trunk_use_vsi_safe_mul_in_frmts.patch added |
---|
Use of new API in gdal/frmts
comment:3 by , 16 years ago
I'm wondering if the change in VSIMalloc/Calloc/Realloc is not a bit too disruptive... I had to change hfa_band.cpp line 891 in order not to break hfa_write.py.
comment:4 by , 16 years ago
Milestone: | → 1.6.0 |
---|
Even,
I'd be more comfortable holding this change to 1.6 at this point. Feel free to apply the patch once we have branched off a 1.5 branch.
comment:5 by , 16 years ago
Cc: | added |
---|
comment:8 by , 16 years ago
Resolution: | → fixed |
---|---|
Status: | new → closed |
Use of new API in gdal/gcore