Funny (or unfortunate...), I had submitted a patch for
this as well, just two months ago.  It's in 1.28 (which right
now is at RC 2).

Fix is similar, only addition is resetting the attributes on bind,
around line 3212 in trunk.  (Or it would have gone in line 3058
in 1.26.)

- Tim

Chamberlin, Rod wrote, on or about 2/18/2011 7:50 PM:
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);





--
Tim Oertel       |"Why should I be content to simply
VP Engineering   | live in this world, when I, as a
Ashergroup, Inc. | human being, can CREATE it?!"
585-586-0020     | IM via google/XMPP: ma...@jabber.com

Reply via email to