PCI: hotplug: Constify hotplug_slot_ops

Hotplug drivers cannot declare their hotplug_slot_ops const, making them
attractive targets for attackers, because upon registration of a hotplug
slot, __pci_hp_initialize() writes to the "owner" and "mod_name" members
in that struct.

Fix by moving these members to struct hotplug_slot and constify every
driver's hotplug_slot_ops except for pciehp.

pciehp constructs its hotplug_slot_ops at runtime based on the PCIe
port's capabilities, hence cannot declare them const.  It can be
converted to __write_rarely once that's mainlined:
http://www.openwall.com/lists/kernel-hardening/2016/11/16/3

Signed-off-by: Lukas Wunner <lukas@wunner.de>
Signed-off-by: Bjorn Helgaas <bhelgaas@google.com>
Reviewed-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
Acked-by: Tyrel Datwyler <tyreld@linux.vnet.ibm.com>  # drivers/pci/hotplug/rpa*
Acked-by: Andy Shevchenko <andy.shevchenko@gmail.com> # drivers/platform/x86
Cc: Len Brown <lenb@kernel.org>
Cc: Scott Murray <scott@spiteful.org>
Cc: Benjamin Herrenschmidt <benh@kernel.crashing.org>
Cc: Paul Mackerras <paulus@samba.org>
Cc: Michael Ellerman <mpe@ellerman.id.au>
Cc: Oliver OHalloran <oliveroh@au1.ibm.com>
Cc: Gavin Shan <gwshan@linux.vnet.ibm.com>
Cc: Sebastian Ott <sebott@linux.vnet.ibm.com>
Cc: Gerald Schaefer <gerald.schaefer@de.ibm.com>
Cc: Corentin Chary <corentin.chary@gmail.com>
Cc: Darren Hart <dvhart@infradead.org>

diff --git a/drivers/pci/hotplug/acpiphp_core.c b/drivers/pci/hotplug/acpiphp_core.c
index ad32ffbc4b91900cf15c7533dfcea81694bf6ddf..e883cef0f3bca7928535b9900fa92463c1cc3bd8 100644 (file)
--- a/drivers/pci/hotplug/acpiphp_core.c
+++ b/drivers/pci/hotplug/acpiphp_core.c
@@ -57,7 +57,7 @@ static int get_attention_status(struct hotplug_slot *slot, u8 *value);
 static int get_latch_status(struct hotplug_slot *slot, u8 *value);
 static int get_adapter_status(struct hotplug_slot *slot, u8 *value);
 
-static struct hotplug_slot_ops acpi_hotplug_slot_ops = {
+static const struct hotplug_slot_ops acpi_hotplug_slot_ops = {
        .enable_slot            = enable_slot,
        .disable_slot           = disable_slot,
        .set_attention_status   = set_attention_status,

diff --git a/drivers/pci/hotplug/cpci_hotplug_core.c b/drivers/pci/hotplug/cpci_hotplug_core.c
index 52a339baf06c03913ceb7993b4cf3a7eb008539e..97c32e4c74c869305b1b27510ec69c0e445d2b98 100644 (file)
--- a/drivers/pci/hotplug/cpci_hotplug_core.c
+++ b/drivers/pci/hotplug/cpci_hotplug_core.c
@@ -57,7 +57,7 @@ static int get_attention_status(struct hotplug_slot *slot, u8 *value);
 static int get_adapter_status(struct hotplug_slot *slot, u8 *value);
 static int get_latch_status(struct hotplug_slot *slot, u8 *value);
 
-static struct hotplug_slot_ops cpci_hotplug_slot_ops = {
+static const struct hotplug_slot_ops cpci_hotplug_slot_ops = {
        .enable_slot = enable_slot,
        .disable_slot = disable_slot,
        .set_attention_status = set_attention_status,

diff --git a/drivers/pci/hotplug/cpqphp_core.c b/drivers/pci/hotplug/cpqphp_core.c
index 5a06636e910ad4e359cbd79d37e5f6b5bb268709..3409b62fceacee9e000ef6515e30108f4adb0dfb 100644 (file)
--- a/drivers/pci/hotplug/cpqphp_core.c
+++ b/drivers/pci/hotplug/cpqphp_core.c
@@ -560,7 +560,7 @@ static int get_adapter_status(struct hotplug_slot *hotplug_slot, u8 *value)
        return 0;
 }
 
-static struct hotplug_slot_ops cpqphp_hotplug_slot_ops = {
+static const struct hotplug_slot_ops cpqphp_hotplug_slot_ops = {
        .set_attention_status = set_attention_status,
        .enable_slot =          process_SI,
        .disable_slot =         process_SS,

diff --git a/drivers/pci/hotplug/ibmphp.h b/drivers/pci/hotplug/ibmphp.h
index fddb78606c7446c7eaaf6fb065b4b1ab84653601..db387e10581e648570e204b98745b70a186832d4 100644 (file)
--- a/drivers/pci/hotplug/ibmphp.h
+++ b/drivers/pci/hotplug/ibmphp.h
@@ -740,7 +740,7 @@ int ibmphp_do_disable_slot(struct slot *slot_cur);
 int ibmphp_update_slot_info(struct slot *);    /* This function is called from HPC, so we need it to not be be static */
 int ibmphp_configure_card(struct pci_func *, u8);
 int ibmphp_unconfigure_card(struct slot **, int);
-extern struct hotplug_slot_ops ibmphp_hotplug_slot_ops;
+extern const struct hotplug_slot_ops ibmphp_hotplug_slot_ops;
 
 #endif                         //__IBMPHP_H
 

diff --git a/drivers/pci/hotplug/ibmphp_core.c b/drivers/pci/hotplug/ibmphp_core.c
index 4ea57e9019f16b5876b0b51d89a50f734e3a7e1b..b82fdc17040d434d264dda9eef1d6bd240b3ba41 100644 (file)
--- a/drivers/pci/hotplug/ibmphp_core.c
+++ b/drivers/pci/hotplug/ibmphp_core.c
@@ -1259,7 +1259,7 @@ error:
        goto exit;
 }
 
-struct hotplug_slot_ops ibmphp_hotplug_slot_ops = {
+const struct hotplug_slot_ops ibmphp_hotplug_slot_ops = {
        .set_attention_status =         set_attention_status,
        .enable_slot =                  enable_slot,
        .disable_slot =                 ibmphp_disable_slot,

diff --git a/drivers/pci/hotplug/pci_hotplug_core.c b/drivers/pci/hotplug/pci_hotplug_core.c
index 90fde5f106d833fb3ff063cb20bf48a29e0d903e..ede2ed6f4ce0c8a694ecb4b00dbf096f3cbe1634 100644 (file)
--- a/drivers/pci/hotplug/pci_hotplug_core.c
+++ b/drivers/pci/hotplug/pci_hotplug_core.c
@@ -49,15 +49,15 @@ static DEFINE_MUTEX(pci_hp_mutex);
 #define GET_STATUS(name, type) \
 static int get_##name(struct hotplug_slot *slot, type *value)          \
 {                                                                      \
-       struct hotplug_slot_ops *ops = slot->ops;                       \
+       const struct hotplug_slot_ops *ops = slot->ops;                 \
        int retval = 0;                                                 \
-       if (!try_module_get(ops->owner))                                \
+       if (!try_module_get(slot->owner))                               \
                return -ENODEV;                                         \
        if (ops->get_##name)                                            \
                retval = ops->get_##name(slot, value);                  \
        else                                                            \
                *value = slot->info->name;                              \
-       module_put(ops->owner);                                         \
+       module_put(slot->owner);                                        \
        return retval;                                                  \
 }
 
@@ -90,7 +90,7 @@ static ssize_t power_write_file(struct pci_slot *pci_slot, const char *buf,
        power = (u8)(lpower & 0xff);
        dbg("power = %d\n", power);
 
-       if (!try_module_get(slot->ops->owner)) {
+       if (!try_module_get(slot->owner)) {
                retval = -ENODEV;
                goto exit;
        }
@@ -109,7 +109,7 @@ static ssize_t power_write_file(struct pci_slot *pci_slot, const char *buf,
                err("Illegal value specified for power\n");
                retval = -EINVAL;
        }
-       module_put(slot->ops->owner);
+       module_put(slot->owner);
 
 exit:
        if (retval)
@@ -138,7 +138,8 @@ static ssize_t attention_read_file(struct pci_slot *pci_slot, char *buf)
 static ssize_t attention_write_file(struct pci_slot *pci_slot, const char *buf,
                                    size_t count)
 {
-       struct hotplug_slot_ops *ops = pci_slot->hotplug->ops;
+       struct hotplug_slot *slot = pci_slot->hotplug;
+       const struct hotplug_slot_ops *ops = slot->ops;
        unsigned long lattention;
        u8 attention;
        int retval = 0;
@@ -147,13 +148,13 @@ static ssize_t attention_write_file(struct pci_slot *pci_slot, const char *buf,
        attention = (u8)(lattention & 0xff);
        dbg(" - attention = %d\n", attention);
 
-       if (!try_module_get(ops->owner)) {
+       if (!try_module_get(slot->owner)) {
                retval = -ENODEV;
                goto exit;
        }
        if (ops->set_attention_status)
-               retval = ops->set_attention_status(pci_slot->hotplug, attention);
-       module_put(ops->owner);
+               retval = ops->set_attention_status(slot, attention);
+       module_put(slot->owner);
 
 exit:
        if (retval)
@@ -213,13 +214,13 @@ static ssize_t test_write_file(struct pci_slot *pci_slot, const char *buf,
        test = (u32)(ltest & 0xffffffff);
        dbg("test = %d\n", test);
 
-       if (!try_module_get(slot->ops->owner)) {
+       if (!try_module_get(slot->owner)) {
                retval = -ENODEV;
                goto exit;
        }
        if (slot->ops->hardware_test)
                retval = slot->ops->hardware_test(slot, test);
-       module_put(slot->ops->owner);
+       module_put(slot->owner);
 
 exit:
        if (retval)
@@ -447,8 +448,8 @@ int __pci_hp_initialize(struct hotplug_slot *slot, struct pci_bus *bus,
        if ((slot->info == NULL) || (slot->ops == NULL))
                return -EINVAL;
 
-       slot->ops->owner = owner;
-       slot->ops->mod_name = mod_name;
+       slot->owner = owner;
+       slot->mod_name = mod_name;
 
        /*
         * No problems if we call this interface from both ACPI_PCI_SLOT

diff --git a/drivers/pci/hotplug/pnv_php.c b/drivers/pci/hotplug/pnv_php.c
index 3276a5e4c430b6b9d4fbc96d1f1522959ffcb7a1..12b92a0ff6881510a3d5eb2ebadbc0da49b554a7 100644 (file)
--- a/drivers/pci/hotplug/pnv_php.c
+++ b/drivers/pci/hotplug/pnv_php.c
@@ -530,7 +530,7 @@ static int pnv_php_disable_slot(struct hotplug_slot *slot)
        return ret;
 }
 
-static struct hotplug_slot_ops php_slot_ops = {
+static const struct hotplug_slot_ops php_slot_ops = {
        .get_power_status       = pnv_php_get_power_state,
        .get_adapter_status     = pnv_php_get_adapter_state,
        .set_attention_status   = pnv_php_set_attention_state,

diff --git a/drivers/pci/hotplug/rpaphp.h b/drivers/pci/hotplug/rpaphp.h
index c8311724bd76af468d2bf6afce8c1e1f5028d07e..f83347819f7bb7aa6db2c798d2157d741539bcb0 100644 (file)
--- a/drivers/pci/hotplug/rpaphp.h
+++ b/drivers/pci/hotplug/rpaphp.h
@@ -70,7 +70,7 @@ struct slot {
        struct hotplug_slot *hotplug_slot;
 };
 
-extern struct hotplug_slot_ops rpaphp_hotplug_slot_ops;
+extern const struct hotplug_slot_ops rpaphp_hotplug_slot_ops;
 extern struct list_head rpaphp_slot_head;
 
 /* function prototypes */

diff --git a/drivers/pci/hotplug/rpaphp_core.c b/drivers/pci/hotplug/rpaphp_core.c
index 857c358b727b839aa1bca6f67b11c16bee282cb1..8620a3f8c987cc8ebdfb31ab686c17baf28393ca 100644 (file)
--- a/drivers/pci/hotplug/rpaphp_core.c
+++ b/drivers/pci/hotplug/rpaphp_core.c
@@ -477,7 +477,7 @@ static int disable_slot(struct hotplug_slot *hotplug_slot)
        return 0;
 }
 
-struct hotplug_slot_ops rpaphp_hotplug_slot_ops = {
+const struct hotplug_slot_ops rpaphp_hotplug_slot_ops = {
        .enable_slot = enable_slot,
        .disable_slot = disable_slot,
        .set_attention_status = set_attention_status,

diff --git a/drivers/pci/hotplug/s390_pci_hpc.c b/drivers/pci/hotplug/s390_pci_hpc.c
index 93b5341d282c3b12e2eeb5923387d456165ad6b8..5bd45fd4a92aef0ff66ff01bf65f69ea1f7e634f 100644 (file)
--- a/drivers/pci/hotplug/s390_pci_hpc.c
+++ b/drivers/pci/hotplug/s390_pci_hpc.c
@@ -130,7 +130,7 @@ static int get_adapter_status(struct hotplug_slot *hotplug_slot, u8 *value)
        return 0;
 }
 
-static struct hotplug_slot_ops s390_hotplug_slot_ops = {
+static const struct hotplug_slot_ops s390_hotplug_slot_ops = {
        .enable_slot =          enable_slot,
        .disable_slot =         disable_slot,
        .get_power_status =     get_power_status,

diff --git a/drivers/pci/hotplug/sgi_hotplug.c b/drivers/pci/hotplug/sgi_hotplug.c
index babd23409f61c2bfb844df703fa7409a0adb509c..af4c28c574dddd6dde6bed616dc58ed85bbd785b 100644 (file)
--- a/drivers/pci/hotplug/sgi_hotplug.c
+++ b/drivers/pci/hotplug/sgi_hotplug.c
@@ -80,7 +80,7 @@ static int enable_slot(struct hotplug_slot *slot);
 static int disable_slot(struct hotplug_slot *slot);
 static inline int get_power_status(struct hotplug_slot *slot, u8 *value);
 
-static struct hotplug_slot_ops sn_hotplug_slot_ops = {
+static const struct hotplug_slot_ops sn_hotplug_slot_ops = {
        .enable_slot            = enable_slot,
        .disable_slot           = disable_slot,
        .get_power_status       = get_power_status,

diff --git a/drivers/pci/hotplug/shpchp_core.c b/drivers/pci/hotplug/shpchp_core.c
index 97cee23f3d516e0d37c93f5f2f4f3af78410a529..26cbea04237c862d01fe0f78d85520780aae49bf 100644 (file)
--- a/drivers/pci/hotplug/shpchp_core.c
+++ b/drivers/pci/hotplug/shpchp_core.c
@@ -51,7 +51,7 @@ static int get_attention_status(struct hotplug_slot *slot, u8 *value);
 static int get_latch_status(struct hotplug_slot *slot, u8 *value);
 static int get_adapter_status(struct hotplug_slot *slot, u8 *value);
 
-static struct hotplug_slot_ops shpchp_hotplug_slot_ops = {
+static const struct hotplug_slot_ops shpchp_hotplug_slot_ops = {
        .set_attention_status = set_attention_status,
        .enable_slot =          enable_slot,
        .disable_slot =         disable_slot,

diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c
index 1835f3a7aa8d2f5a502a0629bfb0c0cc96420dd4..0e54588825cbd611b2234da2eb0909ced2746bf6 100644 (file)
--- a/drivers/pci/pci.c
+++ b/drivers/pci/pci.c
@@ -4571,13 +4571,13 @@ static int pci_reset_hotplug_slot(struct hotplug_slot *hotplug, int probe)
 {
        int rc = -ENOTTY;
 
-       if (!hotplug || !try_module_get(hotplug->ops->owner))
+       if (!hotplug || !try_module_get(hotplug->owner))
                return rc;
 
        if (hotplug->ops->reset_slot)
                rc = hotplug->ops->reset_slot(hotplug, probe);
 
-       module_put(hotplug->ops->owner);
+       module_put(hotplug->owner);
 
        return rc;
 }

diff --git a/drivers/pci/slot.c b/drivers/pci/slot.c
index e634229ece8957607b881e4f1e93c6b7028c85fe..145cd953b51835e2a8749db588e556163765e1e2 100644 (file)
--- a/drivers/pci/slot.c
+++ b/drivers/pci/slot.c
@@ -371,7 +371,7 @@ void pci_hp_create_module_link(struct pci_slot *pci_slot)
 
        if (!slot || !slot->ops)
                return;
-       kobj = kset_find_obj(module_kset, slot->ops->mod_name);
+       kobj = kset_find_obj(module_kset, slot->mod_name);
        if (!kobj)
                return;
        ret = sysfs_create_link(&pci_slot->kobj, kobj, "module");

diff --git a/drivers/platform/x86/asus-wmi.c b/drivers/platform/x86/asus-wmi.c
index 2d6e272315a82eae024c0fe3f242f097c2787f02..a8aa2eadfd82cbf6d0d9d39e35a67de4beecbc0e 100644 (file)
--- a/drivers/platform/x86/asus-wmi.c
+++ b/drivers/platform/x86/asus-wmi.c
@@ -868,8 +868,7 @@ static int asus_get_adapter_status(struct hotplug_slot *hotplug_slot,
        return 0;
 }
 
-static struct hotplug_slot_ops asus_hotplug_slot_ops = {
-       .owner = THIS_MODULE,
+static const struct hotplug_slot_ops asus_hotplug_slot_ops = {
        .get_adapter_status = asus_get_adapter_status,
        .get_power_status = asus_get_adapter_status,
 };

diff --git a/drivers/platform/x86/eeepc-laptop.c b/drivers/platform/x86/eeepc-laptop.c
index a4bbf6ecd1f08d185282d348d50f239d8efb305a..41a364376e91213bdbae8629699e87025c8b4041 100644 (file)
--- a/drivers/platform/x86/eeepc-laptop.c
+++ b/drivers/platform/x86/eeepc-laptop.c
@@ -726,8 +726,7 @@ static int eeepc_get_adapter_status(struct hotplug_slot *hotplug_slot,
        return 0;
 }
 
-static struct hotplug_slot_ops eeepc_hotplug_slot_ops = {
-       .owner = THIS_MODULE,
+static const struct hotplug_slot_ops eeepc_hotplug_slot_ops = {
        .get_adapter_status = eeepc_get_adapter_status,
        .get_power_status = eeepc_get_adapter_status,
 };

diff --git a/include/linux/pci_hotplug.h b/include/linux/pci_hotplug.h
index a6d6650a0490a063c2f221e509979afa6c2fad49..372dbe95c207901ea4b762bb2f12fa13bdb6aa7c 100644 (file)
--- a/include/linux/pci_hotplug.h
+++ b/include/linux/pci_hotplug.h
@@ -16,8 +16,6 @@
 
 /**
  * struct hotplug_slot_ops -the callbacks that the hotplug pci core can use
- * @owner: The module owner of this structure
- * @mod_name: The module name (KBUILD_MODNAME) of this structure
  * @enable_slot: Called when the user wants to enable a specific pci slot
  * @disable_slot: Called when the user wants to disable a specific pci slot
  * @set_attention_status: Called to set the specific slot's attention LED to
@@ -46,8 +44,6 @@
  * set an LED, enable / disable power, etc.)
  */
 struct hotplug_slot_ops {
-       struct module *owner;
-       const char *mod_name;
        int (*enable_slot)              (struct hotplug_slot *slot);
        int (*disable_slot)             (struct hotplug_slot *slot);
        int (*set_attention_status)     (struct hotplug_slot *slot, u8 value);
@@ -82,15 +78,19 @@ struct hotplug_slot_info {
  * this slot.
  * @private: used by the hotplug pci controller driver to store whatever it
  * needs.
+ * @owner: The module owner of this structure
+ * @mod_name: The module name (KBUILD_MODNAME) of this structure
  */
 struct hotplug_slot {
-       struct hotplug_slot_ops         *ops;
+       const struct hotplug_slot_ops   *ops;
        struct hotplug_slot_info        *info;
        void                            *private;
 
        /* Variables below this are for use only by the hotplug pci core. */
        struct list_head                slot_list;
        struct pci_slot                 *pci_slot;
+       struct module                   *owner;
+       const char                      *mod_name;
 };
 
 static inline const char *hotplug_slot_name(const struct hotplug_slot *slot)