All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 0/2] thunderbolt: Link USB4 ports to their USB Type-C connectors
@ 2022-04-18 17:59 Won Chung
  2022-04-18 17:59 ` [PATCH 1/2] misc/mei: Add NULL check to component match callback functions Won Chung
                   ` (2 more replies)
  0 siblings, 3 replies; 12+ messages in thread
From: Won Chung @ 2022-04-18 17:59 UTC (permalink / raw)
  To: Heikki Krogerus, Alexander Usyskin, Mika Westerberg,
	Benson Leung, Prashant Malani, Daniele Ceraolo Spurio,
	linux-kernel
  Cc: Won Chung

Currently, USB port is linked to Type C connector, using the component
framework, if they share the same _PLD fields from ACPI table. Type C
port-mapper searches for devices with the same _PLD values, and
aggregate them as components.

When there is another device that share the same _PLD but does not
registers a component, Type C connector (component master) would never
be bound due to a component match entry device without a component
registered. There exists some cases where USB4 port also shares the same
_PLD with USB port and Type C connector, so we need to register a
component for USB4 ports too, linking USB4 port with Type C connector.
Otherwise, link between USB port and Type C connector would not
work either.

Due to the nature of the component framework, all registered components
are shared by all component match despite the relevance. MEI subsystems
also use the component framework to bind to i915 driver, which try to
match components registered by USB ports and USB4 ports. This can be
problematic since MEI assumes that there is a driver bound to the
component device, while USB4 port does not bind to any drivers. MEI's
component match callback functions should handle such case to avoid NULL
pointer dereference when USB4 port registers a component.

In summary this patch series
1. Fixes MEI subsystem's component match callbacks to handle a component
device without any driver bound
2. Registers a component for USB4 ports to link them to Type C
connectors, similar to USB ports.

Heikki Krogerus (1):
  thunderbolt: Link USB4 ports to their USB Type-C connectors

Won Chung (1):
  misc/mei: Add NULL check to component match callback functions

 .../ABI/testing/sysfs-bus-thunderbolt         | 10 +++++
 drivers/misc/mei/hdcp/mei_hdcp.c              |  2 +-
 drivers/misc/mei/pxp/mei_pxp.c                |  2 +-
 drivers/thunderbolt/usb4_port.c               | 38 +++++++++++++++++++
 4 files changed, 50 insertions(+), 2 deletions(-)

-- 
2.36.0.rc0.470.gd361397f0d-goog


^ permalink raw reply	[flat|nested] 12+ messages in thread

* [PATCH 1/2] misc/mei: Add NULL check to component match callback functions
  2022-04-18 17:59 [PATCH 0/2] thunderbolt: Link USB4 ports to their USB Type-C connectors Won Chung
@ 2022-04-18 17:59 ` Won Chung
  2022-04-25 16:56   ` Won Chung
  2022-04-18 17:59 ` [PATCH 2/2] thunderbolt: Link USB4 ports to their USB Type-C connectors Won Chung
  2022-04-20 10:52 ` [PATCH 0/2] " Mika Westerberg
  2 siblings, 1 reply; 12+ messages in thread
From: Won Chung @ 2022-04-18 17:59 UTC (permalink / raw)
  To: Heikki Krogerus, Alexander Usyskin, Mika Westerberg,
	Benson Leung, Prashant Malani, Daniele Ceraolo Spurio,
	linux-kernel
  Cc: Won Chung

Currently, component_match callback functions used in mei refers to the
driver name, assuming that the component device being matched has a
driver bound. It can cause a NULL pointer dereference when a device
without a driver bound registers a component. This is due to the nature
of the component framework where all registered components are matched
in any component_match callback functions. So even if a component is
registered by a totally irrelevant device, that component is also
shared to these callbacks for i915 driver.

To prevent totally irrelevant device being matched for i915 and causing
a NULL pointer dereference for checking driver name, add a NULL check on
dev->driver to check if there is a driver bound before checking the
driver name.

In the future, the string compare on the driver name, "i915" may need to
be refactored too.

Reviewed-by: Heikki Krogerus <heikki.krogerus@linux.intel.com>
Reviewed-by: Prashant Malani <pmalani@chromium.org>
Signed-off-by: Won Chung <wonchung@google.com>
---
 drivers/misc/mei/hdcp/mei_hdcp.c | 2 +-
 drivers/misc/mei/pxp/mei_pxp.c   | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/misc/mei/hdcp/mei_hdcp.c b/drivers/misc/mei/hdcp/mei_hdcp.c
index ec2a4fce8581..e889a8bd7ac8 100644
--- a/drivers/misc/mei/hdcp/mei_hdcp.c
+++ b/drivers/misc/mei/hdcp/mei_hdcp.c
@@ -784,7 +784,7 @@ static int mei_hdcp_component_match(struct device *dev, int subcomponent,
 {
 	struct device *base = data;
 
-	if (strcmp(dev->driver->name, "i915") ||
+	if (!dev->driver || strcmp(dev->driver->name, "i915") ||
 	    subcomponent != I915_COMPONENT_HDCP)
 		return 0;
 
diff --git a/drivers/misc/mei/pxp/mei_pxp.c b/drivers/misc/mei/pxp/mei_pxp.c
index f7380d387bab..5c39457e3f53 100644
--- a/drivers/misc/mei/pxp/mei_pxp.c
+++ b/drivers/misc/mei/pxp/mei_pxp.c
@@ -131,7 +131,7 @@ static int mei_pxp_component_match(struct device *dev, int subcomponent,
 {
 	struct device *base = data;
 
-	if (strcmp(dev->driver->name, "i915") ||
+	if (!dev->driver || strcmp(dev->driver->name, "i915") ||
 	    subcomponent != I915_COMPONENT_PXP)
 		return 0;
 
-- 
2.36.0.rc0.470.gd361397f0d-goog


^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [PATCH 2/2] thunderbolt: Link USB4 ports to their USB Type-C connectors
  2022-04-18 17:59 [PATCH 0/2] thunderbolt: Link USB4 ports to their USB Type-C connectors Won Chung
  2022-04-18 17:59 ` [PATCH 1/2] misc/mei: Add NULL check to component match callback functions Won Chung
@ 2022-04-18 17:59 ` Won Chung
  2022-04-20 10:52 ` [PATCH 0/2] " Mika Westerberg
  2 siblings, 0 replies; 12+ messages in thread
From: Won Chung @ 2022-04-18 17:59 UTC (permalink / raw)
  To: Heikki Krogerus, Alexander Usyskin, Mika Westerberg,
	Benson Leung, Prashant Malani, Daniele Ceraolo Spurio,
	linux-kernel
  Cc: Won Chung

From: Heikki Krogerus <heikki.krogerus@linux.intel.com>

Creating a symlink pointing to the correct USB Type-C
connector for the on-board USB4 ports when they are created.
The link will be created only if the firmware is able to
describe the connection between the port and its connector.

Signed-off-by: Heikki Krogerus <heikki.krogerus@linux.intel.com>
Signed-off-by: Won Chung <wonchung@google.com>
---
 .../ABI/testing/sysfs-bus-thunderbolt         | 10 +++++
 drivers/thunderbolt/usb4_port.c               | 38 +++++++++++++++++++
 2 files changed, 48 insertions(+)

diff --git a/Documentation/ABI/testing/sysfs-bus-thunderbolt b/Documentation/ABI/testing/sysfs-bus-thunderbolt
index b7e87f6c7d47..f7570c240ce8 100644
--- a/Documentation/ABI/testing/sysfs-bus-thunderbolt
+++ b/Documentation/ABI/testing/sysfs-bus-thunderbolt
@@ -293,6 +293,16 @@ Contact:	thunderbolt-software@lists.01.org
 Description:	This contains XDomain service specific settings as
 		bitmask. Format: %x
 
+What:		/sys/bus/thunderbolt/devices/usb4_portX/connector
+Date:		April 2022
+Contact:	Heikki Krogerus <heikki.krogerus@linux.intel.com>
+Description:
+		Symlink to the USB Type-C connector. This link is only
+		created when USB Type-C Connector Class is enabled,
+		and only if the system firmware is capable of
+		describing the connection between a port and its
+		connector.
+
 What:		/sys/bus/thunderbolt/devices/usb4_portX/link
 Date:		Sep 2021
 KernelVersion:	v5.14
diff --git a/drivers/thunderbolt/usb4_port.c b/drivers/thunderbolt/usb4_port.c
index 29e2a4f9c9f5..6b02945624ee 100644
--- a/drivers/thunderbolt/usb4_port.c
+++ b/drivers/thunderbolt/usb4_port.c
@@ -7,9 +7,37 @@
  */
 
 #include <linux/pm_runtime.h>
+#include <linux/component.h>
+#include <linux/property.h>
 
 #include "tb.h"
 
+static int connector_bind(struct device *dev, struct device *connector, void *data)
+{
+	int ret;
+
+	ret = sysfs_create_link(&dev->kobj, &connector->kobj, "connector");
+	if (ret)
+		return ret;
+
+	ret = sysfs_create_link(&connector->kobj, &dev->kobj, dev_name(dev));
+	if (ret)
+		sysfs_remove_link(&dev->kobj, "connector");
+
+	return ret;
+}
+
+static void connector_unbind(struct device *dev, struct device *connector, void *data)
+{
+	sysfs_remove_link(&connector->kobj, dev_name(dev));
+	sysfs_remove_link(&dev->kobj, "connector");
+}
+
+static const struct component_ops connector_ops = {
+	.bind = connector_bind,
+	.unbind = connector_unbind,
+};
+
 static ssize_t link_show(struct device *dev, struct device_attribute *attr,
 			 char *buf)
 {
@@ -246,6 +274,14 @@ struct usb4_port *usb4_port_device_add(struct tb_port *port)
 		return ERR_PTR(ret);
 	}
 
+	if (dev_fwnode(&usb4->dev)) {
+		ret = component_add(&usb4->dev, &connector_ops);
+		if (ret) {
+			dev_err(&usb4->dev, "failed to add component\n");
+			device_unregister(&usb4->dev);
+		}
+	}
+
 	pm_runtime_no_callbacks(&usb4->dev);
 	pm_runtime_set_active(&usb4->dev);
 	pm_runtime_enable(&usb4->dev);
@@ -265,6 +301,8 @@ struct usb4_port *usb4_port_device_add(struct tb_port *port)
  */
 void usb4_port_device_remove(struct usb4_port *usb4)
 {
+	if (dev_fwnode(&usb4->dev))
+		component_del(&usb4->dev, &connector_ops);
 	device_unregister(&usb4->dev);
 }
 
-- 
2.36.0.rc0.470.gd361397f0d-goog


^ permalink raw reply related	[flat|nested] 12+ messages in thread

* Re: [PATCH 0/2] thunderbolt: Link USB4 ports to their USB Type-C connectors
  2022-04-18 17:59 [PATCH 0/2] thunderbolt: Link USB4 ports to their USB Type-C connectors Won Chung
  2022-04-18 17:59 ` [PATCH 1/2] misc/mei: Add NULL check to component match callback functions Won Chung
  2022-04-18 17:59 ` [PATCH 2/2] thunderbolt: Link USB4 ports to their USB Type-C connectors Won Chung
@ 2022-04-20 10:52 ` Mika Westerberg
  2022-04-20 16:39   ` Won Chung
  2 siblings, 1 reply; 12+ messages in thread
From: Mika Westerberg @ 2022-04-20 10:52 UTC (permalink / raw)
  To: Won Chung
  Cc: Heikki Krogerus, Alexander Usyskin, Benson Leung,
	Prashant Malani, Daniele Ceraolo Spurio, linux-kernel

Hi,

On Mon, Apr 18, 2022 at 05:59:30PM +0000, Won Chung wrote:
> Currently, USB port is linked to Type C connector, using the component
> framework, if they share the same _PLD fields from ACPI table. Type C
> port-mapper searches for devices with the same _PLD values, and
> aggregate them as components.
> 
> When there is another device that share the same _PLD but does not
> registers a component, Type C connector (component master) would never
> be bound due to a component match entry device without a component
> registered. There exists some cases where USB4 port also shares the same
> _PLD with USB port and Type C connector, so we need to register a
> component for USB4 ports too, linking USB4 port with Type C connector.
> Otherwise, link between USB port and Type C connector would not
> work either.
> 
> Due to the nature of the component framework, all registered components
> are shared by all component match despite the relevance. MEI subsystems
> also use the component framework to bind to i915 driver, which try to
> match components registered by USB ports and USB4 ports. This can be
> problematic since MEI assumes that there is a driver bound to the
> component device, while USB4 port does not bind to any drivers. MEI's
> component match callback functions should handle such case to avoid NULL
> pointer dereference when USB4 port registers a component.
> 
> In summary this patch series
> 1. Fixes MEI subsystem's component match callbacks to handle a component
> device without any driver bound
> 2. Registers a component for USB4 ports to link them to Type C
> connectors, similar to USB ports.
> 
> Heikki Krogerus (1):
>   thunderbolt: Link USB4 ports to their USB Type-C connectors
> 
> Won Chung (1):
>   misc/mei: Add NULL check to component match callback functions

The Thunderbolt patch looks good to me. Do you want me to take the both
patches through the Thunderbolt tree or they can go separately? I need
an ack from the mei maintainer it goes through my tree.

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH 0/2] thunderbolt: Link USB4 ports to their USB Type-C connectors
  2022-04-20 10:52 ` [PATCH 0/2] " Mika Westerberg
@ 2022-04-20 16:39   ` Won Chung
  2022-04-21  6:00     ` Mika Westerberg
  0 siblings, 1 reply; 12+ messages in thread
From: Won Chung @ 2022-04-20 16:39 UTC (permalink / raw)
  To: Mika Westerberg, Alexander Usyskin
  Cc: Heikki Krogerus, Benson Leung, Prashant Malani,
	Daniele Ceraolo Spurio, linux-kernel

On Wed, Apr 20, 2022 at 3:54 AM Mika Westerberg
<mika.westerberg@linux.intel.com> wrote:
>
> Hi,
>
> On Mon, Apr 18, 2022 at 05:59:30PM +0000, Won Chung wrote:
> > Currently, USB port is linked to Type C connector, using the component
> > framework, if they share the same _PLD fields from ACPI table. Type C
> > port-mapper searches for devices with the same _PLD values, and
> > aggregate them as components.
> >
> > When there is another device that share the same _PLD but does not
> > registers a component, Type C connector (component master) would never
> > be bound due to a component match entry device without a component
> > registered. There exists some cases where USB4 port also shares the same
> > _PLD with USB port and Type C connector, so we need to register a
> > component for USB4 ports too, linking USB4 port with Type C connector.
> > Otherwise, link between USB port and Type C connector would not
> > work either.
> >
> > Due to the nature of the component framework, all registered components
> > are shared by all component match despite the relevance. MEI subsystems
> > also use the component framework to bind to i915 driver, which try to
> > match components registered by USB ports and USB4 ports. This can be
> > problematic since MEI assumes that there is a driver bound to the
> > component device, while USB4 port does not bind to any drivers. MEI's
> > component match callback functions should handle such case to avoid NULL
> > pointer dereference when USB4 port registers a component.
> >
> > In summary this patch series
> > 1. Fixes MEI subsystem's component match callbacks to handle a component
> > device without any driver bound
> > 2. Registers a component for USB4 ports to link them to Type C
> > connectors, similar to USB ports.
> >
> > Heikki Krogerus (1):
> >   thunderbolt: Link USB4 ports to their USB Type-C connectors
> >
> > Won Chung (1):
> >   misc/mei: Add NULL check to component match callback functions
>
> The Thunderbolt patch looks good to me. Do you want me to take the both
> patches through the Thunderbolt tree or they can go separately? I need
> an ack from the mei maintainer it goes through my tree.

Hi Mika,

I would prefer the two patches to go to the same tree since it can
cause a crash with only the second patch (tbt). Would that sound okay?

Hey Alexander, would you be the right person to ask for an ack on this?

Thanks,
Won

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH 0/2] thunderbolt: Link USB4 ports to their USB Type-C connectors
  2022-04-20 16:39   ` Won Chung
@ 2022-04-21  6:00     ` Mika Westerberg
  2022-04-24  5:12       ` Usyskin, Alexander
  0 siblings, 1 reply; 12+ messages in thread
From: Mika Westerberg @ 2022-04-21  6:00 UTC (permalink / raw)
  To: Won Chung
  Cc: Alexander Usyskin, Heikki Krogerus, Benson Leung,
	Prashant Malani, Daniele Ceraolo Spurio, linux-kernel

Hi,

On Wed, Apr 20, 2022 at 09:39:25AM -0700, Won Chung wrote:
> On Wed, Apr 20, 2022 at 3:54 AM Mika Westerberg
> <mika.westerberg@linux.intel.com> wrote:
> >
> > Hi,
> >
> > On Mon, Apr 18, 2022 at 05:59:30PM +0000, Won Chung wrote:
> > > Currently, USB port is linked to Type C connector, using the component
> > > framework, if they share the same _PLD fields from ACPI table. Type C
> > > port-mapper searches for devices with the same _PLD values, and
> > > aggregate them as components.
> > >
> > > When there is another device that share the same _PLD but does not
> > > registers a component, Type C connector (component master) would never
> > > be bound due to a component match entry device without a component
> > > registered. There exists some cases where USB4 port also shares the same
> > > _PLD with USB port and Type C connector, so we need to register a
> > > component for USB4 ports too, linking USB4 port with Type C connector.
> > > Otherwise, link between USB port and Type C connector would not
> > > work either.
> > >
> > > Due to the nature of the component framework, all registered components
> > > are shared by all component match despite the relevance. MEI subsystems
> > > also use the component framework to bind to i915 driver, which try to
> > > match components registered by USB ports and USB4 ports. This can be
> > > problematic since MEI assumes that there is a driver bound to the
> > > component device, while USB4 port does not bind to any drivers. MEI's
> > > component match callback functions should handle such case to avoid NULL
> > > pointer dereference when USB4 port registers a component.
> > >
> > > In summary this patch series
> > > 1. Fixes MEI subsystem's component match callbacks to handle a component
> > > device without any driver bound
> > > 2. Registers a component for USB4 ports to link them to Type C
> > > connectors, similar to USB ports.
> > >
> > > Heikki Krogerus (1):
> > >   thunderbolt: Link USB4 ports to their USB Type-C connectors
> > >
> > > Won Chung (1):
> > >   misc/mei: Add NULL check to component match callback functions
> >
> > The Thunderbolt patch looks good to me. Do you want me to take the both
> > patches through the Thunderbolt tree or they can go separately? I need
> > an ack from the mei maintainer it goes through my tree.
> 
> Hi Mika,
> 
> I would prefer the two patches to go to the same tree since it can
> cause a crash with only the second patch (tbt). Would that sound okay?

Sounds good to me.

^ permalink raw reply	[flat|nested] 12+ messages in thread

* RE: [PATCH 0/2] thunderbolt: Link USB4 ports to their USB Type-C connectors
  2022-04-21  6:00     ` Mika Westerberg
@ 2022-04-24  5:12       ` Usyskin, Alexander
  2022-04-25 16:51         ` Won Chung
  0 siblings, 1 reply; 12+ messages in thread
From: Usyskin, Alexander @ 2022-04-24  5:12 UTC (permalink / raw)
  To: Mika Westerberg, Won Chung, Winkler, Tomas
  Cc: Heikki Krogerus, Benson Leung, Prashant Malani, Ceraolo Spurio,
	Daniele, linux-kernel

> > > > Currently, USB port is linked to Type C connector, using the component
> > > > framework, if they share the same _PLD fields from ACPI table. Type C
> > > > port-mapper searches for devices with the same _PLD values, and
> > > > aggregate them as components.
> > > >
> > > > When there is another device that share the same _PLD but does not
> > > > registers a component, Type C connector (component master) would never
> > > > be bound due to a component match entry device without a component
> > > > registered. There exists some cases where USB4 port also shares the same
> > > > _PLD with USB port and Type C connector, so we need to register a
> > > > component for USB4 ports too, linking USB4 port with Type C connector.
> > > > Otherwise, link between USB port and Type C connector would not
> > > > work either.
> > > >
> > > > Due to the nature of the component framework, all registered components
> > > > are shared by all component match despite the relevance. MEI subsystems
> > > > also use the component framework to bind to i915 driver, which try to
> > > > match components registered by USB ports and USB4 ports. This can be
> > > > problematic since MEI assumes that there is a driver bound to the
> > > > component device, while USB4 port does not bind to any drivers. MEI's
> > > > component match callback functions should handle such case to avoid
> NULL
> > > > pointer dereference when USB4 port registers a component.
> > > >
> > > > In summary this patch series
> > > > 1. Fixes MEI subsystem's component match callbacks to handle a
> component
> > > > device without any driver bound
> > > > 2. Registers a component for USB4 ports to link them to Type C
> > > > connectors, similar to USB ports.
> > > >
> > > > Heikki Krogerus (1):
> > > >   thunderbolt: Link USB4 ports to their USB Type-C connectors
> > > >
> > > > Won Chung (1):
> > > >   misc/mei: Add NULL check to component match callback functions
> > >
> > > The Thunderbolt patch looks good to me. Do you want me to take the both
> > > patches through the Thunderbolt tree or they can go separately? I need
> > > an ack from the mei maintainer it goes through my tree.
> >
> > Hi Mika,
> >
> > I would prefer the two patches to go to the same tree since it can
> > cause a crash with only the second patch (tbt). Would that sound okay?
> 
> Sounds good to me.

Tomas is MEI maintainer and should ack MEI patch

-- 
Thanks,
Sasha



^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH 0/2] thunderbolt: Link USB4 ports to their USB Type-C connectors
  2022-04-24  5:12       ` Usyskin, Alexander
@ 2022-04-25 16:51         ` Won Chung
  2022-04-28 18:34           ` Won Chung
  0 siblings, 1 reply; 12+ messages in thread
From: Won Chung @ 2022-04-25 16:51 UTC (permalink / raw)
  To: Usyskin, Alexander
  Cc: Mika Westerberg, Winkler, Tomas, Heikki Krogerus, Benson Leung,
	Prashant Malani, Ceraolo Spurio, Daniele, linux-kernel

On Sun, Apr 24, 2022 at 2:12 PM Usyskin, Alexander
<alexander.usyskin@intel.com> wrote:
>
> > > > > Currently, USB port is linked to Type C connector, using the component
> > > > > framework, if they share the same _PLD fields from ACPI table. Type C
> > > > > port-mapper searches for devices with the same _PLD values, and
> > > > > aggregate them as components.
> > > > >
> > > > > When there is another device that share the same _PLD but does not
> > > > > registers a component, Type C connector (component master) would never
> > > > > be bound due to a component match entry device without a component
> > > > > registered. There exists some cases where USB4 port also shares the same
> > > > > _PLD with USB port and Type C connector, so we need to register a
> > > > > component for USB4 ports too, linking USB4 port with Type C connector.
> > > > > Otherwise, link between USB port and Type C connector would not
> > > > > work either.
> > > > >
> > > > > Due to the nature of the component framework, all registered components
> > > > > are shared by all component match despite the relevance. MEI subsystems
> > > > > also use the component framework to bind to i915 driver, which try to
> > > > > match components registered by USB ports and USB4 ports. This can be
> > > > > problematic since MEI assumes that there is a driver bound to the
> > > > > component device, while USB4 port does not bind to any drivers. MEI's
> > > > > component match callback functions should handle such case to avoid
> > NULL
> > > > > pointer dereference when USB4 port registers a component.
> > > > >
> > > > > In summary this patch series
> > > > > 1. Fixes MEI subsystem's component match callbacks to handle a
> > component
> > > > > device without any driver bound
> > > > > 2. Registers a component for USB4 ports to link them to Type C
> > > > > connectors, similar to USB ports.
> > > > >
> > > > > Heikki Krogerus (1):
> > > > >   thunderbolt: Link USB4 ports to their USB Type-C connectors
> > > > >
> > > > > Won Chung (1):
> > > > >   misc/mei: Add NULL check to component match callback functions
> > > >
> > > > The Thunderbolt patch looks good to me. Do you want me to take the both
> > > > patches through the Thunderbolt tree or they can go separately? I need
> > > > an ack from the mei maintainer it goes through my tree.
> > >
> > > Hi Mika,
> > >
> > > I would prefer the two patches to go to the same tree since it can
> > > cause a crash with only the second patch (tbt). Would that sound okay?
> >
> > Sounds good to me.
>
> Tomas is MEI maintainer and should ack MEI patch
>
> --
> Thanks,
> Sasha
>
>

Hi Tomas,

Can you take a look at this patch series and give an ack if it looks
okay to be merged into Mika's tree?
One of the patches adds NULL checks in MEI:
https://lore.kernel.org/all/20220418175932.1809770-2-wonchung@google.com/

Thank you,
Won

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH 1/2] misc/mei: Add NULL check to component match callback functions
  2022-04-18 17:59 ` [PATCH 1/2] misc/mei: Add NULL check to component match callback functions Won Chung
@ 2022-04-25 16:56   ` Won Chung
  2022-04-28 18:11     ` Winkler, Tomas
  0 siblings, 1 reply; 12+ messages in thread
From: Won Chung @ 2022-04-25 16:56 UTC (permalink / raw)
  To: Heikki Krogerus, Alexander Usyskin, Mika Westerberg,
	Benson Leung, Prashant Malani, Daniele Ceraolo Spurio,
	linux-kernel, Winkler, Tomas

On Tue, Apr 19, 2022 at 2:59 AM Won Chung <wonchung@google.com> wrote:
>
> Currently, component_match callback functions used in mei refers to the
> driver name, assuming that the component device being matched has a
> driver bound. It can cause a NULL pointer dereference when a device
> without a driver bound registers a component. This is due to the nature
> of the component framework where all registered components are matched
> in any component_match callback functions. So even if a component is
> registered by a totally irrelevant device, that component is also
> shared to these callbacks for i915 driver.
>
> To prevent totally irrelevant device being matched for i915 and causing
> a NULL pointer dereference for checking driver name, add a NULL check on
> dev->driver to check if there is a driver bound before checking the
> driver name.
>
> In the future, the string compare on the driver name, "i915" may need to
> be refactored too.
>
> Reviewed-by: Heikki Krogerus <heikki.krogerus@linux.intel.com>
> Reviewed-by: Prashant Malani <pmalani@chromium.org>
> Signed-off-by: Won Chung <wonchung@google.com>
> ---
>  drivers/misc/mei/hdcp/mei_hdcp.c | 2 +-
>  drivers/misc/mei/pxp/mei_pxp.c   | 2 +-
>  2 files changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/misc/mei/hdcp/mei_hdcp.c b/drivers/misc/mei/hdcp/mei_hdcp.c
> index ec2a4fce8581..e889a8bd7ac8 100644
> --- a/drivers/misc/mei/hdcp/mei_hdcp.c
> +++ b/drivers/misc/mei/hdcp/mei_hdcp.c
> @@ -784,7 +784,7 @@ static int mei_hdcp_component_match(struct device *dev, int subcomponent,
>  {
>         struct device *base = data;
>
> -       if (strcmp(dev->driver->name, "i915") ||
> +       if (!dev->driver || strcmp(dev->driver->name, "i915") ||
>             subcomponent != I915_COMPONENT_HDCP)
>                 return 0;
>
> diff --git a/drivers/misc/mei/pxp/mei_pxp.c b/drivers/misc/mei/pxp/mei_pxp.c
> index f7380d387bab..5c39457e3f53 100644
> --- a/drivers/misc/mei/pxp/mei_pxp.c
> +++ b/drivers/misc/mei/pxp/mei_pxp.c
> @@ -131,7 +131,7 @@ static int mei_pxp_component_match(struct device *dev, int subcomponent,
>  {
>         struct device *base = data;
>
> -       if (strcmp(dev->driver->name, "i915") ||
> +       if (!dev->driver || strcmp(dev->driver->name, "i915") ||
>             subcomponent != I915_COMPONENT_PXP)
>                 return 0;
>
> --
> 2.36.0.rc0.470.gd361397f0d-goog
>

Hi Tomas,

I am adding you to this patch since you are the maintainer of MEI.
If this looks okay to you, could you also take a look at the comment
thread on the cover letter and give an ack if it is okay to be merged
into thunderbolt tree?
https://lore.kernel.org/all/20220418175932.1809770-1-wonchung@google.com/

Thank you,
Won

^ permalink raw reply	[flat|nested] 12+ messages in thread

* RE: [PATCH 1/2] misc/mei: Add NULL check to component match callback functions
  2022-04-25 16:56   ` Won Chung
@ 2022-04-28 18:11     ` Winkler, Tomas
  0 siblings, 0 replies; 12+ messages in thread
From: Winkler, Tomas @ 2022-04-28 18:11 UTC (permalink / raw)
  To: Won Chung, Heikki Krogerus, Usyskin, Alexander, Mika Westerberg,
	Benson Leung, Prashant Malani, Ceraolo Spurio, Daniele,
	linux-kernel

> 
> On Tue, Apr 19, 2022 at 2:59 AM Won Chung <wonchung@google.com>
> wrote:
> >
> > Currently, component_match callback functions used in mei refers to
> > the driver name, assuming that the component device being matched has
> > a driver bound. It can cause a NULL pointer dereference when a device
> > without a driver bound registers a component. This is due to the
> > nature of the component framework where all registered components are
> > matched in any component_match callback functions. So even if a
> > component is registered by a totally irrelevant device, that component
> > is also shared to these callbacks for i915 driver.
> >
> > To prevent totally irrelevant device being matched for i915 and
> > causing a NULL pointer dereference for checking driver name, add a
> > NULL check on
> > dev->driver to check if there is a driver bound before checking the
> > driver name.
> >
> > In the future, the string compare on the driver name, "i915" may need
> > to be refactored too.
> >
> > Reviewed-by: Heikki Krogerus <heikki.krogerus@linux.intel.com>
> > Reviewed-by: Prashant Malani <pmalani@chromium.org>
> > Signed-off-by: Won Chung <wonchung@google.com>
Acked-by: Tomas Winkler <tomas.winkler@intel.com> 

> > ---
> >  drivers/misc/mei/hdcp/mei_hdcp.c | 2 +-
> >  drivers/misc/mei/pxp/mei_pxp.c   | 2 +-
> >  2 files changed, 2 insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/misc/mei/hdcp/mei_hdcp.c
> > b/drivers/misc/mei/hdcp/mei_hdcp.c
> > index ec2a4fce8581..e889a8bd7ac8 100644
> > --- a/drivers/misc/mei/hdcp/mei_hdcp.c
> > +++ b/drivers/misc/mei/hdcp/mei_hdcp.c
> > @@ -784,7 +784,7 @@ static int mei_hdcp_component_match(struct
> device
> > *dev, int subcomponent,  {
> >         struct device *base = data;
> >
> > -       if (strcmp(dev->driver->name, "i915") ||
> > +       if (!dev->driver || strcmp(dev->driver->name, "i915") ||
> >             subcomponent != I915_COMPONENT_HDCP)
> >                 return 0;
> >
> > diff --git a/drivers/misc/mei/pxp/mei_pxp.c
> > b/drivers/misc/mei/pxp/mei_pxp.c index f7380d387bab..5c39457e3f53
> > 100644
> > --- a/drivers/misc/mei/pxp/mei_pxp.c
> > +++ b/drivers/misc/mei/pxp/mei_pxp.c
> > @@ -131,7 +131,7 @@ static int mei_pxp_component_match(struct device
> > *dev, int subcomponent,  {
> >         struct device *base = data;
> >
> > -       if (strcmp(dev->driver->name, "i915") ||
> > +       if (!dev->driver || strcmp(dev->driver->name, "i915") ||
> >             subcomponent != I915_COMPONENT_PXP)
> >                 return 0;
> >
> > --
> > 2.36.0.rc0.470.gd361397f0d-goog
> >
> 
> Hi Tomas,
> 
> I am adding you to this patch since you are the maintainer of MEI.
> If this looks okay to you, could you also take a look at the comment thread on
> the cover letter and give an ack if it is okay to be merged into thunderbolt
> tree?
> https://lore.kernel.org/all/20220418175932.1809770-1-
> wonchung@google.com/
> 
> Thank you,
> Won

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH 0/2] thunderbolt: Link USB4 ports to their USB Type-C connectors
  2022-04-25 16:51         ` Won Chung
@ 2022-04-28 18:34           ` Won Chung
  2022-04-29  9:28             ` Mika Westerberg
  0 siblings, 1 reply; 12+ messages in thread
From: Won Chung @ 2022-04-28 18:34 UTC (permalink / raw)
  To: Mika Westerberg
  Cc: Winkler, Tomas, Usyskin, Alexander, Heikki Krogerus,
	Benson Leung, Prashant Malani, Ceraolo Spurio, Daniele,
	linux-kernel

On Tue, Apr 26, 2022 at 1:51 AM Won Chung <wonchung@google.com> wrote:
>
> On Sun, Apr 24, 2022 at 2:12 PM Usyskin, Alexander
> <alexander.usyskin@intel.com> wrote:
> >
> > > > > > Currently, USB port is linked to Type C connector, using the component
> > > > > > framework, if they share the same _PLD fields from ACPI table. Type C
> > > > > > port-mapper searches for devices with the same _PLD values, and
> > > > > > aggregate them as components.
> > > > > >
> > > > > > When there is another device that share the same _PLD but does not
> > > > > > registers a component, Type C connector (component master) would never
> > > > > > be bound due to a component match entry device without a component
> > > > > > registered. There exists some cases where USB4 port also shares the same
> > > > > > _PLD with USB port and Type C connector, so we need to register a
> > > > > > component for USB4 ports too, linking USB4 port with Type C connector.
> > > > > > Otherwise, link between USB port and Type C connector would not
> > > > > > work either.
> > > > > >
> > > > > > Due to the nature of the component framework, all registered components
> > > > > > are shared by all component match despite the relevance. MEI subsystems
> > > > > > also use the component framework to bind to i915 driver, which try to
> > > > > > match components registered by USB ports and USB4 ports. This can be
> > > > > > problematic since MEI assumes that there is a driver bound to the
> > > > > > component device, while USB4 port does not bind to any drivers. MEI's
> > > > > > component match callback functions should handle such case to avoid
> > > NULL
> > > > > > pointer dereference when USB4 port registers a component.
> > > > > >
> > > > > > In summary this patch series
> > > > > > 1. Fixes MEI subsystem's component match callbacks to handle a
> > > component
> > > > > > device without any driver bound
> > > > > > 2. Registers a component for USB4 ports to link them to Type C
> > > > > > connectors, similar to USB ports.
> > > > > >
> > > > > > Heikki Krogerus (1):
> > > > > >   thunderbolt: Link USB4 ports to their USB Type-C connectors
> > > > > >
> > > > > > Won Chung (1):
> > > > > >   misc/mei: Add NULL check to component match callback functions
> > > > >
> > > > > The Thunderbolt patch looks good to me. Do you want me to take the both
> > > > > patches through the Thunderbolt tree or they can go separately? I need
> > > > > an ack from the mei maintainer it goes through my tree.
> > > >
> > > > Hi Mika,
> > > >
> > > > I would prefer the two patches to go to the same tree since it can
> > > > cause a crash with only the second patch (tbt). Would that sound okay?
> > >
> > > Sounds good to me.
> >
> > Tomas is MEI maintainer and should ack MEI patch
> >
> > --
> > Thanks,
> > Sasha
> >
> >
>
> Hi Tomas,
>
> Can you take a look at this patch series and give an ack if it looks
> okay to be merged into Mika's tree?
> One of the patches adds NULL checks in MEI:
> https://lore.kernel.org/all/20220418175932.1809770-2-wonchung@google.com/
>
> Thank you,
> Won

Hi Mika,

Tomas has given an ack on the first patch on MEI:
https://lore.kernel.org/all/0136fcb26ca8433899593208af4351c9@intel.com/

Should I resend the patch with an additional "Acked-by" tag?
Or could that be added as the patch is merged into the thunderbolt tree?

Thank you,
Won

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH 0/2] thunderbolt: Link USB4 ports to their USB Type-C connectors
  2022-04-28 18:34           ` Won Chung
@ 2022-04-29  9:28             ` Mika Westerberg
  0 siblings, 0 replies; 12+ messages in thread
From: Mika Westerberg @ 2022-04-29  9:28 UTC (permalink / raw)
  To: Won Chung
  Cc: Winkler, Tomas, Usyskin, Alexander, Heikki Krogerus,
	Benson Leung, Prashant Malani, Ceraolo Spurio, Daniele,
	linux-kernel

Hi,

On Fri, Apr 29, 2022 at 03:34:56AM +0900, Won Chung wrote:
> Hi Mika,
> 
> Tomas has given an ack on the first patch on MEI:
> https://lore.kernel.org/all/0136fcb26ca8433899593208af4351c9@intel.com/
> 
> Should I resend the patch with an additional "Acked-by" tag?
> Or could that be added as the patch is merged into the thunderbolt tree?

Both applied to thunderbolt.git/next, thanks!

^ permalink raw reply	[flat|nested] 12+ messages in thread

end of thread, other threads:[~2022-04-29  9:32 UTC | newest]

Thread overview: 12+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-04-18 17:59 [PATCH 0/2] thunderbolt: Link USB4 ports to their USB Type-C connectors Won Chung
2022-04-18 17:59 ` [PATCH 1/2] misc/mei: Add NULL check to component match callback functions Won Chung
2022-04-25 16:56   ` Won Chung
2022-04-28 18:11     ` Winkler, Tomas
2022-04-18 17:59 ` [PATCH 2/2] thunderbolt: Link USB4 ports to their USB Type-C connectors Won Chung
2022-04-20 10:52 ` [PATCH 0/2] " Mika Westerberg
2022-04-20 16:39   ` Won Chung
2022-04-21  6:00     ` Mika Westerberg
2022-04-24  5:12       ` Usyskin, Alexander
2022-04-25 16:51         ` Won Chung
2022-04-28 18:34           ` Won Chung
2022-04-29  9:28             ` Mika Westerberg

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.