madvise needs to page-alligned memory pointer

[user-none]

According to the man page (http://man7.org/linux/man-pages/man2/madvise.2.html) for madvise under "Linux notes"

The Linux implementation requires that the address addr be page-aligned...

In utils/s2n_mem.c madvise is called:

madvise(b->data, size, MADV_DONTDUMP)

(b->)data is created using realloc, witch does not guarantee page alignment.

Further the man page says the following errno will be returned in the following situation:

EINVAL addr is not page-aligned or length is negative.

The current implementation can fail on Linux.

[user-none]

I can't create a test case for this because the madvise implementation doesn't work.

1. #ifdef MADV_DONTDUMP is showing that MADV_DONTDUMP is undefined so the madvise section is never included in the build. Put a printf after the ifdef and run any test case. You'll never see it printed.
2. S2N_ERR_MADVISE isn't declared so even if the madvise section would be built the build would fail.

[baldwinmatt]

Thanks for the report. regarding MADV_DONTDUMP:

MADV_DONTDUMP (since Linux 3.4)
Exclude from a core dump those pages in the range specified by
addr and length. This is useful in applications that have
large areas of memory that are known not to be useful in a
core dump. The effect of MADV_DONTDUMP takes precedence over
the bit mask that is set via the /proc/PID/coredump_filter
file (see core(5)).

[user-none]

@baldwinmatt
That is the description from the man page but that doesn't negate the fact that the man page also states that the addr must be page-aligned on Linux otherwise it will fail and set errno to EINVAL. Nothing is being done to ensure the memory is page-aligned. So if it was working, without modifications to the memory allocation it would still fail.

[user-none]

I should add that setting MADV_DONTDUMP on all memory can cause the application to run out of address space due to the page-alignment requirement of madvise(). This is especially possible on a 32 bit system.

$ getconf PAGESIZE
4096

Each allocation must be aligned to a 4096 boundary. Meaning the address of the allocated data must be the address of a page boundary. There is 4K between each boundary. A large amount of data can be allocated there but if a small amount of data is allocated then there is a large amount of unusable space due to the next allocation needing to also be on a 4K boundary.

Take the following allocations:

1. 8 bytes page-aligned.
2. 4 bytes page-aligned.

Assuming One and Two are allocated next to each other. One allocates 8 bytes. Two will be aligned to the 4K boundary after One. A total of 8K of memory is reserved due to this. Only 12 bytes are actually needed but 8K is reserved. Since memory is now aligned in 4K blocks the total available memory space is greatly reduced. Not the amount of memory but the amount of allocations.

On a 32bit system only ~2GB of memory is available to a process. With 4K page-alignment allocations the amount usable memory is greatly reduced. This might be okay on a 64 bit system but will still be wasteful.

[user-none]

Example demonstrating the issue https://gist.github.com/user-none/48160df9128b170e7d07

1. Using non page-aligned memory you get a failure.
2. Using page-aligned memory you get success but memory pointers are separated by 4K bytes (page-alignment on my system) blocks. Everything within the 4K block, between the end of the requested allocation and the end of the block, is unusable.
3. Not using page-alignment and not using madvise succeeds with only 32 bytes between each pointer.

Valgrind on 2 and 3 shows 80,000 bytes in 10,000 blocks. However, the size of the block is what matters. 4K byte blocks will run out faster than 32 byte blocks. On an embedded system (phone with 512 MB memory or router with 16 - 64 MB of memory) address space for allocations will be depleted very quickly.

[raycoll]

#156 and #200 should fix this. However, we still do a page aligned allocation when "S2N_DONT_MLOCK" is set. It might be worth falling back to normal malloc if the user doesn't want madvise/mlock