Skip to content

Commit

Permalink
PCI: hotplug: Constify hotplug_slot_ops
Browse files Browse the repository at this point in the history
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 <[email protected]>
Signed-off-by: Bjorn Helgaas <[email protected]>
Reviewed-by: Rafael J. Wysocki <[email protected]>
Acked-by: Tyrel Datwyler <[email protected]>  # drivers/pci/hotplug/rpa*
Acked-by: Andy Shevchenko <[email protected]> # drivers/platform/x86
Cc: Len Brown <[email protected]>
Cc: Scott Murray <[email protected]>
Cc: Benjamin Herrenschmidt <[email protected]>
Cc: Paul Mackerras <[email protected]>
Cc: Michael Ellerman <[email protected]>
Cc: Oliver OHalloran <[email protected]>
Cc: Gavin Shan <[email protected]>
Cc: Sebastian Ott <[email protected]>
Cc: Gerald Schaefer <[email protected]>
Cc: Corentin Chary <[email protected]>
Cc: Darren Hart <[email protected]>
  • Loading branch information
l1k authored and bjorn-helgaas committed Sep 18, 2018
1 parent d758714 commit 81c4b5b
Show file tree
Hide file tree
Showing 17 changed files with 35 additions and 36 deletions.
2 changes: 1 addition & 1 deletion drivers/pci/hotplug/acpiphp_core.c
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
2 changes: 1 addition & 1 deletion drivers/pci/hotplug/cpci_hotplug_core.c
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
2 changes: 1 addition & 1 deletion drivers/pci/hotplug/cpqphp_core.c
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
2 changes: 1 addition & 1 deletion drivers/pci/hotplug/ibmphp.h
Original file line number Diff line number Diff line change
Expand Up @@ -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

2 changes: 1 addition & 1 deletion drivers/pci/hotplug/ibmphp_core.c
Original file line number Diff line number Diff line change
Expand Up @@ -1259,7 +1259,7 @@ int ibmphp_do_disable_slot(struct slot *slot_cur)
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,
Expand Down
27 changes: 14 additions & 13 deletions drivers/pci/hotplug/pci_hotplug_core.c
Original file line number Diff line number Diff line change
Expand Up @@ -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; \
}

Expand Down Expand Up @@ -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;
}
Expand All @@ -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)
Expand Down Expand Up @@ -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;
Expand All @@ -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)
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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
Expand Down
2 changes: 1 addition & 1 deletion drivers/pci/hotplug/pnv_php.c
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
2 changes: 1 addition & 1 deletion drivers/pci/hotplug/rpaphp.h
Original file line number Diff line number Diff line change
Expand Up @@ -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 */
Expand Down
2 changes: 1 addition & 1 deletion drivers/pci/hotplug/rpaphp_core.c
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
2 changes: 1 addition & 1 deletion drivers/pci/hotplug/s390_pci_hpc.c
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
2 changes: 1 addition & 1 deletion drivers/pci/hotplug/sgi_hotplug.c
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
2 changes: 1 addition & 1 deletion drivers/pci/hotplug/shpchp_core.c
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
4 changes: 2 additions & 2 deletions drivers/pci/pci.c
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
Expand Down
2 changes: 1 addition & 1 deletion drivers/pci/slot.c
Original file line number Diff line number Diff line change
Expand Up @@ -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");
Expand Down
3 changes: 1 addition & 2 deletions drivers/platform/x86/asus-wmi.c
Original file line number Diff line number Diff line change
Expand Up @@ -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,
};
Expand Down
3 changes: 1 addition & 2 deletions drivers/platform/x86/eeepc-laptop.c
Original file line number Diff line number Diff line change
Expand Up @@ -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,
};
Expand Down
10 changes: 5 additions & 5 deletions include/linux/pci_hotplug.h
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -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)
Expand Down

0 comments on commit 81c4b5b

Please sign in to comment.