Hi,

To Albert Cahalan, I am answering here since that's me that have
introduced the memset patch.

Lundi 23 novembre 2009, vers 10:56:24 (+0100), Albert Cahalan a écrit :

> I don't have this "supgid" in my procps, so I don't have this bug.

The "memset patch" actually addresses two issues.

First, and more importantly, it fixes the "double free" bug introduced
by the Debian "supgid patch".  That's why I started to look at the ps
code.  I wrote to the Debian BTS, since I knew that "supgid" was not in
the original code.

But it also fixes other subtle problems that I can see in the original
code.  Without the patch, and under the same race condition
(/proc/#/status or /proc/#/statm disappearing just before reading them),
some fields of the proc_t struct are never touched.  As far as I can
see, they just keep the values they got for the preceding process, since
the same buffer is used between several calls of readproc.

With the patch, those fields are initialized to zero.  The values are
surely wrong too, but IMHO much less confusing.

Note that the comment /* statm fields just zero */ in simple_readproc() 
seems to be wrong.  I am unable to find where those fields are zeroed.

The only difference between the two cases is that it is a pointer that
it is not correctly zeroed in the first case, making the problem
evident by the crash it leads to, whereas they are just integers in the
second case, that are just used without any problem.


> Remember that this is some of the most performance-critical code in
> all of procps. Adding a memset is not good. Even the xcalloc isn't
> such a good idea; it really should be a plain malloc.

I chose to use a single memset call instead of enumerating all the
required fields, firstly because it was much less error prone.
Secondly, it made the patch less intruding.

I have tried to compare the performance of ps, with or without the
"memset".  Tests were done with "time ps > /dev/null" and the original
source code downloaded from SourceForge, on a 3GHz Pentium 4, and on a
slower 200MHz Mips R5000.

Even with thousands of processes I am unable to see any significant
performance hit.

Regards,

        Arnaud Giersch




--
To UNSUBSCRIBE, email to debian-bugs-dist-requ...@lists.debian.org
with a subject of "unsubscribe". Trouble? Contact listmas...@lists.debian.org

Reply via email to