FTP bug to resolve

[matute]

Hi everyone,

I had a big problem with FTP in server mode, but fortunately together, with my colleage Marcos Di Fazio, we have solved it. Let me explain the problem and its resolve.

When we tried to make a lots of FTP conections using user authentications from a client software, the connections were made until suddenly we could not connect it any more. Then looking for the origin of problems, we saw that FTP task did not have stack errors or problems and seemed to work perfectly. So, when we went to see memory information we discovered that the high water mark was 99%. It seem that FTP task, with all successive connections, consumes all memory of the system pool!

Well, in front of this huge problem (we need a lots of FTP connections) we decided to review the FTP source code, particulary "ftpd_cmd.c" file. We inspected all of defined functions and its contents and discovered something amazing! One of the function that FTP task uses to makes connections using user authentications is "int_32  FTPd_user(FTPd_SESSION_PTR session_ptr, char_ptr cmd_ptr, char_ptr arg_ptr )" (row 1320 of ftpd_cmd.c). This function, when does a correct connection, NEVER frees the memory that it allocates with "MFS_alloc_path" at beginig!!!

Following, the function that I'm talking. At the end (line 1411 of ftpd_cmd.c file), red colored text, the MFS_free_path() that frees memory allocated first and does not exist in the file):

/*FUNCTION*-------------------------------------------------------------------

*

* Function Name    :   FTPd_user

* Returned Value   :  int_32 error code

* Comments  : 

*

* Usage: 

*

*END*---------------------------------------------------------------------*/

int_32  FTPd_user(FTPd_SESSION_PTR session_ptr, char_ptr cmd_ptr, char_ptr arg_ptr )

{

#if FTPDCFG_ENABLE_USERNAME_AND_PASSWORD && FTPDCFG_USES_MFS

/* Begin CR 2296 */

   uint_32     i=0,j,error;

/* End CR 2296 */

   boolean     found = FALSE;

   MQX_FILE_PTR fd;

   char_ptr    ptr,abs_path;

   ptr = session_ptr->DATA_BUFFER_PTR;

   // Any attempt to log in logs previous user out.

   session_ptr->LOGIN_STATE = FTPD_LOGGED_OUT;

   session_ptr->PASSWORD[0] = '\0';

   if (FTPd_userfile != NULL)  {

      if (MFS_alloc_path(&abs_path) != MFS_NO_ERROR) {

         FTPd_send_msg(session_ptr, ftpmsg_no_memory );

         FTPd_send_msg(session_ptr, ftpmsg_bye);

         return FTPd_ERROR;

      }

      error = _io_rel2abs(abs_path,

                         session_ptr->CURRENT_FS_DIR,

                         FTPd_userfile,

                         FTPD_PATHLEN,

                         session_ptr->CURRENT_FS_NAME);      

      if(error)

      {

         FTPd_send_msg(session_ptr, ftpmsg_bye);

         MFS_free_path(abs_path);

         return FTPd_ERROR; 

      }

      fd=fopen(abs_path, "r");

      if (fd)  {

         while (fgets(ptr, FTPD_LINELEN, fd))  {

            if (*ptr == '\0') continue;

            for (i=0;(i<FTPD_LINELEN) && (ptr[i] != ':') && (ptr[i] != '\0'); i++)  {

            }

            if ((arg_ptr[i]=='\0') && (strncmp(ptr, arg_ptr, i)==0))  {

               found = TRUE;

               break;

            }

         }

         if (found)  {

            strncpy(session_ptr->USERNAME,&ptr[0], i);

            session_ptr->USERNAME[i]='\0';

            //check if there is only username or also password (:)

            if (session_ptr->DATA_BUFFER_PTR[i] == '\0')  {

               session_ptr->LOGIN_STATE = FTPD_LOGGED_IN;

            } else  {

               //there is : delimiter - check if there is valid password

               for (j=i+1;(j<FTPD_LINELEN) && (ptr[j] != ':') && (ptr[j] != '\0'); j++)  {

               }

               if (j==(i+1))  {

                  session_ptr->LOGIN_STATE = FTPD_LOGGED_IN;

               } else  {

                  session_ptr->LOGIN_STATE = FTPD_NEED_PASSWORD;

                  strncpy(session_ptr->PASSWORD,&ptr[i+1], j-i-1);

                  session_ptr->PASSWORD[j-i-1]='\0';

               }

               // todo - here add code for FTP ROOTDIR

            }

         } else  {

            session_ptr->LOGIN_ATTEMPTS++;

            if (session_ptr->LOGIN_ATTEMPTS >= FTPD_MAX_LOGIN_ATTEMPTS)  {

               session_ptr->CONNECTED = FALSE;  

               FTPd_send_msg(session_ptr, ftpmsg_bye);

               MFS_free_path(abs_path);

               return FTPd_ERROR; 

            }

         }

         fclose(fd);    

      }

   } else  {

      session_ptr->LOGIN_STATE = FTPD_LOGGED_IN;

   }

   if (session_ptr->LOGIN_STATE == FTPD_LOGGED_IN)  {

      FTPd_send_msg(session_ptr, ftpmsg_logged);

   } else  {

      if (session_ptr->LOGIN_STATE == FTPD_NEED_PASSWORD)  {

         FTPd_send_msg(session_ptr, ftpmsg_need_password);

      } else  {

         FTPd_send_msg(session_ptr, ftpmsg_not_logged);

      }

   }

   MFS_free_path(abs_path); // This line frees memory that the function allocated at the beging and never frees if connection was made

#else

   FTPd_send_msg(session_ptr, ftpmsg_logged);

#endif // FTPDCFG_ENABLE_USERNAME_AND_PASSWORD || FTPDCFG_USES_MFS

      return FTPd_OK;

}

This MFS_free_path function has solved all of memory problems that we had when made a lots FTP connections, after recompile RTCS module.

Please, review this problem to improve it at the next MQX version, because we are using MQX 3.6 and we saw that in MQX 4.0 the problem does not solve yet.

In attached file, I added MFS_free_path at line 1411.

Best regards!

Original Attachment has been moved to: ftpd_cmd.c.zip

[karelm_]

Hi Matias,

this bug is going to be fixed in MQX 4.0.1.

Best regards,

Karel Mozny

[c0170]

Hello Matias Ferrari,

first of all, thank you for your contribution. I really appreciate sharing what you have found and proposing a fix.

I will hand this to our team. We will update you with the outcome.

Regards,

MartinK