iwlwifi: pcie: make cfg vs. trans_cfg more robust
authorJohannes Berg <johannes.berg@intel.com>
Tue, 30 Mar 2021 13:24:58 +0000 (16:24 +0300)
committerLuca Coelho <luciano.coelho@intel.com>
Wed, 14 Apr 2021 09:07:16 +0000 (12:07 +0300)
If we (for example) have a trans_cfg entry in the PCI IDs table,
but then don't find a full cfg entry for it in the info table,
we fall through to the code that treats the PCI ID table entry
as a full cfg entry. This obviously causes crashes later, e.g.
when trying to build the firmware name string.

Avoid such crashes by using the low bit of the pointer as a tag
for trans_cfg entries (automatically using a macro that checks
the type when assigning) and then checking that before trying to
use the data as a full entry - if it's just a partial entry at
that point, fail.

Since we're adding some macro magic, also check that the type is
in fact either struct iwl_cfg_trans_params or struct iwl_cfg,
failing compilation ("initializer element is not constant") if
it isn't.

Signed-off-by: Johannes Berg <johannes.berg@intel.com>
Signed-off-by: Luca Coelho <luciano.coelho@intel.com>
Link: https://lore.kernel.org/r/iwlwifi.20210330162204.6f69fe6e4128.I921d4ae20ef5276716baeeeda0b001cf25b9b968@changeid
Signed-off-by: Luca Coelho <luciano.coelho@intel.com>
drivers/net/wireless/intel/iwlwifi/pcie/drv.c

index 7ae0892..d7c4625 100644 (file)
 #include "iwl-prph.h"
 #include "internal.h"
 
+#define TRANS_CFG_MARKER BIT(0)
+#define _IS_A(cfg, _struct) __builtin_types_compatible_p(typeof(cfg),  \
+                                                        struct _struct)
+extern int _invalid_type;
+#define _TRANS_CFG_MARKER(cfg)                                         \
+       (__builtin_choose_expr(_IS_A(cfg, iwl_cfg_trans_params),        \
+                              TRANS_CFG_MARKER,                        \
+        __builtin_choose_expr(_IS_A(cfg, iwl_cfg), 0, _invalid_type)))
+#define _ASSIGN_CFG(cfg) (_TRANS_CFG_MARKER(cfg) + (kernel_ulong_t)&(cfg))
+
 #define IWL_PCI_DEVICE(dev, subdev, cfg) \
        .vendor = PCI_VENDOR_ID_INTEL,  .device = (dev), \
        .subvendor = PCI_ANY_ID, .subdevice = (subdev), \
-       .driver_data = (kernel_ulong_t)&(cfg)
+       .driver_data = _ASSIGN_CFG(cfg)
 
 /* Hardware specific file defines the PCI IDs table for that hardware module */
 static const struct pci_device_id iwl_hw_card_ids[] = {
@@ -1099,19 +1109,22 @@ static const struct iwl_dev_info iwl_dev_info_table[] = {
 
 static int iwl_pci_probe(struct pci_dev *pdev, const struct pci_device_id *ent)
 {
-       const struct iwl_cfg_trans_params *trans =
-               (struct iwl_cfg_trans_params *)(ent->driver_data);
+       const struct iwl_cfg_trans_params *trans;
        const struct iwl_cfg *cfg_7265d __maybe_unused = NULL;
        struct iwl_trans *iwl_trans;
        struct iwl_trans_pcie *trans_pcie;
        int i, ret;
+       const struct iwl_cfg *cfg;
+
+       trans = (void *)(ent->driver_data & ~TRANS_CFG_MARKER);
+
        /*
         * This is needed for backwards compatibility with the old
         * tables, so we don't need to change all the config structs
         * at the same time.  The cfg is used to compare with the old
         * full cfg structs.
         */
-       const struct iwl_cfg *cfg = (struct iwl_cfg *)(ent->driver_data);
+       cfg = (void *)(ent->driver_data & ~TRANS_CFG_MARKER);
 
        /* make sure trans is the first element in iwl_cfg */
        BUILD_BUG_ON(offsetof(struct iwl_cfg, trans));
@@ -1226,11 +1239,19 @@ static int iwl_pci_probe(struct pci_dev *pdev, const struct pci_device_id *ent)
 
 #endif
        /*
-        * If we didn't set the cfg yet, assume the trans is actually
-        * a full cfg from the old tables.
+        * If we didn't set the cfg yet, the PCI ID table entry should have
+        * been a full config - if yes, use it, otherwise fail.
         */
-       if (!iwl_trans->cfg)
+       if (!iwl_trans->cfg) {
+               if (ent->driver_data & TRANS_CFG_MARKER) {
+                       pr_err("No config found for PCI dev %04x/%04x, rev=0x%x, rfid=0x%x\n",
+                              pdev->device, pdev->subsystem_device,
+                              iwl_trans->hw_rev, iwl_trans->hw_rf_id);
+                       ret = -EINVAL;
+                       goto out_free_trans;
+               }
                iwl_trans->cfg = cfg;
+       }
 
        /* if we don't have a name yet, copy name from the old cfg */
        if (!iwl_trans->name)