Ryan Freeman wrote:
> On Mon, Mar 16, 2015 at 08:00:37PM -0600, Theo de Raadt wrote:
> > If you find the right memcpy, it simply needs to be changed to memmove.
> > Then try to justify it a bit, and pass it to upstream.
>
> Thanks Theo,
>
> After eyeballing the trace again, i noticed it mentioned the exact
> expect function where the failure was, and grep was helpful to
> narrow it down to a single .c file in expect that has that function.
>
> By process of elimination I ended up with ALL the memcpy->memmove
> so now I am going to work backwards to find the exact one that causes
> the abort trap. The good news is it stopped crashing! But not sure
> which one really needed it.
>
> Will work with upstream on getting it integrated, patch is below
> if anyone doesn't mind telling me 'hey that shouldn't be memmove'.
>
> So /before/ i try and narrow it down to just one, is there a chance
> this is a case where they should just all be memmove?
>
>
> $OpenBSD$
> --- exp_inter.c.orig Mon Mar 16 20:27:50 2015
> +++ exp_inter.c Mon Mar 16 20:53:51 2015
> @@ -474,7 +474,7 @@ intRead(
> }
>
> if (cc > 0) {
> - memcpy (esPtr->input.buffer + esPtr->input.use,
> + memmove (esPtr->input.buffer + esPtr->input.use,
> Tcl_GetUnicodeFromObj (esPtr->input.newchars, NULL),
> cc * sizeof (Tcl_UniChar));
I don't know exactly what that function does, it's possible memcpy is safe
here. But there is nothing wrong with memmove if you're not sure.
The ones below obviously require memmove. the source and destination have the
same base (ustring or u->buffer).
> esPtr->input.use += cc;
> @@ -1564,7 +1564,7 @@ Exp_InteractObjCmd(
> ustring = u->input.buffer;
> if (skip) {
> size -= skip;
> - memcpy(ustring, ustring + skip, size * sizeof(Tcl_UniChar));
> + memmove(ustring, ustring + skip, size * sizeof(Tcl_UniChar));
> }
> }
> u->input.use = size;
> @@ -1824,12 +1824,12 @@ got_action:
> skip += matchLen;
> size -= skip;
> if (size) {
> - memcpy(u->buffer, u->buffer + skip, size);
> + memmove(u->buffer, u->buffer + skip, size);
> }
> } else {
> if (skip) {
> size -= skip;
> - memcpy(u->buffer, u->buffer + skip, size);
> + memmove(u->buffer, u->buffer + skip, size);
> }
> }
> Tcl_SetObjLength(size);
> @@ -2070,12 +2070,12 @@ got_action:
> skip += matchLen;
> size -= skip;
> if (size) {
> - memcpy(u->buffer, u->buffer + skip, size);
> + memmove(u->buffer, u->buffer + skip, size);
> }
> } else {
> if (skip) {
> size -= skip;
> - memcpy(u->buffer, u->buffer + skip, size);
> + memmove(u->buffer, u->buffer + skip, size);
> }
> }
> Tcl_SetObjLength(size);
>
