HI Dave,

thanks for the patch!

Just to clarify, the problem does not present a security risk, because it
just generate a problem for the sysadmin or banana script who look into that
file to check the process PID, internal structs of Monkey (no matter how
many instances) keeps the right details..

will push the patch in a little bit..

thanks!

On Tue, Feb 8, 2011 at 10:46 AM, Davidlohr Bueso <[email protected]> wrote:

> From e800f7b3185c3ad0ad638b8a321a77902b94f425 Mon Sep 17 00:00:00 2001
> From: Davidlohr Bueso <[email protected]>
> Date: Tue, 8 Feb 2011 10:43:42 -0300
>
> There is currently a race condition with Monkey's pidfile. If two instances
> of the webserver are started, the pidfile of the first instance will be
> replaced by the second one.
> This presents a security risk and can be avoided having a pidfile for each
> instance differenciating by port.
>
> Signed-off-by: Davidlohr Bueso <[email protected]>
> ---
>  src/utils.c |   21 +++++++++++++++------
>  1 files changed, 15 insertions(+), 6 deletions(-)
>
> diff --git a/src/utils.c b/src/utils.c
> index f78c47f..091639e 100644
> --- a/src/utils.c
> +++ b/src/utils.c
> @@ -395,16 +395,18 @@ int mk_utils_get_somaxconn()
>  int mk_utils_register_pid()
>  {
>     FILE *pid_file;
> +    unsigned long len = 0;
> +    char *filepath = NULL;
>
> -    remove(config->pid_file_path);
> -    config->pid_status = VAR_OFF;
> +    mk_string_build(&filepath, &len, "%s.%d", config->pid_file_path,
> config->serverport);
>
> -    if ((pid_file = fopen(config->pid_file_path, "w")) == NULL) {
> +    if ((pid_file = fopen(filepath, "w")) == NULL) {
>         mk_error(MK_ERROR_FATAL, "Error: I can't log pid of monkey");
>     }
>
>     fprintf(pid_file, "%i", getpid());
>     fclose(pid_file);
> +    mk_mem_free(filepath);
>     config->pid_status = VAR_ON;
>
>     return 0;
> @@ -413,10 +415,17 @@ int mk_utils_register_pid()
>  /* Remove PID file */
>  int mk_utils_remove_pid()
>  {
> -    char *pidfile = config->pid_file_path;
> -
> +    unsigned long len = 0;
> +    char *filepath = NULL;
> +    int ret;
> +
> +    mk_string_build(&filepath, &len, "%s.%d", config->pid_file_path,
> config->serverport);
> +
>     mk_user_undo_uidgid();
> -    return unlink(pidfile);
> +    ret = unlink(filepath);
> +    mk_mem_free(filepath);
> +    config->pid_status = VAR_OFF;
> +    return ret;
>  }
>
>  void mk_error(int type, const char *format, ...)
> --
> 1.7.1
>
>
>


-- 
Eduardo Silva
http://edsiper.linuxchile.cl
http://www.monkey-project.com
_______________________________________________
Monkey mailing list
[email protected]
http://lists.monkey-project.com/listinfo/monkey

Reply via email to