clang will misalign capabilities in packed structs

[brooksdavis]

The following code fragment demonstrates that clang will misalign capabilities in the pure-capability ABI if they appear in packed structs.

I believe packed should do one of three possible things in this case:

• generate the struct with appropriate padding, possibly emitting a warning about the padding
• fail due to inability to both align the capability and pack the struct
• fail due to attempting packing a struct containing a capability

struct packed_with_pointer {
        char c;
        void *p;
} __attribute__ ((packed));

#ifdef __CHERI_PURE_CAPABILITY__
_Static_assert(sizeof(struct packed_with_pointer) == 2 * sizeof(void *),
    "CHERI pointers must be aligned even when packed");
#else
_Static_assert(sizeof(struct packed_with_pointer) ==
    sizeof(char) + sizeof(void *),
    "Packed struct should be minimally sized");
#endif

[brooksdavis]

I'm not actually encountering a problem, but I was thinking about packed structs in the context of the constraints on objects in arrays sorted by qsort et al and wrote this test to see what was currently happening.

[bsdjhb]

__packed generally breaks alignment and doesn't insert padding for other cases. For example:

struct packed_with_sse {
        char c;
        __mm_uint128_t vec;
} __packed;

would give you an unaligned 'vec' member that could not be used with SSE instructions requiring alignment. In the kernel you can get yourself into trouble with __packed in other ways, e.g.:

struct foo {
      char val;
      struct mtx lock;
} __packed;

Here the packing misaligns the lock cookie inside of the mutex triggering an assertion failure at runtime in mtx_init(). In general I think C programmers are aware that packed must be used with care. I think having packed treat capabilities the same as anything else and misalign them is actually the most consistent behavior.

[davidchisnall]

I agree with brooks that this should be a warning. The difference between this and a vector is that we're likely to be dealing with legacy code that has packed structs assuming either 4- or 8-byte pointers. For example:

struct foo 
{
   int f;
# ifdef __POINTERS_ARE_8_BYTES__
   char padding[4];
# endif
   void *a;
} __packed;

This will help us catch such code.

[bsdjhb]

A warning I'd be ok with. Adding padding I think would be very inconsistent with how __packed otherwise works.

[bsdjhb]

Honestly, in terms of a warning, I'd like to be able to have something like:

struct mutex {
    ...
} __attribute__((__desired_alignment__(16)__));

And then have a new warning that complains if an instance was known to be misaligned due to packing, etc. You could then apply this implicitly to capabilities, but it would be nice if I as a user could tag objects in such a way to get a compile warning. (In particular I would probably tag mtx_lock in struct mtx rather than the whole thing.)

[brooksdavis]

I'm fine with a warning, particularly since we could make it fatal by default with -Werror-misaligning-strongly-aligned-object or the like in the build system.

Some sort of __attribute__((__desired_alignment__(16)__)) seems good. I'd be tempted to have a required_alignment variant which produces an error. This is also something some cryptographic or side car pointer protections schemes probably need.

[davidchisnall]

It appears that clang is now simply ignoring the packed attribute on structs that contain capabilities, which is probably a bug. It's particularly nasty bug, because it doesn't reject packed structs containing non-packed structs that contain pointers.