At 03:51 PM 1/25/2002 -0500, you wrote:
>At 03:26 PM 1/25/2002 -0500, you wrote:
>>Randy Moore <[EMAIL PROTECTED]> wrote:
<snip>
>>   And if you look around line 350, there's a check: if row[0] is NULL,
>>then stop.
>>
>>   So there's some weird memory corruption going on here.
>
>Hmm,  on lines 340 - 341:
>row = (inst->module->sql_fetch_row)(sqlsocket, inst->config);
>(inst->module->sql_finish_select_query)(sqlsocket, inst->config);
>
>We fetch the row and then immediately do the mysql 'finish'.  I've never 
>programmed with mysql in C, but in Perl, this would wipe out the memory 
>that the row reference points to.  I'm guessing that the equivalent in C 
>is probably just to free that memory location.  As long as nothing steps 
>on it before we use it, the code might work.
>
>Could it be that we need to either copy the contents of 'row' to a local 
>variable before doing the 'finish'?  Or possibly, we could postpone the 
>'finish' until after all the logic that uses the contents of 'row'.?

Ah, that was it.  I moved the sql_finish line down below the code that uses 
the returned row and no more crashes.

I've included a patch against last nights CVS snapshot.  It should be 
applied against: src/modules/rlm_sql/rlm_sql.c

While rearranging things, I got rid of several 'goto's by adding a couple 
of extra exit points from the subroutine.  If that goes against your coding 
standards, let me know and I'll prepare one that keeps the 'goto's and only 
has a single exit point.

The patch is also downloadable at:
http://www.axion-it.net/download/sql-chap.patch

--- rlm_sql.c.orig      Tue Jan 15 15:53:48 2002
+++ rlm_sql.c   Fri Jan 25 17:48:33 2002
@@ -325,7 +325,8 @@
          */
         if (sql_set_user(inst, request, sqlusername, 0) < 0) {
                 retval = RLM_MODULE_FAIL;
-               goto release_and_return;
+               sql_release_socket(inst, sqlsocket);
+               return(retval);
         }

         radius_xlat(querystr, MAX_QUERY_LEN, 
inst->config->authenticate_query, request, NULL);
@@ -334,23 +335,26 @@
         if ((inst->module->sql_select_query)(sqlsocket, inst->config, 
querystr) < 0) {
                 radlog(L_ERR, "rlm_sql_authenticate: database query error");
                 retval = RLM_MODULE_FAIL;
-               goto release_and_return;
+               sql_release_socket(inst, sqlsocket);
+               return(retval);
         }

         row = (inst->module->sql_fetch_row)(sqlsocket, inst->config);
-       (inst->module->sql_finish_select_query)(sqlsocket, inst->config);
-       sql_release_socket(inst, sqlsocket);

         if (row == NULL) {
                 radlog(L_ERR, "rlm_sql_authenticate: no rows returned from 
query (no such user)");
                 retval = RLM_MODULE_REJECT;
-               goto return_only;
+               (inst->module->sql_finish_select_query)(sqlsocket, 
inst->config);
+               sql_release_socket(inst, sqlsocket);
+               return(retval);
         }

         if (row[0] == NULL) {
                 radlog(L_ERR, "rlm_sql_authenticate: row[0] returned NULL.");
                 retval = RLM_MODULE_REJECT;
-               goto return_only;
+               (inst->module->sql_finish_select_query)(sqlsocket, 
inst->config);
+               sql_release_socket(inst, sqlsocket);
+               return(retval);
         }

         /*
@@ -385,11 +389,9 @@
                 }
         }

-       goto return_only;

-release_and_return:
+       (inst->module->sql_finish_select_query)(sqlsocket, inst->config);
         sql_release_socket(inst, sqlsocket);
-return_only:
         return(retval);
  }


Randy Moore
Axion Information Technologies, Inc.

email     [EMAIL PROTECTED]
phone   301-408-1200
fax        301-445-3947


- 
List info/subscribe/unsubscribe? See http://www.freeradius.org/list/users.html

Reply via email to