port randstruct to Clang

[kees]

Provide __attribute__(randomize_layout) for structure layout randomization in Clang.

• [x] Initial core Clang support: landed
• [x] Adapt kernel to use Clang feature: proposed
• [x] Fix "missing member" warning: landed
  • [x] Keep flexible array members always at the end
• [x] Enable automatic randomization of function tables: landed
• [x] Fix any remaining kernel build warnings
  • [x] decnet: proposed
  • [x] gfs2: proposed
  • [x] niu: proposed

Nice-to-have:

• [ ] Add "cache-line only" mode

[kees]

It looks like the feature is unhappy about unions in a randomized structure:

../arch/x86/include/asm/cpufeature.h:194:54: error: no member named 'x86_capability' in 'struct cpuinfo_x86'; did you mean 'vmx_capability'?                                                            
                     [cap_byte] "i" (&((const char *)boot_cpu_data.x86_capability)[bit >> 3])       
                                                                   ^~~~~~~~~~~~~~                   
                                                                   vmx_capability                   

[kees]

Proposed fix for above: https://reviews.llvm.org/D123958

[kees]

Proposed fix for above: https://reviews.llvm.org/D123958

This seems to break the boot, with proc things failing. Disabling __randomize_layout for proc_internal.h solves it. Looking deeper... ah, I suspect the flex array got moved. Flex arrays should always stay at the end.

[kees]

struct thing {
        int a;
        int b;
        int c;
        int d;
        int e;
        int f[];
} __attribute__((randomize_layout));

$ ./main 
offsetof(struct thing, a): 4 (val 10)
offsetof(struct thing, b): 0 (val 20)
offsetof(struct thing, c): 12 (val 30)
offsetof(struct thing, d): 16 (val 40)
offsetof(struct thing, e): 8 (val 50)
offsetof(struct thing, f): 8

Looks like f is both not at the end and aliased with the prior member.

[kees]

Bootable:

diff --git a/fs/proc/internal.h b/fs/proc/internal.h
index 06a80f78433d..50582b0a4040 100644
--- a/fs/proc/internal.h
+++ b/fs/proc/internal.h
@@ -64,7 +64,7 @@ struct proc_dir_entry {
        u8 flags;
        u8 namelen;
        char inline_name[];
-} __randomize_layout;
+}; //__randomize_layout;

 #define SIZEOF_PDE     (                               \
        sizeof(struct proc_dir_entry) < 128 ? 128 :     \
diff --git a/include/linux/cred.h b/include/linux/cred.h
index 9ed9232af934..7f5d04b0bc56 100644
--- a/include/linux/cred.h
+++ b/include/linux/cred.h
@@ -26,7 +26,7 @@ struct group_info {
        atomic_t        usage;
        int             ngroups;
        kgid_t          gid[];
-} __randomize_layout;
+}; //__randomize_layout;

 /**
  * get_group_info - Get a reference to a group info structure
diff --git a/ipc/sem.c b/ipc/sem.c
index 0dbdb98fdf2d..a227c25ca058 100644
--- a/ipc/sem.c
+++ b/ipc/sem.c
@@ -124,7 +124,7 @@ struct sem_array {
        unsigned int            use_global_lock;/* >0: global lock required */

        struct sem              sems[];
-} __randomize_layout;
+}; // __randomize_layout;

 /* One queue for each sleeping process in the system. */
 struct sem_queue {

[kees]

Here's the "past the end of the struct" warning I was seeing earlier:

    In file included from ../fs/nfs/inode.c:50:
    In file included from ../fs/nfs/fscache.h:15:
    In file included from ../include/linux/fscache.h:18:
    ../include/linux/netfs.h:298:9: error: casting from randomized structure
    pointer type 'struct inode *' to 'struct netfs_i_context *'
            return (struct netfs_i_context *)(inode + 1);
                   ^
    1 error generated.

Casting this to void * makes it invisible to Clang randstruct, but isn't a particularly friendly solution...

[bwendling]

In this code, they're accessing excess memory at the end of the struct, right? Why shouldn't they use i_private instead? They could even set i_private to inode + 1, or equivalent. Do you know where this is allocated?

[kees]

For ceph, I see it getting handled in the superblock ops:

static const struct super_operations ceph_super_ops = {
        .alloc_inode    = ceph_alloc_inode,

Where I see the netfs context getting initialized...

struct inode *ceph_alloc_inode(struct super_block *sb)
{
        struct ceph_inode_info *ci;
        int i;

        ci = alloc_inode_sb(sb, ceph_inode_cachep, GFP_NOFS);
        if (!ci)
                return NULL;

        dout("alloc_inode %p\n", &ci->vfs_inode);

        /* Set parameters for the netfs library */
        netfs_i_context_init(&ci->vfs_inode, &ceph_netfs_ops);

Which comes from a statically-sized kmem cache:

static int __init init_caches(void)
{
        int error = -ENOMEM;

        ceph_inode_cachep = kmem_cache_create("ceph_inode_info",
                                      sizeof(struct ceph_inode_info),
                                      __alignof__(struct ceph_inode_info),
                                      SLAB_RECLAIM_ACCOUNT|SLAB_MEM_SPREAD|
                                      SLAB_ACCOUNT, ceph_inode_init_once);

[kees]

It's really weird. Everything else in the kernel will normally use container_of but netfs doesn't know what the containing type is. It's possible struct netfs_i_context could be defined as having struct inode within it. The users could use a struct_group(). Hmm...

[kees]

I think the simplest fix is the void * cast. I'll send the kernel series out.

[bwendling]

Okay, thanks.

[bwendling]

BTW, I created this macro to make the reason for the cast more apparent.

diff --git a/include/linux/compiler_types.h b/include/linux/compiler_types.h
index 1c2c33ae1b37..c58a3a638e25 100644
--- a/include/linux/compiler_types.h
+++ b/include/linux/compiler_types.h
@@ -378,4 +378,8 @@ struct ftrace_likely_data {
 #define __diag_ignore_all(option, comment)
 #endif

+#ifndef randstruct_cast
+#define randstruct_cast(type, x)       (type)(void *)(x)
+#endif
+
 #endif /* __LINUX_COMPILER_TYPES_H */

[kees]

Here are more from a wider selection of architecture allmodconfig builds:

drivers/net/ethernet/sun/niu.c:3385:12: error: casting from randomized structure pointer type 'struct address_space *' to 'struct page *'
drivers/net/ethernet/sun/niu.c:3454:12: error: casting from randomized structure pointer type 'struct address_space *' to 'struct page *'
drivers/net/ethernet/sun/niu.c:3521:24: error: casting from randomized structure pointer type 'struct address_space *' to 'struct page *'
drivers/net/ethernet/sun/niu.c:6444:7: error: casting from randomized structure pointer type 'struct address_space *' to 'struct page *'
fs/gfs2/meta_io.h:44:12: error: casting from randomized structure pointer type 'struct address_space *' to 'struct gfs2_glock *'
net/decnet/dn_dev.c:857:25: error: casting from randomized structure pointer type 'struct neighbour *' to 'struct dn_neigh *'
net/decnet/dn_dev.c:905:24: error: casting from randomized structure pointer type 'struct neighbour *' to 'struct dn_neigh *'
net/decnet/dn_neigh.c:429:26: error: casting from randomized structure pointer type 'struct neighbour *' to 'struct dn_neigh *'
net/decnet/dn_route.c:1123:15: error: casting from randomized structure pointer type 'struct neighbour *' to 'struct dn_neigh *'
net/decnet/dn_route.c:1432:15: error: casting from randomized structure pointer type 'struct neighbour *' to 'struct dn_neigh *'

[kees]

decnet: https://lore.kernel.org/netdev/20220508102217.2647184-1-keescook@chromium.org/#t gfs2: https://lore.kernel.org/lkml/20220508100630.2642320-1-keescook@chromium.org/#t

[kees]

niu: https://lore.kernel.org/linux-hardening/20220510205729.3574400-1-keescook@chromium.org/#t

[kees]

All the kernel patches are in -next.

[bwendling]

Could you write up an RFC for the "cache-only" flag?

[ebiggers]

Note, randstruct with clang is currently unusable due to https://github.com/llvm/llvm-project/issues/60349.

[ebiggers]

Note, randstruct with clang is currently unusable due to llvm/llvm-project#60349.

Fixed in clang 16.