Stack declarations for memory protection

[zephyrbot]

Reported by Andrew Boie:

We need a good method for defining stack memory regions. Right now the typical way to do it is with __stack __noinit. We may need something more generic so that we can have fine, architecture-specific control on how the stacks are declared.

For example, on x86 all the stacks are going to need to be aligned to 4096 byte boundaries, AND any given stack area should be immediately preceded by some memory which cannot be written to by the current thread. So I think we will need to position all the stacks in between rodata (which can't be written to) and the data section.

Other arches have much less in the way of constraints on the alignment/size of stacks, but the general requirement is that if the thread attempts to write RAM immediately before the stack (i.e. it overflows) we get an exception.

(Imported from Jira ZEP-2185)

[zephyrbot]

by Andrew Boie:

Andy Gross Vincenzo Frascino Kumar Gala

I have an API proposal more or less as follows, which will deprecate the existing __stack decorator:

/* arch/cpu.h may declare an architecture or platform-specific macro
 * for properly declaring stacks, compatible with MMU/MPU constraints if
 * enabled
 */
#ifdef _ARCH_STACK_DEFINE
#define K_THREAD_STACK_DEFINE(sym, size) _ARCH_THREAD_STACK_DEFINE(sym, size)
#define K_THREAD_STACK_ARRAY_DEFINE(sym, nmemb, size) \
		_ARCH_THREAD_STACK_ARRAY_DEFINE(sym, nmemb, size)
#define K_THREAD_STACK_MEMBER(sym, size) _ARCH_THREAD_STACK_MEMBER(sym, size)
#define K_THREAD_STACK_SIZEOF(sym) _ARCH_THREAD_STACK_SIZEOF(sym)
#else
/**
 * @brief Declare a toplevel thread stack memory region
 *
 * This declares a region of memory suitable for use as a thread's stack.
 *
 * This is the generic, historical definition. Align to STACK_ALIGN and put in
 * 'noinit' section so that it isn't zeroed at boot
 *
 * The declared symbol will always be a character array which can be passed to
 * k_thread_create, but should otherwise not be manipulated.
 *
 * It is legal to precede this definition with the 'static' keyword.
 *
 * It is NOT legal to take the sizeof(sym) and pass that to the stackSize
 * parameter of k_thread_create(), it may not be the same as the
 * 'size' parameter. Use K_THREAD_STACK_SIZEOF() instead.
 *
 * @param sym Thread stack symbol name
 * @param size Size of the stack memory region
 */
#define K_THREAD_STACK_DEFINE(sym, size) \
	char __noinit __aligned(STACK_ALIGN) sym[size]

/**
 * @brief Declare a toplevel array of thread stack memory regions
 *
 * Create an array of equally sized stacks. See K_THREAD_STACK_DEFINE
 * definition for additional details and constraints.
 *
 * This is the generic, historical definition. Align to STACK_ALIGN and put in
 * 'noinit' section so that it isn't zeroed at boot
 *
 * @param sym Thread stack symbol name
 * @param nmemb Number of stacks to declare
 * @param size Size of the stack memory region
 */

#define K_THREAD_STACK_ARRAY_DEFINE(sym, nmemb, size) \
	char __noinit __aligned(STACK_ALIGN) sym[nmemb][size]

/**
 * @brief Declare an embedded stack memory region
 *
 * Used for stacks embedded within other data structures. Use is highly
 * discouraged but in some cases necessary. For memory protection scenarios,
 * it is very important that any RAM preceding this member not be writable
 * by threads else a stack overflow will lead to silent corruption. In other
 * words, the containing data structure should live in RAM owned by the kernel.
 *
 * @param sym Thread stack symbol name
 * @param size Size of the stack memory region
 */
#define K_THREAD_STACK_MEMBER(sym, size) \
	char __aligned(STACK_ALIGN) sym[size]

/**
 * @brief Return the size in bytes of a stack memory region
 *
 * Convenience macro for passing the desired stack size to k_thread_create()
 * since the underlying implementation may actually create something larger
 * (for instance a guard area).
 *
 * @param sym Stack memory symbol
 * @return Size of the stack
 */
#define K_THREAD_STACK_SIZEOF(sym) sizeof(sym)

/**
 * @brief Get a pointer to the physical stack buffer
 *
 * Convenience macro to get at the real underlying stack buffer used by
 * the CPU. Guaranteed to be a character pointer of size K_THREAD_STACK_SIZEOF.
 * This is really only intended for diagnostic tools which want to examine
 * stack memory contents.
 *
 * @param sym Declared stack symbol name
 * @return The buffer itself, a char *
 */
#define K_THREAD_STACK_BUFFER(sym) sym

#endif /* _ARCH_DECLARE_STACK */

The only caveat that users will need to be aware of is that they should not take sizeof() the symbol they define and expect it to match the size passed in. They should pass the defined stack symbol to k_thread_create() but otherwise not do anything to it.

Now on x86 with MMU we have 4k pages. We can pursue some different strategies:

1. Simply add a 4K padding region to precede the real stack data, which would get marked as read-only when switching to that the owning thread.
2. Place all stacks in a special memory section. Upon context switch, mark all stack regions not owned by incoming thread as read-only. Ensure this section is preceded by a read-only or unmapped area of memory. This strategy wastes less space but may not work for applications that assume they can write into other thread's stacks directly.

For #1, the definitions could look like this, we just increase the size of the stacks by 4K:

#define STACK_ALIGN  4096

#define _ARCH_STACK_DEFINE(sym, size) \
	char __noinit __aligned(STACK_ALIGN) sym[size + STACK_ALIGN]

#define _ARCH_STACK_MEMBER(sym, size) \
	char __aligned(STACK_ALIGN) sym[size + STACK_ALIGN]

#define _ARCH_THREAD_STACK_ARRAY_DEFINE(sym, nmemb, size) \
	char __noint __aligned(STACK_ALIGN) sym[nmemb][size + STACK_ALIGN]

And then in x86's _new_thread, skip ahead the supplied stack buffer by 4096.

For #2, we'd have a new RAM section "stacks" at either the very beginning of RAM, or immediately after a different section which is always read-only. The stacks would all get put in that section. There would be no guards, but on context switch, only the incoming thread's stack will be read-write, the rest read-only.

The question is, is the above API definition workable for your needs?

[zephyrbot]

by Andrew Boie:

Pull request with proposed implementation:

#392

[zephyrbot]

by Andrew Boie:

Interface now in-tree.
Specific implementations for X86 and ARM covered in GH-3700 and GH-3626