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);
>
>
> ------------------------------------------------------------------------
>
>
