All of lore.kernel.org
 help / color / mirror / Atom feed
From: Hans de Goede <hdegoede@redhat.com>
To: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>,
	Maxime Ripard <mripard@kernel.org>,
	Thomas Zimmermann <tzimmermann@suse.de>,
	Daniel Vetter <daniel@ffwll.ch>, David Airlie <airlied@linux.ie>,
	Jani Nikula <jani.nikula@linux.intel.com>,
	Joonas Lahtinen <joonas.lahtinen@linux.intel.com>,
	Rodrigo Vivi <rodrigo.vivi@intel.com>,
	Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
	Guenter Roeck <linux@roeck-us.net>,
	Heikki Krogerus <heikki.krogerus@linux.intel.com>
Cc: Hans de Goede <hdegoede@redhat.com>,
	intel-gfx <intel-gfx@lists.freedesktop.org>,
	dri-devel@lists.freedesktop.org,
	platform-driver-x86@vger.kernel.org, linux-usb@vger.kernel.org
Subject: [PATCH 1/9] drm/connector: Make the drm_sysfs connector->kdev device hold a reference to the connector
Date: Wed, 28 Apr 2021 23:52:49 +0200	[thread overview]
Message-ID: <20210428215257.500088-2-hdegoede@redhat.com> (raw)
In-Reply-To: <20210428215257.500088-1-hdegoede@redhat.com>

Userspace could hold open a reference to the connector->kdev device,
through e.g. holding a sysfs-atrtribute open after
drm_sysfs_connector_remove() has been called. In this case the connector
could be free-ed while the connector->kdev device's drvdata is still
pointing to it.

Give drm_connector devices there own device type, which allows
us to specify our own release function and make drm_sysfs_connector_add()
take a reference on the connector object, and have the new release
function put the reference when the device is released.

Giving drm_connector devices there own device type, will also allow
checking if a device is a drm_connector device with a
"if (device->type == &drm_sysfs_device_connector)" check.

Note that the setting of the name member of the device_type struct will
cause udev events for drm_connector-s to now contain DEVTYPE=drm_connector
as extra info. So this extends the uevent part of the userspace API.

Signed-off-by: Hans de Goede <hdegoede@redhat.com>
---
 drivers/gpu/drm/drm_sysfs.c | 54 +++++++++++++++++++++++++++++++------
 1 file changed, 46 insertions(+), 8 deletions(-)

diff --git a/drivers/gpu/drm/drm_sysfs.c b/drivers/gpu/drm/drm_sysfs.c
index f0336c804639..c344c6d5e738 100644
--- a/drivers/gpu/drm/drm_sysfs.c
+++ b/drivers/gpu/drm/drm_sysfs.c
@@ -50,6 +50,10 @@ static struct device_type drm_sysfs_device_minor = {
 	.name = "drm_minor"
 };
 
+static struct device_type drm_sysfs_device_connector = {
+	.name = "drm_connector",
+};
+
 struct class *drm_class;
 
 static char *drm_devnode(struct device *dev, umode_t *mode)
@@ -271,30 +275,64 @@ static const struct attribute_group *connector_dev_groups[] = {
 	NULL
 };
 
+static void drm_sysfs_connector_release(struct device *dev)
+{
+	struct drm_connector *connector = to_drm_connector(dev);
+
+	drm_connector_put(connector);
+	kfree(dev);
+}
+
 int drm_sysfs_connector_add(struct drm_connector *connector)
 {
 	struct drm_device *dev = connector->dev;
+	struct device *kdev;
+	int r;
 
 	if (connector->kdev)
 		return 0;
 
-	connector->kdev =
-		device_create_with_groups(drm_class, dev->primary->kdev, 0,
-					  connector, connector_dev_groups,
-					  "card%d-%s", dev->primary->index,
-					  connector->name);
+	kdev = kzalloc(sizeof(*kdev), GFP_KERNEL);
+	if (!kdev)
+		return -ENOMEM;
+
+	device_initialize(kdev);
+	kdev->class = drm_class;
+	kdev->type = &drm_sysfs_device_connector;
+	kdev->parent = dev->primary->kdev;
+	kdev->groups = connector_dev_groups;
+	kdev->release = drm_sysfs_connector_release;
+	dev_set_drvdata(kdev, connector);
+
+	r = dev_set_name(kdev, "card%d-%s", dev->primary->index, connector->name);
+	if (r)
+		goto err_free;
+
 	DRM_DEBUG("adding \"%s\" to sysfs\n",
 		  connector->name);
 
-	if (IS_ERR(connector->kdev)) {
-		DRM_ERROR("failed to register connector device: %ld\n", PTR_ERR(connector->kdev));
-		return PTR_ERR(connector->kdev);
+	r = device_add(kdev);
+	if (r) {
+		DRM_ERROR("failed to register connector device: %d\n", r);
+		goto err_free;
 	}
 
+	/*
+	 * Ensure the connector object does not get free-ed if userspace still has
+	 * references open to the device through e.g. the connector sysfs-attributes.
+	 */
+	drm_connector_get(connector);
+
+	connector->kdev = kdev;
+
 	if (connector->ddc)
 		return sysfs_create_link(&connector->kdev->kobj,
 				 &connector->ddc->dev.kobj, "ddc");
 	return 0;
+
+err_free:
+	put_device(kdev);
+	return r;
 }
 
 void drm_sysfs_connector_remove(struct drm_connector *connector)
-- 
2.31.1


WARNING: multiple messages have this Message-ID (diff)
From: Hans de Goede <hdegoede@redhat.com>
To: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>,
	Maxime Ripard <mripard@kernel.org>,
	Thomas Zimmermann <tzimmermann@suse.de>,
	Daniel Vetter <daniel@ffwll.ch>, David Airlie <airlied@linux.ie>,
	Jani Nikula <jani.nikula@linux.intel.com>,
	Joonas Lahtinen <joonas.lahtinen@linux.intel.com>,
	Rodrigo Vivi <rodrigo.vivi@intel.com>,
	Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
	Guenter Roeck <linux@roeck-us.net>,
	Heikki Krogerus <heikki.krogerus@linux.intel.com>
Cc: Hans de Goede <hdegoede@redhat.com>,
	intel-gfx <intel-gfx@lists.freedesktop.org>,
	linux-usb@vger.kernel.org, dri-devel@lists.freedesktop.org,
	platform-driver-x86@vger.kernel.org
Subject: [PATCH 1/9] drm/connector: Make the drm_sysfs connector->kdev device hold a reference to the connector
Date: Wed, 28 Apr 2021 23:52:49 +0200	[thread overview]
Message-ID: <20210428215257.500088-2-hdegoede@redhat.com> (raw)
In-Reply-To: <20210428215257.500088-1-hdegoede@redhat.com>

Userspace could hold open a reference to the connector->kdev device,
through e.g. holding a sysfs-atrtribute open after
drm_sysfs_connector_remove() has been called. In this case the connector
could be free-ed while the connector->kdev device's drvdata is still
pointing to it.

Give drm_connector devices there own device type, which allows
us to specify our own release function and make drm_sysfs_connector_add()
take a reference on the connector object, and have the new release
function put the reference when the device is released.

Giving drm_connector devices there own device type, will also allow
checking if a device is a drm_connector device with a
"if (device->type == &drm_sysfs_device_connector)" check.

Note that the setting of the name member of the device_type struct will
cause udev events for drm_connector-s to now contain DEVTYPE=drm_connector
as extra info. So this extends the uevent part of the userspace API.

Signed-off-by: Hans de Goede <hdegoede@redhat.com>
---
 drivers/gpu/drm/drm_sysfs.c | 54 +++++++++++++++++++++++++++++++------
 1 file changed, 46 insertions(+), 8 deletions(-)

diff --git a/drivers/gpu/drm/drm_sysfs.c b/drivers/gpu/drm/drm_sysfs.c
index f0336c804639..c344c6d5e738 100644
--- a/drivers/gpu/drm/drm_sysfs.c
+++ b/drivers/gpu/drm/drm_sysfs.c
@@ -50,6 +50,10 @@ static struct device_type drm_sysfs_device_minor = {
 	.name = "drm_minor"
 };
 
+static struct device_type drm_sysfs_device_connector = {
+	.name = "drm_connector",
+};
+
 struct class *drm_class;
 
 static char *drm_devnode(struct device *dev, umode_t *mode)
@@ -271,30 +275,64 @@ static const struct attribute_group *connector_dev_groups[] = {
 	NULL
 };
 
+static void drm_sysfs_connector_release(struct device *dev)
+{
+	struct drm_connector *connector = to_drm_connector(dev);
+
+	drm_connector_put(connector);
+	kfree(dev);
+}
+
 int drm_sysfs_connector_add(struct drm_connector *connector)
 {
 	struct drm_device *dev = connector->dev;
+	struct device *kdev;
+	int r;
 
 	if (connector->kdev)
 		return 0;
 
-	connector->kdev =
-		device_create_with_groups(drm_class, dev->primary->kdev, 0,
-					  connector, connector_dev_groups,
-					  "card%d-%s", dev->primary->index,
-					  connector->name);
+	kdev = kzalloc(sizeof(*kdev), GFP_KERNEL);
+	if (!kdev)
+		return -ENOMEM;
+
+	device_initialize(kdev);
+	kdev->class = drm_class;
+	kdev->type = &drm_sysfs_device_connector;
+	kdev->parent = dev->primary->kdev;
+	kdev->groups = connector_dev_groups;
+	kdev->release = drm_sysfs_connector_release;
+	dev_set_drvdata(kdev, connector);
+
+	r = dev_set_name(kdev, "card%d-%s", dev->primary->index, connector->name);
+	if (r)
+		goto err_free;
+
 	DRM_DEBUG("adding \"%s\" to sysfs\n",
 		  connector->name);
 
-	if (IS_ERR(connector->kdev)) {
-		DRM_ERROR("failed to register connector device: %ld\n", PTR_ERR(connector->kdev));
-		return PTR_ERR(connector->kdev);
+	r = device_add(kdev);
+	if (r) {
+		DRM_ERROR("failed to register connector device: %d\n", r);
+		goto err_free;
 	}
 
+	/*
+	 * Ensure the connector object does not get free-ed if userspace still has
+	 * references open to the device through e.g. the connector sysfs-attributes.
+	 */
+	drm_connector_get(connector);
+
+	connector->kdev = kdev;
+
 	if (connector->ddc)
 		return sysfs_create_link(&connector->kdev->kobj,
 				 &connector->ddc->dev.kobj, "ddc");
 	return 0;
+
+err_free:
+	put_device(kdev);
+	return r;
 }
 
 void drm_sysfs_connector_remove(struct drm_connector *connector)
-- 
2.31.1

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

WARNING: multiple messages have this Message-ID (diff)
From: Hans de Goede <hdegoede@redhat.com>
To: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>,
	Maxime Ripard <mripard@kernel.org>,
	Thomas Zimmermann <tzimmermann@suse.de>,
	Daniel Vetter <daniel@ffwll.ch>, David Airlie <airlied@linux.ie>,
	Jani Nikula <jani.nikula@linux.intel.com>,
	Joonas Lahtinen <joonas.lahtinen@linux.intel.com>,
	Rodrigo Vivi <rodrigo.vivi@intel.com>,
	Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
	Guenter Roeck <linux@roeck-us.net>,
	Heikki Krogerus <heikki.krogerus@linux.intel.com>
Cc: intel-gfx <intel-gfx@lists.freedesktop.org>,
	linux-usb@vger.kernel.org, dri-devel@lists.freedesktop.org,
	platform-driver-x86@vger.kernel.org
Subject: [Intel-gfx] [PATCH 1/9] drm/connector: Make the drm_sysfs connector->kdev device hold a reference to the connector
Date: Wed, 28 Apr 2021 23:52:49 +0200	[thread overview]
Message-ID: <20210428215257.500088-2-hdegoede@redhat.com> (raw)
In-Reply-To: <20210428215257.500088-1-hdegoede@redhat.com>

Userspace could hold open a reference to the connector->kdev device,
through e.g. holding a sysfs-atrtribute open after
drm_sysfs_connector_remove() has been called. In this case the connector
could be free-ed while the connector->kdev device's drvdata is still
pointing to it.

Give drm_connector devices there own device type, which allows
us to specify our own release function and make drm_sysfs_connector_add()
take a reference on the connector object, and have the new release
function put the reference when the device is released.

Giving drm_connector devices there own device type, will also allow
checking if a device is a drm_connector device with a
"if (device->type == &drm_sysfs_device_connector)" check.

Note that the setting of the name member of the device_type struct will
cause udev events for drm_connector-s to now contain DEVTYPE=drm_connector
as extra info. So this extends the uevent part of the userspace API.

Signed-off-by: Hans de Goede <hdegoede@redhat.com>
---
 drivers/gpu/drm/drm_sysfs.c | 54 +++++++++++++++++++++++++++++++------
 1 file changed, 46 insertions(+), 8 deletions(-)

diff --git a/drivers/gpu/drm/drm_sysfs.c b/drivers/gpu/drm/drm_sysfs.c
index f0336c804639..c344c6d5e738 100644
--- a/drivers/gpu/drm/drm_sysfs.c
+++ b/drivers/gpu/drm/drm_sysfs.c
@@ -50,6 +50,10 @@ static struct device_type drm_sysfs_device_minor = {
 	.name = "drm_minor"
 };
 
+static struct device_type drm_sysfs_device_connector = {
+	.name = "drm_connector",
+};
+
 struct class *drm_class;
 
 static char *drm_devnode(struct device *dev, umode_t *mode)
@@ -271,30 +275,64 @@ static const struct attribute_group *connector_dev_groups[] = {
 	NULL
 };
 
+static void drm_sysfs_connector_release(struct device *dev)
+{
+	struct drm_connector *connector = to_drm_connector(dev);
+
+	drm_connector_put(connector);
+	kfree(dev);
+}
+
 int drm_sysfs_connector_add(struct drm_connector *connector)
 {
 	struct drm_device *dev = connector->dev;
+	struct device *kdev;
+	int r;
 
 	if (connector->kdev)
 		return 0;
 
-	connector->kdev =
-		device_create_with_groups(drm_class, dev->primary->kdev, 0,
-					  connector, connector_dev_groups,
-					  "card%d-%s", dev->primary->index,
-					  connector->name);
+	kdev = kzalloc(sizeof(*kdev), GFP_KERNEL);
+	if (!kdev)
+		return -ENOMEM;
+
+	device_initialize(kdev);
+	kdev->class = drm_class;
+	kdev->type = &drm_sysfs_device_connector;
+	kdev->parent = dev->primary->kdev;
+	kdev->groups = connector_dev_groups;
+	kdev->release = drm_sysfs_connector_release;
+	dev_set_drvdata(kdev, connector);
+
+	r = dev_set_name(kdev, "card%d-%s", dev->primary->index, connector->name);
+	if (r)
+		goto err_free;
+
 	DRM_DEBUG("adding \"%s\" to sysfs\n",
 		  connector->name);
 
-	if (IS_ERR(connector->kdev)) {
-		DRM_ERROR("failed to register connector device: %ld\n", PTR_ERR(connector->kdev));
-		return PTR_ERR(connector->kdev);
+	r = device_add(kdev);
+	if (r) {
+		DRM_ERROR("failed to register connector device: %d\n", r);
+		goto err_free;
 	}
 
+	/*
+	 * Ensure the connector object does not get free-ed if userspace still has
+	 * references open to the device through e.g. the connector sysfs-attributes.
+	 */
+	drm_connector_get(connector);
+
+	connector->kdev = kdev;
+
 	if (connector->ddc)
 		return sysfs_create_link(&connector->kdev->kobj,
 				 &connector->ddc->dev.kobj, "ddc");
 	return 0;
+
+err_free:
+	put_device(kdev);
+	return r;
 }
 
 void drm_sysfs_connector_remove(struct drm_connector *connector)
-- 
2.31.1

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

  reply	other threads:[~2021-04-28 21:53 UTC|newest]

Thread overview: 88+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-04-28 21:52 [PATCH 0/9] drm + usb-type-c: Add support for out-of-band hotplug notification Hans de Goede
2021-04-28 21:52 ` [Intel-gfx] " Hans de Goede
2021-04-28 21:52 ` Hans de Goede
2021-04-28 21:52 ` Hans de Goede [this message]
2021-04-28 21:52   ` [Intel-gfx] [PATCH 1/9] drm/connector: Make the drm_sysfs connector->kdev device hold a reference to the connector Hans de Goede
2021-04-28 21:52   ` Hans de Goede
2021-04-29 11:40   ` Daniel Vetter
2021-04-29 11:40     ` [Intel-gfx] " Daniel Vetter
2021-04-29 11:40     ` Daniel Vetter
2021-04-29 11:54     ` Greg Kroah-Hartman
2021-04-29 11:54       ` [Intel-gfx] " Greg Kroah-Hartman
2021-04-29 11:54       ` Greg Kroah-Hartman
2021-04-29 12:04       ` Daniel Vetter
2021-04-29 12:04         ` [Intel-gfx] " Daniel Vetter
2021-04-29 12:04         ` Daniel Vetter
2021-04-29 12:33         ` Hans de Goede
2021-04-29 12:33           ` [Intel-gfx] " Hans de Goede
2021-04-29 12:33           ` Hans de Goede
2021-04-29 19:09           ` Daniel Vetter
2021-04-29 19:09             ` [Intel-gfx] " Daniel Vetter
2021-04-29 19:09             ` Daniel Vetter
2021-04-30 11:28             ` Hans de Goede
2021-04-30 11:28               ` [Intel-gfx] " Hans de Goede
2021-04-30 11:28               ` Hans de Goede
2021-04-30 11:38               ` Daniel Vetter
2021-04-30 11:38                 ` [Intel-gfx] " Daniel Vetter
2021-04-30 11:38                 ` Daniel Vetter
2021-04-30 13:32                 ` Hans de Goede
2021-04-30 13:32                   ` [Intel-gfx] " Hans de Goede
2021-04-30 13:32                   ` Hans de Goede
2021-04-30 14:30                   ` Daniel Vetter
2021-04-30 14:30                     ` [Intel-gfx] " Daniel Vetter
2021-04-30 14:30                     ` Daniel Vetter
2021-04-30 13:45                 ` Hans de Goede
2021-04-30 13:45                   ` [Intel-gfx] " Hans de Goede
2021-04-30 13:45                   ` Hans de Goede
2021-04-29 12:30     ` Hans de Goede
2021-04-29 12:30       ` [Intel-gfx] " Hans de Goede
2021-04-29 12:30       ` Hans de Goede
2021-04-28 21:52 ` [PATCH 2/9] drm/connector: Add a fwnode pointer to drm_connector and register with ACPI Hans de Goede
2021-04-28 21:52   ` [Intel-gfx] " Hans de Goede
2021-04-28 21:52   ` Hans de Goede
2021-04-28 21:52 ` [PATCH 3/9] drm/connector: Add drm_connector_find_by_fwnode() function Hans de Goede
2021-04-28 21:52   ` [Intel-gfx] " Hans de Goede
2021-04-28 21:52   ` Hans de Goede
2021-04-28 21:52 ` [PATCH 4/9] drm/connector: Add support for out-of-band hotplug notification Hans de Goede
2021-04-28 21:52   ` [Intel-gfx] " Hans de Goede
2021-04-28 21:52   ` Hans de Goede
2021-05-03  8:00   ` Heikki Krogerus
2021-05-03  8:00     ` [Intel-gfx] " Heikki Krogerus
2021-05-03  8:00     ` Heikki Krogerus
2021-05-03 14:35     ` Hans de Goede
2021-05-03 14:35       ` [Intel-gfx] " Hans de Goede
2021-05-03 14:35       ` Hans de Goede
2021-05-04 14:52       ` Heikki Krogerus
2021-05-04 14:52         ` [Intel-gfx] " Heikki Krogerus
2021-05-04 14:52         ` Heikki Krogerus
2021-05-04 15:34         ` Hans de Goede
2021-05-04 15:34           ` [Intel-gfx] " Hans de Goede
2021-05-04 15:34           ` Hans de Goede
2021-05-04 12:53     ` Imre Deak
2021-05-04 12:53       ` [Intel-gfx] " Imre Deak
2021-05-04 12:53       ` Imre Deak
2021-04-28 21:52 ` [PATCH 5/9] drm/i915: Associate ACPI connector nodes with connector entries Hans de Goede
2021-04-28 21:52   ` [Intel-gfx] " Hans de Goede
2021-04-28 21:52   ` Hans de Goede
2021-04-28 21:52 ` [PATCH 6/9] drm/i915/dp: Add support for out-of-bound hotplug events Hans de Goede
2021-04-28 21:52   ` [Intel-gfx] " Hans de Goede
2021-04-28 21:52   ` Hans de Goede
2021-04-28 21:52 ` [PATCH 7/9] usb: typec: altmodes/displayport: Make dp_altmode_notify() more generic Hans de Goede
2021-04-28 21:52   ` [Intel-gfx] " Hans de Goede
2021-04-28 21:52   ` Hans de Goede
2021-04-28 21:52 ` [PATCH 8/9] usb: typec: altmodes/displayport: Notify drm subsys of hotplug events Hans de Goede
2021-04-28 21:52   ` [Intel-gfx] " Hans de Goede
2021-04-28 21:52   ` Hans de Goede
2021-04-29  2:08   ` kernel test robot
2021-04-29 14:16     ` Hans de Goede
2021-04-29  4:40   ` kernel test robot
2021-04-28 21:52 ` [PATCH 9/9] platform/x86/intel_cht_int33fe: Correct "displayport" fwnode reference Hans de Goede
2021-04-28 21:52   ` [Intel-gfx] " Hans de Goede
2021-04-28 21:52   ` Hans de Goede
2021-04-28 21:57 ` [PATCH 0/9] drm + usb-type-c: Add support for out-of-band hotplug notification Hans de Goede
2021-04-28 21:57   ` [Intel-gfx] " Hans de Goede
2021-04-28 21:57   ` Hans de Goede
2021-04-28 22:18 ` [Intel-gfx] ✗ Fi.CI.CHECKPATCH: warning for " Patchwork
2021-04-28 22:21 ` [Intel-gfx] ✗ Fi.CI.SPARSE: " Patchwork
2021-04-28 22:49 ` [Intel-gfx] ✓ Fi.CI.BAT: success " Patchwork
2021-04-29  0:26 ` [Intel-gfx] ✓ Fi.CI.IGT: " Patchwork

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20210428215257.500088-2-hdegoede@redhat.com \
    --to=hdegoede@redhat.com \
    --cc=airlied@linux.ie \
    --cc=daniel@ffwll.ch \
    --cc=dri-devel@lists.freedesktop.org \
    --cc=gregkh@linuxfoundation.org \
    --cc=heikki.krogerus@linux.intel.com \
    --cc=intel-gfx@lists.freedesktop.org \
    --cc=jani.nikula@linux.intel.com \
    --cc=joonas.lahtinen@linux.intel.com \
    --cc=linux-usb@vger.kernel.org \
    --cc=linux@roeck-us.net \
    --cc=maarten.lankhorst@linux.intel.com \
    --cc=mripard@kernel.org \
    --cc=platform-driver-x86@vger.kernel.org \
    --cc=rodrigo.vivi@intel.com \
    --cc=tzimmermann@suse.de \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.