Re: [Qemu-devel] [PATCHv2 03/22] block: avoid buffer overrun by using pstrcpy, not strncpy

[Stefan Weil]

Am 30.05.2012 09:46, schrieb Jim Meyering:
> From: Jim Meyering<address@hidden>
>
> Also, use PATH_MAX, rather than the arbitrary 1024.
> Using PATH_MAX is more consistent with other filename-related
> variables in this file, like backing_filename and tmp_filename.
>
> Acked-by: Kevin Wolf<address@hidden>
> Signed-off-by: Jim Meyering<address@hidden>
> ---
>   block.c | 5 +++--
>   1 file changed, 3 insertions(+), 2 deletions(-)
>
> diff --git a/block.c b/block.c
> index af2ab4f..efc7071 100644
> --- a/block.c
> +++ b/block.c
> @@ -1232,7 +1232,7 @@ int bdrv_commit(BlockDriverState *bs)
>       int n, ro, open_flags;
>       int ret = 0, rw_ret = 0;
>       uint8_t *buf;
> -    char filename[1024];
> +    char filename[PATH_MAX];
>       BlockDriverState *bs_rw, *bs_ro;
>
>       if (!drv)
> @@ -1252,7 +1252,8 @@ int bdrv_commit(BlockDriverState *bs)
>
>       backing_drv = bs->backing_hd->drv;
>       ro = bs->backing_hd->read_only;
> -    strncpy(filename, bs->backing_hd->filename, sizeof(filename));
> +    /* Use pstrcpy (not strncpy): filename must be NUL-terminated. */
> +    pstrcpy(filename, sizeof(filename), bs->backing_hd->filename);
>       open_flags =  bs->backing_hd->open_flags;
>
>       if (ro) {

PATH_MAX can have any value, from 259 or 512 on Windows to
4096 on Linux or even more.

I usually avoid arrays with more than some hundred bytes on the stack.
Even if the stack is large enough, it's not good for caching.

Of course, avoiding arbitrary limits like PATH_MAX would be best.

Using a QEMU_PATH_MAX which is the same for all hosts might be
the second best solution.

Regards,
Stefan Weil