multipath-tools overflow
authorArkadiusz Miskiewicz <arekm@maven.pl>
Sat, 27 Nov 2010 18:21:21 +0000 (19:21 +0100)
committerChristophe Varoqui <christophe.varoqui@opensvc.com>
Sat, 27 Nov 2010 19:44:05 +0000 (20:44 +0100)
On Saturday 27 of November 2010, you wrote:

[...]

> the whole logarea is memset to 0 by logarea_init(), and each dequeued
> message is also memset to 0 by log_dequeue(), so it seems normal that
> msg->str value is 0x0, but it's really its address that matters.

Ok, got it. Pointers, memory areas in my debugging session - are looking
good then.

>
> It's not clear to me : are you actually hitting a bug or is it your
> debug session that puzzles you ?

I'm hitting a bug. multipathd dies for me at that strcpy(). Now I think
the bug is strcpy usage instead of memcpy because I'm building with
-O2 -D_FORTIFY_SOURCE=2 which turns on special glibc overflow
detection.

That detection seem to be smart enough to know that &str area is not
a string memory and aborts the program.

Found similar problem discussed here
http://sourceware.org/ml/binutils/2005-11/msg00308.html

glibc aborts the program:
[pid 13432] writev(2, [{"*** ", 4}, {"buffer overflow detected", 24},
{" ***: ", 6}, {"/home/users/arekm/rpm/BUILD/multipath-tools-0.4.9
/multipathd/multipathd", 71}, {" terminated\n", 12}], 5) = 117

same for valgrind:
**13436** *** strcpy_chk: buffer overflow detected ***: program terminated
==13436==    at 0x4024997: VALGRIND_PRINTF_BACKTRACE (valgrind.h:4477)
==13436==    by 0x40265F8: __strcpy_chk (mc_replace_strmem.c:781)
==13436==    by 0x40EDC06: log_enqueue (string3.h:107)
==13436==    by 0x40ED68A: log_safe (log_pthread.c:24)
==13436==    by 0x40E296A: dlog (debug.c:36)
==13436==    by 0x804ECEC: pidfile_create (pidfile.c:37)
==13436==    by 0x804E731: main (main.c:1424)

The bug is not visible if I run multipathd in debug mode (-d).

This patch fixes the problem for me by avoiding false positive on strcpy_chk.

libmultipath/log.c

index e56e46b..57b7696 100644 (file)
@@ -142,7 +142,7 @@ int log_enqueue (int prio, const char * fmt, va_list ap)
        la->empty = 0;
        msg = (struct logmsg *)la->tail;
        msg->prio = prio;
-       strcpy((void *)&msg->str, buff);
+       memcpy((void *)&msg->str, buff, strlen(buff) + 1);
        lastmsg->next = la->tail;
        msg->next = la->head;