I've been using the Oracle bind table functionality in DBD::Oracle, and have
discovered that if you reuse a cursor and change the size of the bound array,
then you run into memory problems. Tracing this into the DBD::Oracle code, it
looks like dbdimp.c only allocates the storage buffer the first time it enters
the binding functions (based on the value of phs->array_numstruct). However,
the array size reported to Oracle is the size of the physical perl storage
array. This means on subsequent calls with a different array size can have the
allocated memory and stored memory sizes out of sync resulting in memory
corruption.
I could not find a reason (other than a marginal performance gain) for maintain
the check preventing reallocation of the storage array, so I'm submitting a
patch to remove this.
This patch was built against DBD::Oracle v1.27
Let me know if you have any questions/comments,
Rod.
--- DBD-Oracle-1.27/dbdimp.c 2010-12-14 16:41:56.000000000 -0800
+++ DBD-Oracle-1.27.new/dbdimp.c 2011-02-18 15:24:18.000000000 -0800
@@ -1625,6 +1625,7 @@
int flag_data_is_utf8=0;
int need_allocate_rows;
int buflen;
+ int numarrayentries;
if( ( ! SvROK(phs->sv) ) || (SvTYPE(SvRV(phs->sv))!=SVt_PVAV)
) { /* Allow only array binds */
croak("dbd_rebind_ph_varchar2_table(): bad bind variable. ARRAY
reference required, but got %s for '%s'.",
neatsvpv(phs->sv,0), phs->name);
@@ -1638,9 +1639,12 @@
/* If no number of entries to bind specified,
* set phs->array_numstruct to the scalar(@array) bound.
*/
- if( phs->array_numstruct <= 0 ){
+ /* This used to check to see if phs->array_numstruct was nonzero, but
+ this doesn't allow us to resize storage if the array sizes change
+ on subsequent calls. This potentially results in a buffer overflow:
+ rodcham 20110218 */
/* av_len() returns last array index, or -1 is array is empty */
- int numarrayentries=av_len( arr );
+ numarrayentries=av_len( arr );
if( numarrayentries >= 0 ){
phs->array_numstruct = numarrayentries+1;
if (trace_level >= 2 || dbd_verbose >= 3 ){
@@ -1648,7 +1652,6 @@
phs->array_numstruct);
}
}
- }
/* Fix charset */
csform = phs->csform;
if (trace_level >= 2 || dbd_verbose >= 3 ){
@@ -2011,6 +2014,7 @@
AV *arr;
int need_allocate_rows;
int buflen;
+ int numarrayentries;
/*int flag_data_is_utf8=0;*/
if( ( ! SvROK(phs->sv) ) || (SvTYPE(SvRV(phs->sv))!=SVt_PVAV)
) { /* Allow only array binds */
@@ -2037,17 +2041,19 @@
/* If no number of entries to bind specified,
* set phs->array_numstruct to the scalar(@array) bound.
*/
- if( phs->array_numstruct <= 0 ){
-/* av_len() returns last array index, or -1 is array is empty */
- int numarrayentries=av_len( arr );
- if( numarrayentries >= 0 ){
- phs->array_numstruct =
numarrayentries+1;
- if (trace_level >= 2 ||
dbd_verbose >= 3 ){
-
PerlIO_printf(DBILOGFP, "dbd_rebind_ph_number_table(): array_numstruct=%d
(calculated) \n",
-
phs->array_numstruct);
- }
- }
- }
+ /* This used to check to see if phs->array_numstruct was nonzero, but
+ this doesn't allow us to resize storage if the array sizes change
+ on subsequent calls. This potentially results in a buffer overflow:
+ rodcham 20110218 */
+ /* av_len() returns last array index, or -1 is array is empty */
+ numarrayentries=av_len( arr );
+ if( numarrayentries >= 0 ){
+ phs->array_numstruct = numarrayentries+1;
+ if (trace_level >= 2 || dbd_verbose >= 3 ){
+ PerlIO_printf(DBILOGFP, "dbd_rebind_ph_number_table():
array_numstruct=%d (calculated) \n",
+ phs->array_numstruct);
+ }
+ }
/* Calculate each bound structure maxlen.
* maxlen(int) = sizeof(int);
* maxlen(double) = sizeof(double);
