Re: Avoid incomplete copy string (src/backend/access/transam/xlog.c)

Em qui., 27 de jun. de 2024 às 01:01, Yugo NAGATA <nagata(at)sraoss(dot)co(dot)jp>
escreveu:

> On Mon, 24 Jun 2024 08:25:36 -0300
> Ranier Vilela <ranier(dot)vf(at)gmail(dot)com> wrote:
>
> > Em dom., 23 de jun. de 2024 às 23:56, Richard Guo <
> guofenglinux(at)gmail(dot)com>
> > escreveu:
> >
> > > On Mon, Jun 24, 2024 at 7:51 AM Ranier Vilela <ranier(dot)vf(at)gmail(dot)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.

From the linux man page:
https://linux.die.net/man/3/strlcpy

" The *strlcpy*() function copies up to *size* - 1 characters from the
NUL-terminated string *src* to *dst*, NUL-terminating the result. "

best regards,
Ranier Vilela