2022-03-31 02:42:31

by Won Chung

[permalink] [raw]
Subject: [PATCH] misc/mei: Add NULL check to component match callback functions

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.

Signed-off-by: Heikki Krogerus <[email protected]>
Signed-off-by: Mika Westerberg <[email protected]>
Signed-off-by: Won Chung <[email protected]>
---
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") ||
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") ||
subcomponent != I915_COMPONENT_PXP)
return 0;

--
2.35.1.1021.g381101b075-goog


2022-03-31 02:54:20

by Won Chung

[permalink] [raw]
Subject: Re: [PATCH] misc/mei: Add NULL check to component match callback functions

On Wed, Mar 30, 2022 at 11:18 AM Prashant Malani <[email protected]> wrote:
>
> Hi Won,
>
> On Wed, 30 Mar 2022 at 10:58, Won Chung <[email protected]> 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.
>>
> IMO this should have a "Fixes" tag, and be picked back to stable branches.
> Also, please use my chromium.org account ([email protected]) for upstream communications.
>
> Thanks!
>>
>> Signed-off-by: Heikki Krogerus <[email protected]>
>> Signed-off-by: Mika Westerberg <[email protected]>
>> Signed-off-by: Won Chung <[email protected]>
>> ---
>> 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") ||
>> 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") ||
>> subcomponent != I915_COMPONENT_PXP)
>> return 0;
>>
>> --
>> 2.35.1.1021.g381101b075-goog
>>
>
>
> --
> -Prashant

Hi Prashant,

This currently does not fix a patch in the upstream, but is for a
future patch of adding component_add to usb4_port. Would we need the
"Fixes" tag for a future patch too?
Thinking again, I think it might be a better idea to have this as a
series of patches along with the patch to be sent after this one.

I changed the recipient email to send this to your chromium.org
account. Sorry for that.

Thanks,
Won

2022-03-31 03:12:19

by Benson Leung

[permalink] [raw]
Subject: Re: [PATCH] misc/mei: Add NULL check to component match callback functions

Hi Won,

On Wed, Mar 30, 2022 at 11:43:33AM -0700, Won Chung wrote:
> On Wed, Mar 30, 2022 at 11:18 AM Prashant Malani <[email protected]> wrote:
> >
> > Hi Won,
> >
> > On Wed, 30 Mar 2022 at 10:58, Won Chung <[email protected]> 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.
> >>
> > IMO this should have a "Fixes" tag, and be picked back to stable branches.
> > Also, please use my chromium.org account ([email protected]) for upstream communications.
> >
> > Thanks!
>
> Hi Prashant,
>
> This currently does not fix a patch in the upstream, but is for a
> future patch of adding component_add to usb4_port. Would we need the
> "Fixes" tag for a future patch too?

I believe it is considered a fix to an original patch. Won, you should go
back through git blame of this file to see which original commit originally
added the component match callback in this and the other files.


> Thinking again, I think it might be a better idea to have this as a
> series of patches along with the patch to be sent after this one.
>

I think these are focused enough that you don't need to send them in series.

> I changed the recipient email to send this to your chromium.org
> account. Sorry for that.
>
> Thanks,
> Won

Thanks,
Benson

--
Benson Leung
Staff Software Engineer
Chrome OS Kernel
Google Inc.
[email protected]
Chromium OS Project
[email protected]


Attachments:
(No filename) (1.62 kB)
signature.asc (235.00 B)
Download all attachments