[LU-13525] struct sepol_downcall_data is badly formed

• Status: Resolved
• Project: Lustre
• Affects Version/s: Lustre 2.14.0, Lustre 2.12.5
• Fix Version/s: Lustre 2.14.0
• Type: Bug
• Priority: Major
• Reporter: Andreas Dilger
• Assignee: Sebastien Buisson
• Resolution: Fixed
• Labels: LTS12
• Severity: 3

Description

The struct sepol_downcall_data is badly formed for use between kernel and userspace for several reasons:

struct sepol_downcall_data {
        __u32           sdd_magic;
        __kernel_time_t sdd_sepol_mtime;
        __u16           sdd_sepol_len;
        char            sdd_sepol[0];
};

Firstly, it uses a "time_t" (now "_kernel_time_t") field, which can be variably sized, either 32-bit or 64-bit depending on the size of "long" (or now "_kernel_long_t"). This should be a fixed-size __s64 type so that it is always the same.

The change from time_t to kernel_time_t in patch https://review.whamcloud.com/37678 "LU-13274 uapi: make lustre UAPI headers C99 compliant" may have broken ABI compatibility for 32-bit userspace (if a 32-bit l_getsepol is used in a VM), but was only landed as commit v2_13_52-157-g7a7309fa84 and b2_12_4-22-g0417dce9fc so has not been in a release yet, so can still be fixed before 2.14.0 and 2.12.5 are released.

Secondly, it has __u32 sdd_magic that is immediately before a (maybe) 64-bit field, which introduces a (maybe) 32-bit padding into the field, depending on the compiler and architecture. The 64-bit fields in a structure should always be naturally aligned on 64-bit boundaries to avoid potential incompatibility in the structure definition.

Thirdly, it has _u16 sdd_sepol_len which _may be followed by padding.

A better struct sepol_downcall_data that is safe for data transfer between the kernel and userspace would be:

/* old interface struct is deprecated in 2.14 */
#define SEPOL_DOWNCALL_MAGIC_OLD 0x8b8bb842
struct sepol_downcall_data_old {
        __u32           sdd_magic;
        __kernel_time_t sdd_sepol_mtime;
        __u16           sdd_sepol_len;
        char            sdd_sepol[0];
};

#define SEPOL_DOWNCALL_MAGIC 0x8b8bb843
struct sepol_downcall_data {
        __u32           sdd_magic;
        __u16           sdd_sepol_len;
        __u16           sdd_padding1;
        __s64          sdd_sepol_mtime;
        char            sdd_sepol[0];
};

We need to keep compatibility with struct sepol_downcall_data_old for some short time, because this was included in the 2.12.1 release, so lprocfs_sptlrpc_sepol_seq_write() should handle both old and new structs, with a version check for 2.16.53 or so.

Issue Links

• is related to: LU-8955 Send SELinux policy info to server
• is related to: LU-13274 Building againt lustreapi using -std=c99

Comments

[Gerrit Updater]

Sebastien Buisson ([e-mail]) uploaded a new patch: https://review.whamcloud.com/38580
Subject: LU-13525 sec: better struct sepol_downcall_data
Project: fs/lustre-release
Branch: master
Current Patch Set: 1
Commit: 9107bdbf80019781890b850ce0f4b9cd72ae94f9

[Gerrit Updater]

Oleg Drokin ([e-mail]) merged in patch https://review.whamcloud.com/38580/
Subject: LU-13525 sec: better struct sepol_downcall_data
Project: fs/lustre-release
Branch: master
Current Patch Set:
Commit: 82b8cb5528f489e9ceb7a1899722fc4108e85739

[James A Simmons]

This is done?

[James A Simmons]

The patch landed so this looks done. If not please reopen.