KSPP / linux

Linux kernel source tree (Kernel Self Protection Project)
https://kernsec.org/wiki/index.php/Kernel_Self_Protection_Project
Other
83 stars 5 forks source link

Double flex array at the end of struct #137

Closed GustavoARSilva closed 2 years ago

GustavoARSilva commented 3 years ago

There is a special case in the kernel where two flexible arrays are needed at the end of a structure, and that are intended to share the same memory layout[1]:

include/linux/filter.h:565:
struct bpf_prog {
        unsigned int            (*bpf_func)(const void *ctx,
                                            const struct bpf_insn *insn);
        ...
       struct sock_filter      insns[0];
       struct bpf_insn         insnsi[];
 };

A possible solution for this is to use a union. However, flexible arrays are not allowed as direct members in unions[2][3]. So, in order to work around this issue we can use something like the following macro formed by an anonymous union and a couple of embedded structures, each of which containing a flexible-array member together with a zero-sized struct which, in turn, serves as a mandatory object for an otherwise not allowed structure-with-flex-array-as-only-member[4][5], this is: "Flexible array members may only appear as the last member of a struct that is otherwise non-empty"[6] (yep; it's a bit tricky and complex to explain :) ):

#define flex_array_union(flex1, flex2)   \
         union { \
             struct { \
                 struct { }; \
                 flex1; \
             }; \
             struct { \
                 struct { }; \
                 flex2; \
             }; \
         }

Here are some instances that need to be addressed:

--- ./drivers/net/wireless/ath/ath10k/htt.h
+++ /tmp/nothing/drivers/net/wireless/ath/ath10k/htt.h
@@ -1675,7 +1675,6 @@ struct htt_tx_fetch_ind {
        __le16 num_resp_ids;
        __le16 num_records;
        __le32 resp_ids[0]; /* ath10k_htt_get_tx_fetch_ind_resp_ids() */
-       struct htt_tx_fetch_record records[];
 } __packed;
--- ./drivers/net/wireless/intel/iwlwifi/fw/api/tx.h
+++ /tmp/nothing/drivers/net/wireless/intel/iwlwifi/fw/api/tx.h
@@ -714,7 +714,6 @@ struct iwl_mvm_compressed_ba_notif {
        __le16 tfd_cnt;
        __le16 ra_tid_cnt;
        struct iwl_mvm_compressed_ba_ratid ra_tid[0];
-       struct iwl_mvm_compressed_ba_tfd tfd[];
 } __packed; /* COMPRESSED_BA_RES_API_S_VER_4 */
--- ./drivers/net/wireless/intel/iwlwifi/dvm/commands.h
+++ /tmp/nothing/drivers/net/wireless/intel/iwlwifi/dvm/commands.h
@@ -1252,7 +1252,6 @@ struct iwl_tx_cmd {
         * length is 26 or 30 bytes, followed by payload data
         */
        u8 payload[0];
-       struct ieee80211_hdr hdr[];
 } __packed;
--- ./drivers/net/wireless/intel/iwlegacy/commands.h
+++ /tmp/nothing/drivers/net/wireless/intel/iwlegacy/commands.h
@@ -1409,7 +1409,6 @@ struct il3945_tx_cmd {
         * length is 26 or 30 bytes, followed by payload data
         */
        u8 payload[0];
-       struct ieee80211_hdr hdr[];
 } __packed;

 /*
@@ -1511,7 +1510,6 @@ struct il_tx_cmd {
         * length is 26 or 30 bytes, followed by payload data
         */
        u8 payload[0];
-       struct ieee80211_hdr hdr[];
 } __packed;
--- ./drivers/scsi/aic94xx/aic94xx_sds.c
+++ /tmp/nothing/drivers/scsi/aic94xx/aic94xx_sds.c
@@ -518,7 +518,6 @@ struct asd_ms_conn_map {
        u8    usage_model_id;
        u32   _resvd;
        struct asd_ms_conn_desc conn_desc[0];
-       struct asd_ms_node_desc node_desc[];
 } __attribute__ ((packed));
--- ./drivers/crypto/chelsio/chcr_crypto.h
+++ /tmp/nothing/drivers/crypto/chelsio/chcr_crypto.h
@@ -223,7 +223,6 @@ struct chcr_authenc_ctx {

 struct __aead_ctx {
        struct chcr_gcm_ctx gcm[0];
-       struct chcr_authenc_ctx authenc[];
 };

 struct chcr_aead_ctx {
@@ -247,7 +246,6 @@ struct hmac_ctx {
 struct __crypto_ctx {
        struct hmac_ctx hmacctx[0];
        struct ablk_ctx ablkctx[0];
-       struct chcr_aead_ctx aeadctx[];
 };
--- ./include/scsi/sas.h
+++ /tmp/nothing/include/scsi/sas.h
@@ -324,7 +324,6 @@ struct ssp_response_iu {
        __be32 response_data_len;

        u8     resp_data[0];
-       u8     sense_data[];
 } __attribute__ ((packed));

 struct ssp_command_iu {
@@ -555,7 +554,6 @@ struct ssp_response_iu {
        __be32 response_data_len;

        u8     resp_data[0];
-       u8     sense_data[];
 } __attribute__ ((packed));

The instances above were found with the following Coccinelle script:

@@
identifier S, flex1, flex2;
type T1, T2;
@@

struct S {
  ...
  T1 flex1[0];
* T2 flex2[];
};

[1] https://git.kernel.org/linus/d26c0cc53950464a24adfa76867f1d71f0cbbea6 [2] https://godbolt.org/z/osPhqaGxo [3] https://godbolt.org/z/jnqK84bsK [4] https://godbolt.org/z/hhjjY3bx4 [5] https://godbolt.org/z/osef3eveK [6] https://gcc.gnu.org/onlinedocs/gcc/Zero-Length.html

kees commented 3 years ago

I think the wrapper needs have named structs, so we can use __UNIQUE_ID() for that:

#define flex_array_union(flex1, flex2)   \
         union { \
             struct { \
                 struct { } __UNIQUE_ID(__fau_); \
                 flex1; \
             }; \
             struct { \
                 struct { } __UNIQUE_ID(__fau_); \
                 flex2; \
             }; \
         }
kees commented 2 years ago

Commit 3080ea5553cc909b000d1f1d964a9041962f2c5b and commit fa7845cfd53f3b1d3f60efa55db89805595bc045 (v5.16)