crash in DBD::mysql/ statement leak in DBD::mysql

--=-=-=

This applies to the current CPAN-version of the DBD::mysql module.

When using server side prepared statements, storage for 'bind values'
is allocated in dbd_st_prepare and freed in dbd_st_finish after all
rows resulting from one execution of a query have been fetched. This
means the next attempt to execute the prepared statement will
segfault when trying to use those storage.

Fix for this issue: Move the freeing-code to the dbd_st_destroy
routine.

Additionally, the person who delete the 'free everything' code from
dbd_st_destroy accidentally deleted the once line of code that was
different from the similar chunk in dbd_st_finish, namely, the
mysql_stmt_close call, meaning, prepared statements are leaked in the
server.

Fix for this issue: Re-add mysql_stmt_close in dbd_st_destroy.

The attached patch does both of this.

NB: Both of this issues have already been reported as bugs, but
apparently, the responsible person believed that the first doesn't
actually exist and for the second, a patch has been proposed which
would make the thing worse (calling mysql_stmt_close in finish will
prevent re-executing prepared select statements after all results of
one execution have been fetched). Additionally, the patch implements
only the two fixes and not a larger set of 'random other changes',
like the one for #1 available from the Mysql-BTS did.


--=-=-=
Content-Disposition: attachment; filename=patch
Content-Description: patch for DBD::Mysql

diff -rNu DBD-mysql.cpan/dbdimp.c DBD-mysql/dbdimp.c
--- DBD-mysql.cpan/dbdimp.c 2007-05-21 10:07:12.000000000 +0200
+++ DBD-mysql/dbdimp.c 2007-05-21 12:30:27.000000000 +0200
@@ -3720,10 +3720,6 @@
#endif

#if MYSQL_VERSION_ID >= SERVER_PREPARE_VERSION
- int i;
- int num_fields;
- imp_sth_fbh_t *fbh;
-
if (dbis->debug >= 2)
{
PerlIO_printf(DBILOGFP, "\n--> dbd_st_finish\n");
@@ -3740,32 +3736,6 @@
return 0;
}
}
- /* clean up other statement allocations */
- if (DBIc_NUM_PARAMS(imp_sth) > 0)
- {
- if (dbis->debug >= 2)
- PerlIO_printf(DBILOGFP,
- "\tFreeing %d parameters\n",
- DBIc_NUM_PARAMS(imp_sth));
- free_bind(imp_sth->bind);
- free_fbind(imp_sth->fbind);
- imp_sth->bind= NULL;
- imp_sth->fbind= NULL;
- }
- num_fields= DBIc_NUM_FIELDS(imp_sth);
-
- if (imp_sth->fbh)
- {
- for (fbh= imp_sth->fbh, i= 0; i < num_fields; i++, fbh++)
- {
- if (fbh->data)
- Safefree(fbh->data);
- }
- free_fbuffer(imp_sth->fbh);
- free_bind(imp_sth->buffer);
- imp_sth->buffer= NULL;
- imp_sth->fbh= NULL;
- }
}
#endif

@@ -3812,6 +3782,47 @@

int i;

+#if MYSQL_VERSION_ID >= SERVER_PREPARE_VERSION
+ imp_sth_fbh_t *fbh;
+ int n;
+
+ n = DBIc_NUM_PARAMS(imp_sth);
+ if (n)
+ {
+ if (dbis->debug >= 2)
+ PerlIO_printf(DBILOGFP, "\tFreeing %d parameters\n",
+ n);
+
+ free_bind(imp_sth->bind);
+ free_fbind(imp_sth->fbind);
+ }
+
+ fbh = imp_sth->fbh;
+ if (fbh)
+ {
+ n = DBIc_NUM_FIELDS(imp_sth);
+ i = 0;
+ while (i < n)
+ {
+ if (fbh[i].data) Safefree(fbh[i].data);
+ ++i;
+ }
+
+ free_fbuffer(fbh);
+ free_bind(imp_sth->buffer);
+ }
+
+ if (imp_sth->stmt)
+ {
+ if (!mysql_stmt_close(imp_sth->stmt))
+ {
+ do_error(DBIc_PARENT_H(imp_sth), mysql_stmt_errno(imp_sth->stmt),
+ mysql_stmt_error(imp_sth->stmt),
+ mysql_stmt_sqlstate(imp_sth->stmt));
+ }
+ }
+#endif
+
/* dbd_st_finish has already been called by .xs code if needed. */

/* Free values allocated by dbd_bind_ph */


--=-=-=
Content-Type: text/plain; charset=us-ascii


--
MySQL Perl Mailing List
For list archives: http://lists.mysql.com/perl
To unsubscribe: http://lists.mysql.com/perl?unsub=gcdmp-msql-mysql-modules@m .gmane.org
--=-=-=--

Re: crash in DBD::mysql/ statement leak in DBD::mysql

Rainer,

Thanks for the patch! I will test it this week against various versions
of MySQL.

Regards,

Patrick

Rainer Weikusat wrote:

>This applies to the current CPAN-version of the DBD::mysql module.
>
>When using server side prepared statements, storage for 'bind values'
>is allocated in dbd_st_prepare and freed in dbd_st_finish after all
>rows resulting from one execution of a query have been fetched. This
>means the next attempt to execute the prepared statement will
>segfault when trying to use those storage.
>
>Fix for this issue: Move the freeing-code to the dbd_st_destroy
>routine.
>
>Additionally, the person who delete the 'free everything' code from
>dbd_st_destroy accidentally deleted the once line of code that was
>different from the similar chunk in dbd_st_finish, namely, the
>mysql_stmt_close call, meaning, prepared statements are leaked in the
>server.
>
>Fix for this issue: Re-add mysql_stmt_close in dbd_st_destroy.
>
>The attached patch does both of this.
>
>NB: Both of this issues have already been reported as bugs, but
>apparently, the responsible person believed that the first doesn't
>actually exist and for the second, a patch has been proposed which
>would make the thing worse (calling mysql_stmt_close in finish will
>prevent re-executing prepared select statements after all results of
>one execution have been fetched). Additionally, the patch implements
>only the two fixes and not a larger set of 'random other changes',
>like the one for #1 available from the Mysql-BTS did.
>
>
>
>----------------------------------------------------------- -------------
>
>diff -rNu DBD-mysql.cpan/dbdimp.c DBD-mysql/dbdimp.c
>--- DBD-mysql.cpan/dbdimp.c 2007-05-21 10:07:12.000000000 +0200
>+++ DBD-mysql/dbdimp.c 2007-05-21 12:30:27.000000000 +0200
>@@ -3720,10 +3720,6 @@
> #endif
>
> #if MYSQL_VERSION_ID >= SERVER_PREPARE_VERSION
>- int i;
>- int num_fields;
>- imp_sth_fbh_t *fbh;
>-
> if (dbis->debug >= 2)
> {
> PerlIO_printf(DBILOGFP, "\n--> dbd_st_finish\n");
>@@ -3740,32 +3736,6 @@
> return 0;
> }
> }
>- /* clean up other statement allocations */
>- if (DBIc_NUM_PARAMS(imp_sth) > 0)
>- {
>- if (dbis->debug >= 2)
>- PerlIO_printf(DBILOGFP,
>- "\tFreeing %d parameters\n",
>- DBIc_NUM_PARAMS(imp_sth));
>- free_bind(imp_sth->bind);
>- free_fbind(imp_sth->fbind);
>- imp_sth->bind= NULL;
>- imp_sth->fbind= NULL;
>- }
>- num_fields= DBIc_NUM_FIELDS(imp_sth);
>-
>- if (imp_sth->fbh)
>- {
>- for (fbh= imp_sth->fbh, i= 0; i < num_fields; i++, fbh++)
>- {
>- if (fbh->data)
>- Safefree(fbh->data);
>- }
>- free_fbuffer(imp_sth->fbh);
>- free_bind(imp_sth->buffer);
>- imp_sth->buffer= NULL;
>- imp_sth->fbh= NULL;
>- }
> }
> #endif
>
>@@ -3812,6 +3782,47 @@
>
> int i;
>
>+#if MYSQL_VERSION_ID >= SERVER_PREPARE_VERSION
>+ imp_sth_fbh_t *fbh;
>+ int n;
>+
>+ n = DBIc_NUM_PARAMS(imp_sth);
>+ if (n)
>+ {
>+ if (dbis->debug >= 2)
>+ PerlIO_printf(DBILOGFP, "\tFreeing %d parameters\n",
>+ n);
>+
>+ free_bind(imp_sth->bind);
>+ free_fbind(imp_sth->fbind);
>+ }
>+
>+ fbh = imp_sth->fbh;
>+ if (fbh)
>+ {
>+ n = DBIc_NUM_FIELDS(imp_sth);
>+ i = 0;
>+ while (i < n)
>+ {
>+ if (fbh[i].data) Safefree(fbh[i].data);
>+ ++i;
>+ }
>+
>+ free_fbuffer(fbh);
>+ free_bind(imp_sth->buffer);
>+ }
>+
>+ if (imp_sth->stmt)
>+ {
>+ if (!mysql_stmt_close(imp_sth->stmt))
>+ {
>+ do_error(DBIc_PARENT_H(imp_sth), mysql_stmt_errno(imp_sth->stmt),
>+ mysql_stmt_error(imp_sth->stmt),
>+ mysql_stmt_sqlstate(imp_sth->stmt));
>+ }
>+ }
>+#endif
>+
> /* dbd_st_finish has already been called by .xs code if needed. */
>
> /* Free values allocated by dbd_bind_ph */
>
>
>
>----------------------------------------------------------- -------------
>
>
>
>


--
Patrick Galbraith, Senior Programmer
Grazr - Easy feed grazing and sharing
http://www.grazr.com

Satyam Eva Jayate - Truth Alone Triumphs
Mundaka Upanishad




--
MySQL Perl Mailing List
For list archives: http://lists.mysql.com/perl
To unsubscribe: http://lists.mysql.com/perl?unsub=gcdmp-msql-mysql-modules@m .gmane.org

Re: crash in DBD::mysql/ statement leak in DBD::mysql

Patrick Galbraith writes:
> Thanks for the patch! I will test it this week against various
> versions of MySQL.

One problem I know of already is that the error handling in
dbd_st_destroy is b0rked. The logic is inverted and getting at the dbh
the way I tried to does not always work this way. What I currently use
looks like this:

if (mysql_stmt_close(imp_sth->stmt))
{
/*
Only documented 'known error' is 'server has gone
away'. In case of others, the prepared statement
will be leaked in the server.
*/
PerlIO_printf(DBILOGFP,
"DESTROY(%p): mysql_stmt_close failed, error %d (%s).\n",
sth, mysql_stmt_errno(imp_sth->stmt),
mysql_stmt_error(imp_sth->stmt));
}

I don't think there is a point in calling do_error with the sth,
because the DESTROY context implies that the (Perl-)program has no longer
a way to access the statement handle (otherwise, destroy wouldn't
have been called).

Information which may be of use: I am using this w/ mysql 5.0.32/
Debian stable and it works there.


--
MySQL Perl Mailing List
For list archives: http://lists.mysql.com/perl
To unsubscribe: http://lists.mysql.com/perl?unsub=gcdmp-msql-mysql-modules@m .gmane.org