1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
31
32
33
34
35
36
37
38
39
40
41
42
43
44
45
46
47
48
49
50
51
52
53
54
55
56
57
58
59
60
61
62
63
64
65
66
67
68
69
|
From e1d69df0cdd1627676501df3a533b25ffadaeff0 Mon Sep 17 00:00:00 2001
From: Arkadiusz Miskiewicz <arekm@maven.pl>
Date: Sat, 27 Nov 2010 19:21:21 +0100
Subject: [PATCH] multipath-tools overflow
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 | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/libmultipath/log.c b/libmultipath/log.c
index e56e46b..57b7696 100644
--- a/libmultipath/log.c
+++ b/libmultipath/log.c
@@ -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;
--
1.7.6.5
|
