On Thu, Mar 31, 2022 at 12:32:24PM +0300, Heikki Krogerus wrote:
On Thu, Mar 31, 2022 at 11:00:46AM +0200, Greg KH wrote:
On Thu, Mar 31, 2022 at 08:49:18AM +0000, Won Chung wrote:
Component match callback functions need to check if expected data is passed to them. Without this check, it can cause a NULL pointer dereference when another driver registers a component before i915 drivers have their component master fully bind.
How can that happen in a real system? Or does this just happen for when you are doing development and testing?
Fixes: 1e8d19d9b0dfc ("mei: hdcp: bind only with i915 on the same PCH") Fixes: c2004ce99ed73 ("mei: pxp: export pavp client to me client bus") Suggested-by: Heikki Krogerus heikki.krogerus@linux.intel.com Suggested-by: Mika Westerberg mika.westerberg@linux.intel.com Signed-off-by: Won Chung wonchung@google.com Cc: stable@vger.kernel.org
Why does this need to go to stable? How can this be triggered in older kernels?
Changes from v2:
- Correctly add "Suggested-by" tag
- Add "Cc: stable@vger.kernel.org"
Changes from v1:
- Add "Fixes" tag
- Send to stable@vger.kernel.org
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..843dbc2b21b1 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 (!base || !dev->driver || strcmp(dev->driver->name, "i915") ||
How can base be NULL?
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..e32a81da8af6 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 (!base || !dev->driver || strcmp(dev->driver->name, "i915") ||
Same here, shouldn't this be caught by the driver core or bus and match should not be called?
Why not fix this in the component/driver core instead?
A component is just a device that is declared to be a "component", and the code that declares it as component does not have to be the driver of that device. You simply can't assume that it's bind to a driver like this function does.
In our case the "components" are USB ports, so devices that are never bind to drivers.
And going off of the driver name is sane? That feels ripe for bugs and problems in the future, but hey, I don't understand the need for this driver to care about another driver at all.
And why is a USB device being passed to something that it thinks is a PCI device? That too feels really wrong and ripe for problems.
thanks,
greg k-h