List:Commits« Previous MessageNext Message »
From:Olav Sandstaa Date:March 5 2010 12:59pm
Subject:Re: bzr commit into mysql-next-mr-bugfixing branch (marc.alff:3122)
Bug#51738
View as plain text  
Hi Marc,

Thanks for fixing this. The code change looks fine and I have verified 
that it solves the problem. OK to push this fix.

Still, I have one question: what will happen if there exists multiple 
very long files with the almost same name and these after truncation 
will be "identical"? E.g. find_or_create_file() get called twice with 
two different filenames but due to the truncation these two filenames 
becomes "identical". Will this be a problem?

Thanks,
Olav

Marc Alff wrote:
> #At file:///home/malff/BZR_TREE/mysql-next-mr-bugfixing-51738/ based on
> revid:alik@stripped
>
>  3122 Marc Alff	2010-03-04
>       Bug#51738 Unit test pfs_instr-t crashes
>       
>       The unit test pfs_instr-t:
>       - generates a very long (10,000) bytes file name
>       - calls find_or_create_file.
>       
>       This leads to a buffer overflow in mysys in my_realpath(),
>       because my_realpath and mysys file APIs in general do not
>       test for input parameters: mysys assumes every file name
>       is less that FN_REFLEN in length.
>       
>       Calling find_or_create_file with a very long file name is likely
>       to happen when instrumenting third party code that does not use mysys,
>       so this test is legitimate.
>       
>       The fix is to make find_or_create_file in the performance schema
>       more robust in this case.
>
>     modified:
>       storage/perfschema/pfs_instr.cc
> === modified file 'storage/perfschema/pfs_instr.cc'
> --- a/storage/perfschema/pfs_instr.cc	2010-03-02 00:10:01 +0000
> +++ b/storage/perfschema/pfs_instr.cc	2010-03-05 01:36:54 +0000
> @@ -746,6 +746,26 @@ find_or_create_file(PFS_thread *thread,
>      }
>    }
>  
> +  char safe_buffer[FN_REFLEN];
> +  const char *safe_filename;
> +
> +  if (len >= FN_REFLEN)
> +  {
> +    /*
> +      The instrumented code uses file names that exceeds FN_REFLEN.
> +      This could be legal for instrumentation on non mysys APIs,
> +      so we support it.
> +      Truncate the file name so that:
> +      - it fits into pfs->m_filename
> +      - it is safe to use mysys apis to normalize the file name.
> +    */
> +    memcpy(safe_buffer, filename, FN_REFLEN - 2);
> +    safe_buffer[FN_REFLEN - 1]= 0;
> +    safe_filename= safe_buffer;
> +  }
> +  else
> +    safe_filename= filename;
> +
>    /*
>      Normalize the file name to avoid duplicates when using aliases:
>      - absolute or relative paths
> @@ -759,7 +779,7 @@ find_or_create_file(PFS_thread *thread,
>      Ignore errors, the file may not exist.
>      my_realpath always provide a best effort result in buffer.
>    */
> -  (void) my_realpath(buffer, filename, MYF(0));
> +  (void) my_realpath(buffer, safe_filename, MYF(0));
>  
>    normalized_filename= buffer;
>    normalized_length= strlen(normalized_filename);
>
>   
> ------------------------------------------------------------------------
>
>

Thread
bzr commit into mysql-next-mr-bugfixing branch (marc.alff:3122) Bug#51738Marc Alff5 Mar
  • Re: bzr commit into mysql-next-mr-bugfixing branch (marc.alff:3122)Bug#51738Olav Sandstaa5 Mar