Hello,
This patch series finishes the constification of struct drm_driver in the DRM core by avoiding modifications to the structure for legacy PCI drivers. This improves security by avoiding function pointers in writable memory.
The change turned out to be fairly easy, with preparation in patch 1/3 that moves the only non-const field out of drm_driver, and patch 2/3 that finishes the constification in the DRM core. Patch 3/3 is a low-hanging fruit that constifies the structure in a few drivers.
The remaining non-const drivers modify the structure internally to set the num_ioctls fields, and in a few cases, change the driver features. This will require a bit more work to fix these cases.
Laurent Pinchart (3): drm: Move legacy device list out of drm_driver drm: Use a const drm_driver for legacy PCI devices drm: Constify drm_driver in drivers that don't modify it
drivers/gpu/drm/arc/arcpgu_drv.c | 2 +- drivers/gpu/drm/drm_drv.c | 4 ---- drivers/gpu/drm/drm_pci.c | 33 +++++++++++++++++++++----------- drivers/gpu/drm/kmb/kmb_drv.c | 2 +- drivers/gpu/drm/tdfx/tdfx_drv.c | 2 +- include/drm/drm_device.h | 14 +++----------- include/drm/drm_drv.h | 2 -- include/drm/drm_legacy.h | 10 ++++++---- 8 files changed, 34 insertions(+), 35 deletions(-)
The drm_driver structure contains a single field (legacy_dev_list) that is modified by the DRM core, used to store a linked list of legacy DRM devices associated with the driver. In order to make the structure const, move the field out to a global variable. This requires locking access to the global where the local field didn't require serialization, but this only affects legacy drivers, and isn't in any hot path.
While at it, compile-out the legacy_dev_list field when DRM_LEGACY isn't defined.
Signed-off-by: Laurent Pinchart laurent.pinchart+renesas@ideasonboard.com Reviewed-by: Daniel Vetter daniel.vetter@ffwll.ch Reviewed-by: Emil Velikov emil.velikov@collabora.com --- Changes since v1:
- Move the legacy_dev_list to the end of struct drm_device, in the existing DRM_LEGACY section - Drop the kerneldoc comment for legacy_dev_list --- drivers/gpu/drm/drm_pci.c | 25 +++++++++++++++++-------- include/drm/drm_device.h | 10 +++------- include/drm/drm_drv.h | 2 -- 3 files changed, 20 insertions(+), 17 deletions(-)
diff --git a/drivers/gpu/drm/drm_pci.c b/drivers/gpu/drm/drm_pci.c index 6dba4b8ce4fe..dfb138aaccba 100644 --- a/drivers/gpu/drm/drm_pci.c +++ b/drivers/gpu/drm/drm_pci.c @@ -24,6 +24,8 @@
#include <linux/dma-mapping.h> #include <linux/export.h> +#include <linux/list.h> +#include <linux/mutex.h> #include <linux/pci.h> #include <linux/slab.h>
@@ -36,6 +38,9 @@ #include "drm_legacy.h"
#ifdef CONFIG_DRM_LEGACY +/* List of devices hanging off drivers with stealth attach. */ +static LIST_HEAD(legacy_dev_list); +static DEFINE_MUTEX(legacy_dev_list_lock);
/** * drm_pci_alloc - Allocate a PCI consistent memory block, for DMA. @@ -225,10 +230,11 @@ static int drm_get_pci_dev(struct pci_dev *pdev, if (ret) goto err_agp;
- /* No locking needed since shadow-attach is single-threaded since it may - * only be called from the per-driver module init hook. */ - if (drm_core_check_feature(dev, DRIVER_LEGACY)) - list_add_tail(&dev->legacy_dev_list, &driver->legacy_dev_list); + if (drm_core_check_feature(dev, DRIVER_LEGACY)) { + mutex_lock(&legacy_dev_list_lock); + list_add_tail(&dev->legacy_dev_list, &legacy_dev_list); + mutex_unlock(&legacy_dev_list_lock); + }
return 0;
@@ -261,7 +267,6 @@ int drm_legacy_pci_init(struct drm_driver *driver, struct pci_driver *pdriver) return -EINVAL;
/* If not using KMS, fall back to stealth mode manual scanning. */ - INIT_LIST_HEAD(&driver->legacy_dev_list); for (i = 0; pdriver->id_table[i].vendor != 0; i++) { pid = &pdriver->id_table[i];
@@ -304,11 +309,15 @@ void drm_legacy_pci_exit(struct drm_driver *driver, struct pci_driver *pdriver) if (!(driver->driver_features & DRIVER_LEGACY)) { WARN_ON(1); } else { - list_for_each_entry_safe(dev, tmp, &driver->legacy_dev_list, + mutex_lock(&legacy_dev_list_lock); + list_for_each_entry_safe(dev, tmp, &legacy_dev_list, legacy_dev_list) { - list_del(&dev->legacy_dev_list); - drm_put_dev(dev); + if (dev->driver == driver) { + list_del(&dev->legacy_dev_list); + drm_put_dev(dev); + } } + mutex_unlock(&legacy_dev_list_lock); } DRM_INFO("Module unloaded\n"); } diff --git a/include/drm/drm_device.h b/include/drm/drm_device.h index 283a93ce4617..bd5abe7cd48f 100644 --- a/include/drm/drm_device.h +++ b/include/drm/drm_device.h @@ -51,13 +51,6 @@ enum switch_power_state { * may contain multiple heads. */ struct drm_device { - /** - * @legacy_dev_list: - * - * List of devices per driver for stealth attach cleanup - */ - struct list_head legacy_dev_list; - /** @if_version: Highest interface version set */ int if_version;
@@ -336,6 +329,9 @@ struct drm_device { /* Everything below here is for legacy driver, never use! */ /* private: */ #if IS_ENABLED(CONFIG_DRM_LEGACY) + /* List of devices per driver for stealth attach cleanup */ + struct list_head legacy_dev_list; + /* Context handle management - linked list of context handles */ struct list_head ctxlist;
diff --git a/include/drm/drm_drv.h b/include/drm/drm_drv.h index 02787319246a..827838e0a97e 100644 --- a/include/drm/drm_drv.h +++ b/include/drm/drm_drv.h @@ -499,8 +499,6 @@ struct drm_driver { /* Everything below here is for legacy driver, never use! */ /* private: */
- /* List of devices hanging off this driver with stealth attach. */ - struct list_head legacy_dev_list; int (*firstopen) (struct drm_device *); void (*preclose) (struct drm_device *, struct drm_file *file_priv); int (*dma_ioctl) (struct drm_device *dev, void *data, struct drm_file *file_priv);
On Tue, Dec 15, 2020 at 10:31:24PM +0200, Laurent Pinchart wrote:
The drm_driver structure contains a single field (legacy_dev_list) that is modified by the DRM core, used to store a linked list of legacy DRM devices associated with the driver. In order to make the structure const, move the field out to a global variable. This requires locking access to the global where the local field didn't require serialization, but this only affects legacy drivers, and isn't in any hot path.
While at it, compile-out the legacy_dev_list field when DRM_LEGACY isn't defined.
Signed-off-by: Laurent Pinchart laurent.pinchart+renesas@ideasonboard.com Reviewed-by: Daniel Vetter daniel.vetter@ffwll.ch
Hah, I didn't notice my review here, read it again, still looks good :-) -Daniel
Reviewed-by: Emil Velikov emil.velikov@collabora.com
Changes since v1:
- Move the legacy_dev_list to the end of struct drm_device, in the existing DRM_LEGACY section
- Drop the kerneldoc comment for legacy_dev_list
drivers/gpu/drm/drm_pci.c | 25 +++++++++++++++++-------- include/drm/drm_device.h | 10 +++------- include/drm/drm_drv.h | 2 -- 3 files changed, 20 insertions(+), 17 deletions(-)
diff --git a/drivers/gpu/drm/drm_pci.c b/drivers/gpu/drm/drm_pci.c index 6dba4b8ce4fe..dfb138aaccba 100644 --- a/drivers/gpu/drm/drm_pci.c +++ b/drivers/gpu/drm/drm_pci.c @@ -24,6 +24,8 @@
#include <linux/dma-mapping.h> #include <linux/export.h> +#include <linux/list.h> +#include <linux/mutex.h> #include <linux/pci.h> #include <linux/slab.h>
@@ -36,6 +38,9 @@ #include "drm_legacy.h"
#ifdef CONFIG_DRM_LEGACY +/* List of devices hanging off drivers with stealth attach. */ +static LIST_HEAD(legacy_dev_list); +static DEFINE_MUTEX(legacy_dev_list_lock);
/**
- drm_pci_alloc - Allocate a PCI consistent memory block, for DMA.
@@ -225,10 +230,11 @@ static int drm_get_pci_dev(struct pci_dev *pdev, if (ret) goto err_agp;
- /* No locking needed since shadow-attach is single-threaded since it may
* only be called from the per-driver module init hook. */- if (drm_core_check_feature(dev, DRIVER_LEGACY))
list_add_tail(&dev->legacy_dev_list, &driver->legacy_dev_list);
if (drm_core_check_feature(dev, DRIVER_LEGACY)) {
mutex_lock(&legacy_dev_list_lock);list_add_tail(&dev->legacy_dev_list, &legacy_dev_list);mutex_unlock(&legacy_dev_list_lock);}
return 0;
@@ -261,7 +267,6 @@ int drm_legacy_pci_init(struct drm_driver *driver, struct pci_driver *pdriver) return -EINVAL;
/* If not using KMS, fall back to stealth mode manual scanning. */
- INIT_LIST_HEAD(&driver->legacy_dev_list); for (i = 0; pdriver->id_table[i].vendor != 0; i++) { pid = &pdriver->id_table[i];
@@ -304,11 +309,15 @@ void drm_legacy_pci_exit(struct drm_driver *driver, struct pci_driver *pdriver) if (!(driver->driver_features & DRIVER_LEGACY)) { WARN_ON(1); } else {
list_for_each_entry_safe(dev, tmp, &driver->legacy_dev_list,
mutex_lock(&legacy_dev_list_lock);list_for_each_entry_safe(dev, tmp, &legacy_dev_list, legacy_dev_list) {
list_del(&dev->legacy_dev_list);drm_put_dev(dev);
if (dev->driver == driver) {list_del(&dev->legacy_dev_list);drm_put_dev(dev);}mutex_unlock(&legacy_dev_list_lock);} diff --git a/include/drm/drm_device.h b/include/drm/drm_device.h index 283a93ce4617..bd5abe7cd48f 100644 --- a/include/drm/drm_device.h +++ b/include/drm/drm_device.h @@ -51,13 +51,6 @@ enum switch_power_state {
- may contain multiple heads.
*/ struct drm_device {
- /**
* @legacy_dev_list:** List of devices per driver for stealth attach cleanup*/- struct list_head legacy_dev_list;
- /** @if_version: Highest interface version set */ int if_version;
@@ -336,6 +329,9 @@ struct drm_device { /* Everything below here is for legacy driver, never use! */ /* private: */ #if IS_ENABLED(CONFIG_DRM_LEGACY)
- /* List of devices per driver for stealth attach cleanup */
- struct list_head legacy_dev_list;
- /* Context handle management - linked list of context handles */ struct list_head ctxlist;
diff --git a/include/drm/drm_drv.h b/include/drm/drm_drv.h index 02787319246a..827838e0a97e 100644 --- a/include/drm/drm_drv.h +++ b/include/drm/drm_drv.h @@ -499,8 +499,6 @@ struct drm_driver { /* Everything below here is for legacy driver, never use! */ /* private: */
- /* List of devices hanging off this driver with stealth attach. */
- struct list_head legacy_dev_list; int (*firstopen) (struct drm_device *); void (*preclose) (struct drm_device *, struct drm_file *file_priv); int (*dma_ioctl) (struct drm_device *dev, void *data, struct drm_file *file_priv);
-- Regards,
Laurent Pinchart
dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Now that the legacy PCI support code doesn't need to write to the drm_driver structure, it can be treated as const through the whole DRM core, unconditionally. This allows declaring the structure as const in all drivers, removing one possible attack vector.
Signed-off-by: Laurent Pinchart laurent.pinchart+renesas@ideasonboard.com --- drivers/gpu/drm/drm_drv.c | 4 ---- drivers/gpu/drm/drm_pci.c | 8 +++++--- include/drm/drm_device.h | 4 ---- include/drm/drm_legacy.h | 10 ++++++---- 4 files changed, 11 insertions(+), 15 deletions(-)
diff --git a/drivers/gpu/drm/drm_drv.c b/drivers/gpu/drm/drm_drv.c index 734303802bc3..3f57e880685e 100644 --- a/drivers/gpu/drm/drm_drv.c +++ b/drivers/gpu/drm/drm_drv.c @@ -589,11 +589,7 @@ static int drm_dev_init(struct drm_device *dev,
kref_init(&dev->ref); dev->dev = get_device(parent); -#ifdef CONFIG_DRM_LEGACY - dev->driver = (struct drm_driver *)driver; -#else dev->driver = driver; -#endif
INIT_LIST_HEAD(&dev->managed.resources); spin_lock_init(&dev->managed.lock); diff --git a/drivers/gpu/drm/drm_pci.c b/drivers/gpu/drm/drm_pci.c index dfb138aaccba..5370e6b492fd 100644 --- a/drivers/gpu/drm/drm_pci.c +++ b/drivers/gpu/drm/drm_pci.c @@ -201,7 +201,7 @@ static void drm_pci_agp_init(struct drm_device *dev)
static int drm_get_pci_dev(struct pci_dev *pdev, const struct pci_device_id *ent, - struct drm_driver *driver) + const struct drm_driver *driver) { struct drm_device *dev; int ret; @@ -255,7 +255,8 @@ static int drm_get_pci_dev(struct pci_dev *pdev, * * Return: 0 on success or a negative error code on failure. */ -int drm_legacy_pci_init(struct drm_driver *driver, struct pci_driver *pdriver) +int drm_legacy_pci_init(const struct drm_driver *driver, + struct pci_driver *pdriver) { struct pci_dev *pdev = NULL; const struct pci_device_id *pid; @@ -300,7 +301,8 @@ EXPORT_SYMBOL(drm_legacy_pci_init); * Unregister a DRM driver shadow-attached through drm_legacy_pci_init(). This * is deprecated and only used by dri1 drivers. */ -void drm_legacy_pci_exit(struct drm_driver *driver, struct pci_driver *pdriver) +void drm_legacy_pci_exit(const struct drm_driver *driver, + struct pci_driver *pdriver) { struct drm_device *dev, *tmp;
diff --git a/include/drm/drm_device.h b/include/drm/drm_device.h index bd5abe7cd48f..939904ae88fc 100644 --- a/include/drm/drm_device.h +++ b/include/drm/drm_device.h @@ -76,11 +76,7 @@ struct drm_device { } managed;
/** @driver: DRM driver managing the device */ -#ifdef CONFIG_DRM_LEGACY - struct drm_driver *driver; -#else const struct drm_driver *driver; -#endif
/** * @dev_private: diff --git a/include/drm/drm_legacy.h b/include/drm/drm_legacy.h index 852d7451eeb1..8ed04e9be997 100644 --- a/include/drm/drm_legacy.h +++ b/include/drm/drm_legacy.h @@ -198,8 +198,10 @@ struct drm_dma_handle *drm_pci_alloc(struct drm_device *dev, size_t size, size_t align); void drm_pci_free(struct drm_device *dev, struct drm_dma_handle *dmah);
-int drm_legacy_pci_init(struct drm_driver *driver, struct pci_driver *pdriver); -void drm_legacy_pci_exit(struct drm_driver *driver, struct pci_driver *pdriver); +int drm_legacy_pci_init(const struct drm_driver *driver, + struct pci_driver *pdriver); +void drm_legacy_pci_exit(const struct drm_driver *driver, + struct pci_driver *pdriver);
#else
@@ -214,13 +216,13 @@ static inline void drm_pci_free(struct drm_device *dev, { }
-static inline int drm_legacy_pci_init(struct drm_driver *driver, +static inline int drm_legacy_pci_init(const struct drm_driver *driver, struct pci_driver *pdriver) { return -EINVAL; }
-static inline void drm_legacy_pci_exit(struct drm_driver *driver, +static inline void drm_legacy_pci_exit(const struct drm_driver *driver, struct pci_driver *pdriver) { }
On Tue, Dec 15, 2020 at 10:31:25PM +0200, Laurent Pinchart wrote:
Now that the legacy PCI support code doesn't need to write to the drm_driver structure, it can be treated as const through the whole DRM core, unconditionally. This allows declaring the structure as const in all drivers, removing one possible attack vector.
Signed-off-by: Laurent Pinchart laurent.pinchart+renesas@ideasonboard.com
I didn't inquire the compiler whether you got all the combos right, but looks complete.
Reviewed-by: Daniel Vetter daniel.vetter@ffwll.ch
drivers/gpu/drm/drm_drv.c | 4 ---- drivers/gpu/drm/drm_pci.c | 8 +++++--- include/drm/drm_device.h | 4 ---- include/drm/drm_legacy.h | 10 ++++++---- 4 files changed, 11 insertions(+), 15 deletions(-)
diff --git a/drivers/gpu/drm/drm_drv.c b/drivers/gpu/drm/drm_drv.c index 734303802bc3..3f57e880685e 100644 --- a/drivers/gpu/drm/drm_drv.c +++ b/drivers/gpu/drm/drm_drv.c @@ -589,11 +589,7 @@ static int drm_dev_init(struct drm_device *dev,
kref_init(&dev->ref); dev->dev = get_device(parent); -#ifdef CONFIG_DRM_LEGACY
- dev->driver = (struct drm_driver *)driver;
-#else dev->driver = driver; -#endif
INIT_LIST_HEAD(&dev->managed.resources); spin_lock_init(&dev->managed.lock); diff --git a/drivers/gpu/drm/drm_pci.c b/drivers/gpu/drm/drm_pci.c index dfb138aaccba..5370e6b492fd 100644 --- a/drivers/gpu/drm/drm_pci.c +++ b/drivers/gpu/drm/drm_pci.c @@ -201,7 +201,7 @@ static void drm_pci_agp_init(struct drm_device *dev)
static int drm_get_pci_dev(struct pci_dev *pdev, const struct pci_device_id *ent,
struct drm_driver *driver)
const struct drm_driver *driver){ struct drm_device *dev; int ret; @@ -255,7 +255,8 @@ static int drm_get_pci_dev(struct pci_dev *pdev,
- Return: 0 on success or a negative error code on failure.
*/ -int drm_legacy_pci_init(struct drm_driver *driver, struct pci_driver *pdriver) +int drm_legacy_pci_init(const struct drm_driver *driver,
struct pci_driver *pdriver){ struct pci_dev *pdev = NULL; const struct pci_device_id *pid; @@ -300,7 +301,8 @@ EXPORT_SYMBOL(drm_legacy_pci_init);
- Unregister a DRM driver shadow-attached through drm_legacy_pci_init(). This
- is deprecated and only used by dri1 drivers.
*/ -void drm_legacy_pci_exit(struct drm_driver *driver, struct pci_driver *pdriver) +void drm_legacy_pci_exit(const struct drm_driver *driver,
struct pci_driver *pdriver){ struct drm_device *dev, *tmp;
diff --git a/include/drm/drm_device.h b/include/drm/drm_device.h index bd5abe7cd48f..939904ae88fc 100644 --- a/include/drm/drm_device.h +++ b/include/drm/drm_device.h @@ -76,11 +76,7 @@ struct drm_device { } managed;
/** @driver: DRM driver managing the device */ -#ifdef CONFIG_DRM_LEGACY
- struct drm_driver *driver;
-#else const struct drm_driver *driver; -#endif
/** * @dev_private: diff --git a/include/drm/drm_legacy.h b/include/drm/drm_legacy.h index 852d7451eeb1..8ed04e9be997 100644 --- a/include/drm/drm_legacy.h +++ b/include/drm/drm_legacy.h @@ -198,8 +198,10 @@ struct drm_dma_handle *drm_pci_alloc(struct drm_device *dev, size_t size, size_t align); void drm_pci_free(struct drm_device *dev, struct drm_dma_handle *dmah);
-int drm_legacy_pci_init(struct drm_driver *driver, struct pci_driver *pdriver); -void drm_legacy_pci_exit(struct drm_driver *driver, struct pci_driver *pdriver); +int drm_legacy_pci_init(const struct drm_driver *driver,
struct pci_driver *pdriver);+void drm_legacy_pci_exit(const struct drm_driver *driver,
struct pci_driver *pdriver);#else
@@ -214,13 +216,13 @@ static inline void drm_pci_free(struct drm_device *dev, { }
-static inline int drm_legacy_pci_init(struct drm_driver *driver, +static inline int drm_legacy_pci_init(const struct drm_driver *driver, struct pci_driver *pdriver) { return -EINVAL; }
-static inline void drm_legacy_pci_exit(struct drm_driver *driver, +static inline void drm_legacy_pci_exit(const struct drm_driver *driver, struct pci_driver *pdriver) { } -- Regards,
Laurent Pinchart
dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
On Wed, Dec 16, 2020 at 03:29:00PM +0100, Daniel Vetter wrote:
On Tue, Dec 15, 2020 at 10:31:25PM +0200, Laurent Pinchart wrote:
Now that the legacy PCI support code doesn't need to write to the drm_driver structure, it can be treated as const through the whole DRM core, unconditionally. This allows declaring the structure as const in all drivers, removing one possible attack vector.
Signed-off-by: Laurent Pinchart laurent.pinchart+renesas@ideasonboard.com
I didn't inquire the compiler whether you got all the combos right, but looks complete.
I've compile-tested with CONFIG_DRM_LEGACY enabled and disabled, and CONFIG_PCI enabled, so we should be good.
Reviewed-by: Daniel Vetter daniel.vetter@ffwll.ch
drivers/gpu/drm/drm_drv.c | 4 ---- drivers/gpu/drm/drm_pci.c | 8 +++++--- include/drm/drm_device.h | 4 ---- include/drm/drm_legacy.h | 10 ++++++---- 4 files changed, 11 insertions(+), 15 deletions(-)
diff --git a/drivers/gpu/drm/drm_drv.c b/drivers/gpu/drm/drm_drv.c index 734303802bc3..3f57e880685e 100644 --- a/drivers/gpu/drm/drm_drv.c +++ b/drivers/gpu/drm/drm_drv.c @@ -589,11 +589,7 @@ static int drm_dev_init(struct drm_device *dev,
kref_init(&dev->ref); dev->dev = get_device(parent); -#ifdef CONFIG_DRM_LEGACY
- dev->driver = (struct drm_driver *)driver;
-#else dev->driver = driver; -#endif
INIT_LIST_HEAD(&dev->managed.resources); spin_lock_init(&dev->managed.lock); diff --git a/drivers/gpu/drm/drm_pci.c b/drivers/gpu/drm/drm_pci.c index dfb138aaccba..5370e6b492fd 100644 --- a/drivers/gpu/drm/drm_pci.c +++ b/drivers/gpu/drm/drm_pci.c @@ -201,7 +201,7 @@ static void drm_pci_agp_init(struct drm_device *dev)
static int drm_get_pci_dev(struct pci_dev *pdev, const struct pci_device_id *ent,
struct drm_driver *driver)
const struct drm_driver *driver){ struct drm_device *dev; int ret; @@ -255,7 +255,8 @@ static int drm_get_pci_dev(struct pci_dev *pdev,
- Return: 0 on success or a negative error code on failure.
*/ -int drm_legacy_pci_init(struct drm_driver *driver, struct pci_driver *pdriver) +int drm_legacy_pci_init(const struct drm_driver *driver,
struct pci_driver *pdriver){ struct pci_dev *pdev = NULL; const struct pci_device_id *pid; @@ -300,7 +301,8 @@ EXPORT_SYMBOL(drm_legacy_pci_init);
- Unregister a DRM driver shadow-attached through drm_legacy_pci_init(). This
- is deprecated and only used by dri1 drivers.
*/ -void drm_legacy_pci_exit(struct drm_driver *driver, struct pci_driver *pdriver) +void drm_legacy_pci_exit(const struct drm_driver *driver,
struct pci_driver *pdriver){ struct drm_device *dev, *tmp;
diff --git a/include/drm/drm_device.h b/include/drm/drm_device.h index bd5abe7cd48f..939904ae88fc 100644 --- a/include/drm/drm_device.h +++ b/include/drm/drm_device.h @@ -76,11 +76,7 @@ struct drm_device { } managed;
/** @driver: DRM driver managing the device */ -#ifdef CONFIG_DRM_LEGACY
- struct drm_driver *driver;
-#else const struct drm_driver *driver; -#endif
/** * @dev_private: diff --git a/include/drm/drm_legacy.h b/include/drm/drm_legacy.h index 852d7451eeb1..8ed04e9be997 100644 --- a/include/drm/drm_legacy.h +++ b/include/drm/drm_legacy.h @@ -198,8 +198,10 @@ struct drm_dma_handle *drm_pci_alloc(struct drm_device *dev, size_t size, size_t align); void drm_pci_free(struct drm_device *dev, struct drm_dma_handle *dmah);
-int drm_legacy_pci_init(struct drm_driver *driver, struct pci_driver *pdriver); -void drm_legacy_pci_exit(struct drm_driver *driver, struct pci_driver *pdriver); +int drm_legacy_pci_init(const struct drm_driver *driver,
struct pci_driver *pdriver);+void drm_legacy_pci_exit(const struct drm_driver *driver,
struct pci_driver *pdriver);#else
@@ -214,13 +216,13 @@ static inline void drm_pci_free(struct drm_device *dev, { }
-static inline int drm_legacy_pci_init(struct drm_driver *driver, +static inline int drm_legacy_pci_init(const struct drm_driver *driver, struct pci_driver *pdriver) { return -EINVAL; }
-static inline void drm_legacy_pci_exit(struct drm_driver *driver, +static inline void drm_legacy_pci_exit(const struct drm_driver *driver, struct pci_driver *pdriver) { }
A non-const structure containing function pointers is a possible attack vector. The drm_driver structure is already const in most drivers, but there are a few exceptions. Constify the structure in the drivers that don't need to modify at, as a low-hanging fruit. The rest of the drivers will need a more complex fix.
Signed-off-by: Laurent Pinchart laurent.pinchart+renesas@ideasonboard.com --- drivers/gpu/drm/arc/arcpgu_drv.c | 2 +- drivers/gpu/drm/kmb/kmb_drv.c | 2 +- drivers/gpu/drm/tdfx/tdfx_drv.c | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-)
diff --git a/drivers/gpu/drm/arc/arcpgu_drv.c b/drivers/gpu/drm/arc/arcpgu_drv.c index f164818ec477..077d006b1fbf 100644 --- a/drivers/gpu/drm/arc/arcpgu_drv.c +++ b/drivers/gpu/drm/arc/arcpgu_drv.c @@ -145,7 +145,7 @@ static void arcpgu_debugfs_init(struct drm_minor *minor) } #endif
-static struct drm_driver arcpgu_drm_driver = { +static const struct drm_driver arcpgu_drm_driver = { .driver_features = DRIVER_MODESET | DRIVER_GEM | DRIVER_ATOMIC, .name = "arcpgu", .desc = "ARC PGU Controller", diff --git a/drivers/gpu/drm/kmb/kmb_drv.c b/drivers/gpu/drm/kmb/kmb_drv.c index a31a840ce634..3c49668ec946 100644 --- a/drivers/gpu/drm/kmb/kmb_drv.c +++ b/drivers/gpu/drm/kmb/kmb_drv.c @@ -400,7 +400,7 @@ static void kmb_irq_reset(struct drm_device *drm)
DEFINE_DRM_GEM_CMA_FOPS(fops);
-static struct drm_driver kmb_driver = { +static const struct drm_driver kmb_driver = { .driver_features = DRIVER_GEM | DRIVER_MODESET | DRIVER_ATOMIC, .irq_handler = kmb_isr, diff --git a/drivers/gpu/drm/tdfx/tdfx_drv.c b/drivers/gpu/drm/tdfx/tdfx_drv.c index ab699bf0ac5c..58c185c299f4 100644 --- a/drivers/gpu/drm/tdfx/tdfx_drv.c +++ b/drivers/gpu/drm/tdfx/tdfx_drv.c @@ -56,7 +56,7 @@ static const struct file_operations tdfx_driver_fops = { .llseek = noop_llseek, };
-static struct drm_driver driver = { +static const struct drm_driver driver = { .driver_features = DRIVER_LEGACY, .fops = &tdfx_driver_fops, .name = DRIVER_NAME,
On Tue, Dec 15, 2020 at 10:31:26PM +0200, Laurent Pinchart wrote:
A non-const structure containing function pointers is a possible attack vector. The drm_driver structure is already const in most drivers, but there are a few exceptions. Constify the structure in the drivers that don't need to modify at, as a low-hanging fruit. The rest of the drivers will need a more complex fix.
Signed-off-by: Laurent Pinchart laurent.pinchart+renesas@ideasonboard.com
Reviewed-by: Daniel Vetter daniel.vetter@ffwll.ch
drivers/gpu/drm/arc/arcpgu_drv.c | 2 +- drivers/gpu/drm/kmb/kmb_drv.c | 2 +- drivers/gpu/drm/tdfx/tdfx_drv.c | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-)
diff --git a/drivers/gpu/drm/arc/arcpgu_drv.c b/drivers/gpu/drm/arc/arcpgu_drv.c index f164818ec477..077d006b1fbf 100644 --- a/drivers/gpu/drm/arc/arcpgu_drv.c +++ b/drivers/gpu/drm/arc/arcpgu_drv.c @@ -145,7 +145,7 @@ static void arcpgu_debugfs_init(struct drm_minor *minor) } #endif
-static struct drm_driver arcpgu_drm_driver = { +static const struct drm_driver arcpgu_drm_driver = { .driver_features = DRIVER_MODESET | DRIVER_GEM | DRIVER_ATOMIC, .name = "arcpgu", .desc = "ARC PGU Controller", diff --git a/drivers/gpu/drm/kmb/kmb_drv.c b/drivers/gpu/drm/kmb/kmb_drv.c index a31a840ce634..3c49668ec946 100644 --- a/drivers/gpu/drm/kmb/kmb_drv.c +++ b/drivers/gpu/drm/kmb/kmb_drv.c @@ -400,7 +400,7 @@ static void kmb_irq_reset(struct drm_device *drm)
DEFINE_DRM_GEM_CMA_FOPS(fops);
-static struct drm_driver kmb_driver = { +static const struct drm_driver kmb_driver = { .driver_features = DRIVER_GEM | DRIVER_MODESET | DRIVER_ATOMIC, .irq_handler = kmb_isr, diff --git a/drivers/gpu/drm/tdfx/tdfx_drv.c b/drivers/gpu/drm/tdfx/tdfx_drv.c index ab699bf0ac5c..58c185c299f4 100644 --- a/drivers/gpu/drm/tdfx/tdfx_drv.c +++ b/drivers/gpu/drm/tdfx/tdfx_drv.c @@ -56,7 +56,7 @@ static const struct file_operations tdfx_driver_fops = { .llseek = noop_llseek, };
-static struct drm_driver driver = { +static const struct drm_driver driver = { .driver_features = DRIVER_LEGACY, .fops = &tdfx_driver_fops, .name = DRIVER_NAME, -- Regards,
Laurent Pinchart
dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
dri-devel@lists.freedesktop.org
-
Daniel Vetter -
Laurent Pinchart -
Laurent Pinchart