Em qui., 27 de jun. de 2024 às 08:48, Ranier Vilela <ranier...@gmail.com> escreveu:
> Em qui., 27 de jun. de 2024 às 01:01, Yugo NAGATA <nag...@sraoss.co.jp>
> escreveu:
>
>> On Mon, 24 Jun 2024 08:25:36 -0300
>> Ranier Vilela <ranier...@gmail.com> wrote:
>>
>> > Em dom., 23 de jun. de 2024 às 23:56, Richard Guo <
>> guofengli...@gmail.com>
>> > escreveu:
>> >
>> > > On Mon, Jun 24, 2024 at 7:51 AM Ranier Vilela <ranier...@gmail.com>
>> wrote:
>> > > > In src/include/access/xlogbackup.h, the field *name*
>> > > > has one byte extra to store null-termination.
>> > > >
>> > > > But, in the function *do_pg_backup_start*,
>> > > > I think that is a mistake in the line (8736):
>> > > >
>> > > > memcpy(state->name, backupidstr, strlen(backupidstr));
>> > > >
>> > > > memcpy with strlen does not copy the whole string.
>> > > > strlen returns the exact length of the string, without
>> > > > the null-termination.
>> > >
>> > > I noticed that the two callers of do_pg_backup_start both allocate
>> > > BackupState with palloc0. Can we rely on this to ensure that the
>> > > BackupState.name is initialized with null-termination?
>> > >
>> > I do not think so.
>> > It seems to me the best solution is to use Michael's suggestion,
>> strlcpy +
>> > sizeof.
>> >
>> > Currently we have this:
>> > memcpy(state->name, "longlongpathexample1",
>> > strlen("longlongpathexample1"));
>> > printf("%s\n", state->name);
>> > longlongpathexample1
>> >
>> > Next random call:
>> > memcpy(state->name, "longpathexample2", strlen("longpathexample2"));
>> > printf("%s\n", state->name);
>> > longpathexample2ple1
>>
>> In the current uses, BackupState is freed (by pfree or
>> MemoryContextDelete)
>> after each use of BackupState, so the memory space is not reused as your
>> scenario above, and there would not harms even if the null-termination is
>> omitted.
>>
>> However, I wonder it is better to use strlcpy without assuming such the
>> good
>> manner of callers.
>>
> Thanks for your inputs.
>
> strlcpy is used across all the sources, so this style is better and safe.
>
> Here v4, attached, with MAXPGPATH -1, according to your suggestion.
>
Now with file patch really attached.
best regards,
Ranier Vilela
v4-avoid-incomplete-copy-string-do_pg_backup_start.patch
Description: Binary data
