Re: [hackers] [sbase] [PATCH] xinstall: Fix broken memmove with -t

From: Dimitris Papastamos <sin_AT_2f30.org>
Date: Fri, 2 Dec 2016 12:24:54 +0000

On Fri, Dec 02, 2016 at 01:22:16PM +0100, Laslo Hunhold wrote:
> On Thu, 1 Dec 2016 22:50:20 -0800
> Michael Forney <mforney_AT_mforney.org> wrote:
>
> Hey Michael,
>
> > memmove moves a number of bytes, not pointers, so if you passed a
> > number of arguments that is larger than the pointer byte size, you
> > could end up crashing or skipping the install of a file and
> > installing another twice.
>
> well-observed, nice find!
>
> > Also, argv was never decreased to match the moved arguments, so the -t
> > parameter was added in the NULL argv slot.
>
> > - memmove(argv - 1, argv, argc);
> > + argv = memmove(argv - 1, argv, argc * sizeof(*argv));
>
> I got to admit that this piece of code is really ugly to begin with. We
> _must not_ use memmove here as we invoke undefined behaviour, given the
> two memory regions overlap.

it is not undefined behaviour, you are thinking of memcpy
Received on Fri Dec 02 2016 - 13:24:54 CET