#29 buffer overflows, sighandlers (with patch)

[Dan M.]

sipp 1.0 has a tendency to allocate filename strings
using new char [] when a local char array of size
PATH_MAX would do. There are two problems with this:

1) The number of characters allocated is sometimes too
small. Witness:

/* 11 for '_screen.log' and 6 for pid */
L_file_name = new char [ strlen(scenario_file)+11+6 ] ;
sprintf (L_file_name, "%s_%d_screen.log",
scenario_file, getpid());

In fact two extra chars are required (one for '_'
between %s and %d and one for the final terminating '\0')

Result: segmentation faults and corrupted log filenames
on recent Linux versions.

2) You also use new from signal handlers, thus counting
on malloc() being reentrant (it is not, usually).

I haven't looked at sipp 1.1, but whether the bug is
fixed there or not should not matter as 1.0 is the
stable release and "should" be maintained.

Anyway, the patch is attached. If there is a patch
management system where I can submit this, please let
me know.

[Dan M.]

Patch to fix buffer overflows

[Olivier Jacques]

Logged In: YES
user_id=999021

Thanks for reporting this one.
For point 1): I will check in your fix in 1.1, which should
be "released" pretty soon.
For point 2): can you explain where you observed this?

Thanks again.

[Dan M.]

Logged In: YES
user_id=590785

The signal handler I was talking about is in sipp_sigusr2()
in sipp.cpp (sipp 1.0).

new() uses malloc() which is not async-signal safe, and so
should not be called from signal handlers. My earlier patch
might not be sufficient as sipp_sigusr2() calls other
functions that must be double-checked for async-signal
safety as well.

It's best to do as little as possible in a signal handler.
In any case, you must limit yourself to calling async-signal
safe functions. The usual recipe for a handler is to post a
semaphore with sem_post() (which is async-signal safe) and
return immediately; you can have another thread wait for the
semaphore and do the actual work. Threads are subject to
fewer restrictions, as many functions are thread-safe but
not re-entrant / async-signal safe.

If you break the rules, the program might only crash
sporadically. When the signal happens to interrupt malloc(),
the heap might be corrupted. The crash might occur
immediately or sometime later.

[Olivier Boulkroune]

Logged In: YES
user_id=1475960
Originator: NO

Too old