Re: [PATCH] Add MALLOC_CHECK_ and MALLOC_PERTURB_ libc env to the test suite for detecting heap corruption

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Elia Pinto <gitter.spiros@xxxxxxxxx> writes:

> Recent versions of Linux libc (later than 5.4.23) and glibc (2.x)
> include a malloc() implementation which is tunable via environment
> variables. When MALLOC_CHECK_ is set, a special (less efficient)
> implementation is used which is designed to be tolerant against
> simple errors, such as double calls of free() with the same argument,
> or overruns of a single byte (off-by-one bugs). When MALLOC_CHECK_
> is set to 3, a diagnostic message is printed on stderr
> and the program is aborted.
> ...
> Signed-off-by: Elia Pinto <gitter.spiros@xxxxxxxxx>
> ---
> This the third reroll of the original patch.

Well written.  I have one suggestion and a question, though.

>  t/test-lib.sh |    9 +++++++++
>  1 file changed, 9 insertions(+)
>
> diff --git a/t/test-lib.sh b/t/test-lib.sh
> index 78c4286..f34b861 100644
> --- a/t/test-lib.sh
> +++ b/t/test-lib.sh
> @@ -93,6 +93,15 @@ export GIT_AUTHOR_EMAIL GIT_AUTHOR_NAME
>  export GIT_COMMITTER_EMAIL GIT_COMMITTER_NAME
>  export EDITOR
>  
> +# Add libc MALLOC and MALLOC_PERTURB test 
> +# only if we are not executing the test with valgrind
> +expr "$GIT_TEST_OPTS" : ".*\(--valgrind\)" >/dev/null || {

I would write this like

	if ! expr " $GIT_TEST_OPTS " : ".* --valgrind " >/dev/null
	then
		...
	fi

> +	MALLOC_CHECK_=3
> +	export MALLOC_CHECK_
> +	MALLOC_PERTURB_="$( expr \( $$ % 255 \) + 1)"

How was this expression chosen?  The only effect I can think of to
use a randomly picked value here is to make it impossible to make
the test repeatable and reproducible, so you must have had some
benefit that outweighs the downside, but I cannot think of any.
Wouldn't MALLOC_PERTURB_=165 (i.e. 0xA5, half of the bits in the
byte is set, including the MSB, and is an odd number) be equally a
good choice without repeatability downside, for example?

Also, doesn't the above give 256 sometimes, which would not fit in a
byte?

> +	export MALLOC_PERTURB_
> +}
> +
>  # Protect ourselves from common misconfiguration to export
>  # CDPATH into the environment
>  unset CDPATH
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]