* [PATCH 1/3] drm/msm/dsi: check src_pll for null in dsi manager
2018-01-12 20:55 [PATCH 0/3] drm/msm/dsi: improve pointer validation checks Lloyd Atkinson
@ 2018-01-12 20:55 ` Lloyd Atkinson
2018-01-15 14:40 ` Rob Clark
2018-01-12 20:55 ` [PATCH 2/3] drm/msm/dsi: correct DSI id bounds check during registration Lloyd Atkinson
` (5 subsequent siblings)
6 siblings, 1 reply; 13+ messages in thread
From: Lloyd Atkinson @ 2018-01-12 20:55 UTC (permalink / raw)
To: dri-devel; +Cc: linux-arm-msm, Lloyd Atkinson
Add checks for failure after retrieving the src_pll, since it
may fail. This prevents an invalid pointer dereference later in
msm_dsi_pll_get_clk_provider.
Signed-off-by: Lloyd Atkinson <latkinso@codeaurora.org>
---
drivers/gpu/drm/msm/dsi/dsi_manager.c | 4 ++++
1 file changed, 4 insertions(+)
diff --git a/drivers/gpu/drm/msm/dsi/dsi_manager.c b/drivers/gpu/drm/msm/dsi/dsi_manager.c
index 8552481..d276358 100644
--- a/drivers/gpu/drm/msm/dsi/dsi_manager.c
+++ b/drivers/gpu/drm/msm/dsi/dsi_manager.c
@@ -88,6 +88,8 @@ static int dsi_mgr_setup_components(int id)
msm_dsi_phy_set_usecase(msm_dsi->phy, MSM_DSI_PHY_STANDALONE);
src_pll = msm_dsi_phy_get_pll(msm_dsi->phy);
+ if (!src_pll)
+ return -EINVAL;
ret = msm_dsi_host_set_src_pll(msm_dsi->host, src_pll);
} else if (!other_dsi) {
ret = 0;
@@ -116,6 +118,8 @@ static int dsi_mgr_setup_components(int id)
msm_dsi_phy_set_usecase(clk_slave_dsi->phy,
MSM_DSI_PHY_SLAVE);
src_pll = msm_dsi_phy_get_pll(clk_master_dsi->phy);
+ if (!src_pll)
+ return -EINVAL;
ret = msm_dsi_host_set_src_pll(msm_dsi->host, src_pll);
if (ret)
return ret;
--
QUALCOMM Canada, on behalf of Qualcomm Innovation Center, Inc. is a member
of Code Aurora Forum, hosted by The Linux Foundation
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply related [flat|nested] 13+ messages in thread* Re: [PATCH 1/3] drm/msm/dsi: check src_pll for null in dsi manager
2018-01-12 20:55 ` [PATCH 1/3] drm/msm/dsi: check src_pll for null in dsi manager Lloyd Atkinson
@ 2018-01-15 14:40 ` Rob Clark
0 siblings, 0 replies; 13+ messages in thread
From: Rob Clark @ 2018-01-15 14:40 UTC (permalink / raw)
To: Lloyd Atkinson; +Cc: dri-devel, linux-arm-msm
On Fri, Jan 12, 2018 at 3:55 PM, Lloyd Atkinson <latkinso@codeaurora.org> wrote:
> Add checks for failure after retrieving the src_pll, since it
> may fail. This prevents an invalid pointer dereference later in
> msm_dsi_pll_get_clk_provider.
>
> Signed-off-by: Lloyd Atkinson <latkinso@codeaurora.org>
> ---
> drivers/gpu/drm/msm/dsi/dsi_manager.c | 4 ++++
> 1 file changed, 4 insertions(+)
>
> diff --git a/drivers/gpu/drm/msm/dsi/dsi_manager.c b/drivers/gpu/drm/msm/dsi/dsi_manager.c
> index 8552481..d276358 100644
> --- a/drivers/gpu/drm/msm/dsi/dsi_manager.c
> +++ b/drivers/gpu/drm/msm/dsi/dsi_manager.c
> @@ -88,6 +88,8 @@ static int dsi_mgr_setup_components(int id)
>
> msm_dsi_phy_set_usecase(msm_dsi->phy, MSM_DSI_PHY_STANDALONE);
> src_pll = msm_dsi_phy_get_pll(msm_dsi->phy);
> + if (!src_pll)
> + return -EINVAL;
hmm, this is a bit awkward, and probably something that we should have
noticed/fixed by now.. but in case CONFIG_DRM_MSM_DSI_PLL is not
enabled, msm_dsi_pll_init() returns ERR_PTR(-ENODEV). But if it is
enabled, then error paths return NULL.
Probably we should fix the enabled case to propagate back
ERR_PTR(errno) and never return NULL, and then use IS_ERR() here.
(I guess also IS_ERR_OR_NULL() here would do the job.. but perhaps
best to fix the root issue)
BR,
-R
> ret = msm_dsi_host_set_src_pll(msm_dsi->host, src_pll);
> } else if (!other_dsi) {
> ret = 0;
> @@ -116,6 +118,8 @@ static int dsi_mgr_setup_components(int id)
> msm_dsi_phy_set_usecase(clk_slave_dsi->phy,
> MSM_DSI_PHY_SLAVE);
> src_pll = msm_dsi_phy_get_pll(clk_master_dsi->phy);
> + if (!src_pll)
> + return -EINVAL;
> ret = msm_dsi_host_set_src_pll(msm_dsi->host, src_pll);
> if (ret)
> return ret;
> --
> QUALCOMM Canada, on behalf of Qualcomm Innovation Center, Inc. is a member
> of Code Aurora Forum, hosted by The Linux Foundation
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-arm-msm" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply [flat|nested] 13+ messages in thread
* [PATCH 2/3] drm/msm/dsi: correct DSI id bounds check during registration
2018-01-12 20:55 [PATCH 0/3] drm/msm/dsi: improve pointer validation checks Lloyd Atkinson
2018-01-12 20:55 ` [PATCH 1/3] drm/msm/dsi: check src_pll for null in dsi manager Lloyd Atkinson
@ 2018-01-12 20:55 ` Lloyd Atkinson
2018-01-15 14:44 ` Rob Clark
2018-01-12 20:55 ` [PATCH 3/3] drm/msm/dsi: check msm_dsi and dsi pointers before use Lloyd Atkinson
` (4 subsequent siblings)
6 siblings, 1 reply; 13+ messages in thread
From: Lloyd Atkinson @ 2018-01-12 20:55 UTC (permalink / raw)
To: dri-devel; +Cc: Lloyd Atkinson, linux-arm-msm
Check DSI instance id argument against the proper boundary size
to protect against invalid configuration of the DSI id.
Signed-off-by: Lloyd Atkinson <latkinso@codeaurora.org>
---
drivers/gpu/drm/msm/dsi/dsi_manager.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/msm/dsi/dsi_manager.c b/drivers/gpu/drm/msm/dsi/dsi_manager.c
index d276358..dec2f74 100644
--- a/drivers/gpu/drm/msm/dsi/dsi_manager.c
+++ b/drivers/gpu/drm/msm/dsi/dsi_manager.c
@@ -862,7 +862,7 @@ int msm_dsi_manager_register(struct msm_dsi *msm_dsi)
int id = msm_dsi->id;
int ret;
- if (id > DSI_MAX) {
+ if (id >= DSI_MAX) {
pr_err("%s: invalid id %d\n", __func__, id);
return -EINVAL;
}
--
QUALCOMM Canada, on behalf of Qualcomm Innovation Center, Inc. is a member
of Code Aurora Forum, hosted by The Linux Foundation
^ permalink raw reply related [flat|nested] 13+ messages in thread* Re: [PATCH 2/3] drm/msm/dsi: correct DSI id bounds check during registration
2018-01-12 20:55 ` [PATCH 2/3] drm/msm/dsi: correct DSI id bounds check during registration Lloyd Atkinson
@ 2018-01-15 14:44 ` Rob Clark
0 siblings, 0 replies; 13+ messages in thread
From: Rob Clark @ 2018-01-15 14:44 UTC (permalink / raw)
To: Lloyd Atkinson; +Cc: dri-devel, linux-arm-msm
On Fri, Jan 12, 2018 at 3:55 PM, Lloyd Atkinson <latkinso@codeaurora.org> wrote:
> Check DSI instance id argument against the proper boundary size
> to protect against invalid configuration of the DSI id.
>
> Signed-off-by: Lloyd Atkinson <latkinso@codeaurora.org>
> ---
> drivers/gpu/drm/msm/dsi/dsi_manager.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/msm/dsi/dsi_manager.c b/drivers/gpu/drm/msm/dsi/dsi_manager.c
> index d276358..dec2f74 100644
> --- a/drivers/gpu/drm/msm/dsi/dsi_manager.c
> +++ b/drivers/gpu/drm/msm/dsi/dsi_manager.c
> @@ -862,7 +862,7 @@ int msm_dsi_manager_register(struct msm_dsi *msm_dsi)
> int id = msm_dsi->id;
> int ret;
>
> - if (id > DSI_MAX) {
> + if (id >= DSI_MAX) {
good catch, I've queued this up on msm-next
BR,
-R
> pr_err("%s: invalid id %d\n", __func__, id);
> return -EINVAL;
> }
> --
> QUALCOMM Canada, on behalf of Qualcomm Innovation Center, Inc. is a member
> of Code Aurora Forum, hosted by The Linux Foundation
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-arm-msm" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply [flat|nested] 13+ messages in thread
* [PATCH 3/3] drm/msm/dsi: check msm_dsi and dsi pointers before use
2018-01-12 20:55 [PATCH 0/3] drm/msm/dsi: improve pointer validation checks Lloyd Atkinson
2018-01-12 20:55 ` [PATCH 1/3] drm/msm/dsi: check src_pll for null in dsi manager Lloyd Atkinson
2018-01-12 20:55 ` [PATCH 2/3] drm/msm/dsi: correct DSI id bounds check during registration Lloyd Atkinson
@ 2018-01-12 20:55 ` Lloyd Atkinson
2018-01-15 14:48 ` Rob Clark
2018-01-16 21:26 ` [PATCH v2 0/3] drm/msm/dsi: improve pointer validation checks Lloyd Atkinson
` (3 subsequent siblings)
6 siblings, 1 reply; 13+ messages in thread
From: Lloyd Atkinson @ 2018-01-12 20:55 UTC (permalink / raw)
To: dri-devel; +Cc: Lloyd Atkinson, linux-arm-msm
Move null checks of pointer arguments to the beginning of the
modeset init function since they are referenced immediately
instead of after they have already been used.
Signed-off-by: Lloyd Atkinson <latkinso@codeaurora.org>
---
drivers/gpu/drm/msm/dsi/dsi.c | 22 ++++++++++------------
1 file changed, 10 insertions(+), 12 deletions(-)
diff --git a/drivers/gpu/drm/msm/dsi/dsi.c b/drivers/gpu/drm/msm/dsi/dsi.c
index 98742d7..be855db 100644
--- a/drivers/gpu/drm/msm/dsi/dsi.c
+++ b/drivers/gpu/drm/msm/dsi/dsi.c
@@ -196,7 +196,7 @@ int msm_dsi_modeset_init(struct msm_dsi *msm_dsi, struct drm_device *dev,
struct drm_bridge *ext_bridge;
int ret;
- if (WARN_ON(!encoder))
+ if (!encoder || !msm_dsi || !dev)
return -EINVAL;
msm_dsi->dev = dev;
@@ -245,19 +245,17 @@ int msm_dsi_modeset_init(struct msm_dsi *msm_dsi, struct drm_device *dev,
return 0;
fail:
- if (msm_dsi) {
- /* bridge/connector are normally destroyed by drm: */
- if (msm_dsi->bridge) {
- msm_dsi_manager_bridge_destroy(msm_dsi->bridge);
- msm_dsi->bridge = NULL;
- }
+ /* bridge/connector are normally destroyed by drm: */
+ if (msm_dsi->bridge) {
+ msm_dsi_manager_bridge_destroy(msm_dsi->bridge);
+ msm_dsi->bridge = NULL;
+ }
- /* don't destroy connector if we didn't make it */
- if (msm_dsi->connector && !msm_dsi->external_bridge)
- msm_dsi->connector->funcs->destroy(msm_dsi->connector);
+ /* don't destroy connector if we didn't make it */
+ if (msm_dsi->connector && !msm_dsi->external_bridge)
+ msm_dsi->connector->funcs->destroy(msm_dsi->connector);
- msm_dsi->connector = NULL;
- }
+ msm_dsi->connector = NULL;
return ret;
}
--
QUALCOMM Canada, on behalf of Qualcomm Innovation Center, Inc. is a member
of Code Aurora Forum, hosted by The Linux Foundation
^ permalink raw reply related [flat|nested] 13+ messages in thread* Re: [PATCH 3/3] drm/msm/dsi: check msm_dsi and dsi pointers before use
2018-01-12 20:55 ` [PATCH 3/3] drm/msm/dsi: check msm_dsi and dsi pointers before use Lloyd Atkinson
@ 2018-01-15 14:48 ` Rob Clark
2018-01-15 15:01 ` Lloyd Atkinson
0 siblings, 1 reply; 13+ messages in thread
From: Rob Clark @ 2018-01-15 14:48 UTC (permalink / raw)
To: Lloyd Atkinson; +Cc: dri-devel, linux-arm-msm
On Fri, Jan 12, 2018 at 3:55 PM, Lloyd Atkinson <latkinso@codeaurora.org> wrote:
> Move null checks of pointer arguments to the beginning of the
> modeset init function since they are referenced immediately
> instead of after they have already been used.
>
> Signed-off-by: Lloyd Atkinson <latkinso@codeaurora.org>
> ---
> drivers/gpu/drm/msm/dsi/dsi.c | 22 ++++++++++------------
> 1 file changed, 10 insertions(+), 12 deletions(-)
>
> diff --git a/drivers/gpu/drm/msm/dsi/dsi.c b/drivers/gpu/drm/msm/dsi/dsi.c
> index 98742d7..be855db 100644
> --- a/drivers/gpu/drm/msm/dsi/dsi.c
> +++ b/drivers/gpu/drm/msm/dsi/dsi.c
> @@ -196,7 +196,7 @@ int msm_dsi_modeset_init(struct msm_dsi *msm_dsi, struct drm_device *dev,
> struct drm_bridge *ext_bridge;
> int ret;
>
> - if (WARN_ON(!encoder))
> + if (!encoder || !msm_dsi || !dev)
hmm, the checking if msm_dsi is null later in the fail: case is
certainly sketchy after we've already deref'd it.. so this looks like
the right thing. But I'd like to keep the WARN_ON(), since this is a
case that shouldn't really happen. The WARN_ON() nicely documents
that none of these parameters are expected to be NULL, and it gives a
big shouty message to anyone who inadvertently changes something that
breaks that assumption. Other than that, it looks good.
BR,
-R
> return -EINVAL;
>
> msm_dsi->dev = dev;
> @@ -245,19 +245,17 @@ int msm_dsi_modeset_init(struct msm_dsi *msm_dsi, struct drm_device *dev,
>
> return 0;
> fail:
> - if (msm_dsi) {
> - /* bridge/connector are normally destroyed by drm: */
> - if (msm_dsi->bridge) {
> - msm_dsi_manager_bridge_destroy(msm_dsi->bridge);
> - msm_dsi->bridge = NULL;
> - }
> + /* bridge/connector are normally destroyed by drm: */
> + if (msm_dsi->bridge) {
> + msm_dsi_manager_bridge_destroy(msm_dsi->bridge);
> + msm_dsi->bridge = NULL;
> + }
>
> - /* don't destroy connector if we didn't make it */
> - if (msm_dsi->connector && !msm_dsi->external_bridge)
> - msm_dsi->connector->funcs->destroy(msm_dsi->connector);
> + /* don't destroy connector if we didn't make it */
> + if (msm_dsi->connector && !msm_dsi->external_bridge)
> + msm_dsi->connector->funcs->destroy(msm_dsi->connector);
>
> - msm_dsi->connector = NULL;
> - }
> + msm_dsi->connector = NULL;
>
> return ret;
> }
> --
> QUALCOMM Canada, on behalf of Qualcomm Innovation Center, Inc. is a member
> of Code Aurora Forum, hosted by The Linux Foundation
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-arm-msm" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply [flat|nested] 13+ messages in thread* Re: [PATCH 3/3] drm/msm/dsi: check msm_dsi and dsi pointers before use
2018-01-15 14:48 ` Rob Clark
@ 2018-01-15 15:01 ` Lloyd Atkinson
2018-01-15 15:25 ` Rob Clark
0 siblings, 1 reply; 13+ messages in thread
From: Lloyd Atkinson @ 2018-01-15 15:01 UTC (permalink / raw)
To: Rob Clark; +Cc: linux-arm-msm, dri-devel
On 1/15/2018 9:48 AM, Rob Clark wrote:
> On Fri, Jan 12, 2018 at 3:55 PM, Lloyd Atkinson <latkinso@codeaurora.org> wrote:
>> Move null checks of pointer arguments to the beginning of the
>> modeset init function since they are referenced immediately
>> instead of after they have already been used.
>>
>> Signed-off-by: Lloyd Atkinson <latkinso@codeaurora.org>
>> ---
>> drivers/gpu/drm/msm/dsi/dsi.c | 22 ++++++++++------------
>> 1 file changed, 10 insertions(+), 12 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/msm/dsi/dsi.c b/drivers/gpu/drm/msm/dsi/dsi.c
>> index 98742d7..be855db 100644
>> --- a/drivers/gpu/drm/msm/dsi/dsi.c
>> +++ b/drivers/gpu/drm/msm/dsi/dsi.c
>> @@ -196,7 +196,7 @@ int msm_dsi_modeset_init(struct msm_dsi *msm_dsi, struct drm_device *dev,
>> struct drm_bridge *ext_bridge;
>> int ret;
>>
>> - if (WARN_ON(!encoder))
>> + if (!encoder || !msm_dsi || !dev)
>
> hmm, the checking if msm_dsi is null later in the fail: case is
> certainly sketchy after we've already deref'd it.. so this looks like
> the right thing. But I'd like to keep the WARN_ON(), since this is a
> case that shouldn't really happen. The WARN_ON() nicely documents
> that none of these parameters are expected to be NULL, and it gives a
> big shouty message to anyone who inadvertently changes something that
> breaks that assumption. Other than that, it looks good.
>
> BR,
> -R
Sure. Do you want to add WARN_ONs to msm_dsi and dev as well?
Thanks,
Lloyd
>
>
>> return -EINVAL;
>>
>> msm_dsi->dev = dev;
>> @@ -245,19 +245,17 @@ int msm_dsi_modeset_init(struct msm_dsi *msm_dsi, struct drm_device *dev,
>>
>> return 0;
>> fail:
>> - if (msm_dsi) {
>> - /* bridge/connector are normally destroyed by drm: */
>> - if (msm_dsi->bridge) {
>> - msm_dsi_manager_bridge_destroy(msm_dsi->bridge);
>> - msm_dsi->bridge = NULL;
>> - }
>> + /* bridge/connector are normally destroyed by drm: */
>> + if (msm_dsi->bridge) {
>> + msm_dsi_manager_bridge_destroy(msm_dsi->bridge);
>> + msm_dsi->bridge = NULL;
>> + }
>>
>> - /* don't destroy connector if we didn't make it */
>> - if (msm_dsi->connector && !msm_dsi->external_bridge)
>> - msm_dsi->connector->funcs->destroy(msm_dsi->connector);
>> + /* don't destroy connector if we didn't make it */
>> + if (msm_dsi->connector && !msm_dsi->external_bridge)
>> + msm_dsi->connector->funcs->destroy(msm_dsi->connector);
>>
>> - msm_dsi->connector = NULL;
>> - }
>> + msm_dsi->connector = NULL;
>>
>> return ret;
>> }
>> --
>> QUALCOMM Canada, on behalf of Qualcomm Innovation Center, Inc. is a member
>> of Code Aurora Forum, hosted by The Linux Foundation
>>
>> --
>> To unsubscribe from this list: send the line "unsubscribe linux-arm-msm" in
>> the body of a message to majordomo@vger.kernel.org
>> More majordomo info at http://vger.kernel.org/majordomo-info.html
--
QUALCOMM Canada, on behalf of Qualcomm Innovation Center, Inc. is a member
of Code Aurora Forum, hosted by The Linux Foundation
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply [flat|nested] 13+ messages in thread* Re: [PATCH 3/3] drm/msm/dsi: check msm_dsi and dsi pointers before use
2018-01-15 15:01 ` Lloyd Atkinson
@ 2018-01-15 15:25 ` Rob Clark
0 siblings, 0 replies; 13+ messages in thread
From: Rob Clark @ 2018-01-15 15:25 UTC (permalink / raw)
To: Lloyd Atkinson; +Cc: dri-devel, linux-arm-msm
On Mon, Jan 15, 2018 at 10:01 AM, Lloyd Atkinson
<latkinso@codeaurora.org> wrote:
> On 1/15/2018 9:48 AM, Rob Clark wrote:
>> On Fri, Jan 12, 2018 at 3:55 PM, Lloyd Atkinson <latkinso@codeaurora.org> wrote:
>>> Move null checks of pointer arguments to the beginning of the
>>> modeset init function since they are referenced immediately
>>> instead of after they have already been used.
>>>
>>> Signed-off-by: Lloyd Atkinson <latkinso@codeaurora.org>
>>> ---
>>> drivers/gpu/drm/msm/dsi/dsi.c | 22 ++++++++++------------
>>> 1 file changed, 10 insertions(+), 12 deletions(-)
>>>
>>> diff --git a/drivers/gpu/drm/msm/dsi/dsi.c b/drivers/gpu/drm/msm/dsi/dsi.c
>>> index 98742d7..be855db 100644
>>> --- a/drivers/gpu/drm/msm/dsi/dsi.c
>>> +++ b/drivers/gpu/drm/msm/dsi/dsi.c
>>> @@ -196,7 +196,7 @@ int msm_dsi_modeset_init(struct msm_dsi *msm_dsi, struct drm_device *dev,
>>> struct drm_bridge *ext_bridge;
>>> int ret;
>>>
>>> - if (WARN_ON(!encoder))
>>> + if (!encoder || !msm_dsi || !dev)
>>
>> hmm, the checking if msm_dsi is null later in the fail: case is
>> certainly sketchy after we've already deref'd it.. so this looks like
>> the right thing. But I'd like to keep the WARN_ON(), since this is a
>> case that shouldn't really happen. The WARN_ON() nicely documents
>> that none of these parameters are expected to be NULL, and it gives a
>> big shouty message to anyone who inadvertently changes something that
>> breaks that assumption. Other than that, it looks good.
>>
>> BR,
>> -R
>
> Sure. Do you want to add WARN_ONs to msm_dsi and dev as well?
>
yup, thanks
BR,
-R
> Thanks,
> Lloyd
>
>>
>>
>>> return -EINVAL;
>>>
>>> msm_dsi->dev = dev;
>>> @@ -245,19 +245,17 @@ int msm_dsi_modeset_init(struct msm_dsi *msm_dsi, struct drm_device *dev,
>>>
>>> return 0;
>>> fail:
>>> - if (msm_dsi) {
>>> - /* bridge/connector are normally destroyed by drm: */
>>> - if (msm_dsi->bridge) {
>>> - msm_dsi_manager_bridge_destroy(msm_dsi->bridge);
>>> - msm_dsi->bridge = NULL;
>>> - }
>>> + /* bridge/connector are normally destroyed by drm: */
>>> + if (msm_dsi->bridge) {
>>> + msm_dsi_manager_bridge_destroy(msm_dsi->bridge);
>>> + msm_dsi->bridge = NULL;
>>> + }
>>>
>>> - /* don't destroy connector if we didn't make it */
>>> - if (msm_dsi->connector && !msm_dsi->external_bridge)
>>> - msm_dsi->connector->funcs->destroy(msm_dsi->connector);
>>> + /* don't destroy connector if we didn't make it */
>>> + if (msm_dsi->connector && !msm_dsi->external_bridge)
>>> + msm_dsi->connector->funcs->destroy(msm_dsi->connector);
>>>
>>> - msm_dsi->connector = NULL;
>>> - }
>>> + msm_dsi->connector = NULL;
>>>
>>> return ret;
>>> }
>>> --
>>> QUALCOMM Canada, on behalf of Qualcomm Innovation Center, Inc. is a member
>>> of Code Aurora Forum, hosted by The Linux Foundation
>>>
>>> --
>>> To unsubscribe from this list: send the line "unsubscribe linux-arm-msm" in
>>> the body of a message to majordomo@vger.kernel.org
>>> More majordomo info at http://vger.kernel.org/majordomo-info.html
>
>
> --
> QUALCOMM Canada, on behalf of Qualcomm Innovation Center, Inc. is a member
> of Code Aurora Forum, hosted by The Linux Foundation
^ permalink raw reply [flat|nested] 13+ messages in thread
* [PATCH v2 0/3] drm/msm/dsi: improve pointer validation checks
2018-01-12 20:55 [PATCH 0/3] drm/msm/dsi: improve pointer validation checks Lloyd Atkinson
` (2 preceding siblings ...)
2018-01-12 20:55 ` [PATCH 3/3] drm/msm/dsi: check msm_dsi and dsi pointers before use Lloyd Atkinson
@ 2018-01-16 21:26 ` Lloyd Atkinson
2018-01-16 21:26 ` [PATCH v2 1/3] drm/msm/dsi: check for failure on retrieving pll in dsi manager Lloyd Atkinson
` (2 subsequent siblings)
6 siblings, 0 replies; 13+ messages in thread
From: Lloyd Atkinson @ 2018-01-16 21:26 UTC (permalink / raw)
To: dri-devel; +Cc: Lloyd Atkinson, linux-arm-msm, robdclark
This series improves a few pointer validation checks around the
drm/msm/dsi driver.
v2 incoporates feedback on patch 1/3 and patch 3/3.
Lloyd Atkinson (3):
drm/msm/dsi: check for failure on retrieving pll in dsi manager
drm/msm/dsi: correct DSI id bounds check during registration
drm/msm/dsi: check msm_dsi and dsi pointers before use
drivers/gpu/drm/msm/dsi/dsi.c | 22 ++++++++++------------
drivers/gpu/drm/msm/dsi/dsi_manager.c | 6 +++++-
drivers/gpu/drm/msm/dsi/phy/dsi_phy.c | 6 +++---
drivers/gpu/drm/msm/dsi/pll/dsi_pll.c | 2 +-
4 files changed, 19 insertions(+), 17 deletions(-)
--
QUALCOMM Canada, on behalf of Qualcomm Innovation Center, Inc. is a member
of Code Aurora Forum, hosted by The Linux Foundation
^ permalink raw reply [flat|nested] 13+ messages in thread* [PATCH v2 1/3] drm/msm/dsi: check for failure on retrieving pll in dsi manager
2018-01-12 20:55 [PATCH 0/3] drm/msm/dsi: improve pointer validation checks Lloyd Atkinson
` (3 preceding siblings ...)
2018-01-16 21:26 ` [PATCH v2 0/3] drm/msm/dsi: improve pointer validation checks Lloyd Atkinson
@ 2018-01-16 21:26 ` Lloyd Atkinson
2018-01-16 21:26 ` [PATCH v2 2/3] drm/msm/dsi: correct DSI id bounds check during registration Lloyd Atkinson
2018-01-16 21:26 ` [PATCH v2 3/3] drm/msm/dsi: check msm_dsi and dsi pointers before use Lloyd Atkinson
6 siblings, 0 replies; 13+ messages in thread
From: Lloyd Atkinson @ 2018-01-16 21:26 UTC (permalink / raw)
To: dri-devel; +Cc: Lloyd Atkinson, linux-arm-msm, robdclark
Make msm_dsi_pll_init consistently return an error code instead
of NULL when pll initialization fails so that later pll
retrieval can check against an error code. Add checks for these
failures after retrieval of src_pll to avoid invalid pointer
dereferences later in msm_dsi_pll_get_clk_provider.
Signed-off-by: Lloyd Atkinson <latkinso@codeaurora.org>
---
drivers/gpu/drm/msm/dsi/dsi_manager.c | 4 ++++
drivers/gpu/drm/msm/dsi/phy/dsi_phy.c | 6 +++---
drivers/gpu/drm/msm/dsi/pll/dsi_pll.c | 2 +-
3 files changed, 8 insertions(+), 4 deletions(-)
diff --git a/drivers/gpu/drm/msm/dsi/dsi_manager.c b/drivers/gpu/drm/msm/dsi/dsi_manager.c
index 8552481..1a54fd6 100644
--- a/drivers/gpu/drm/msm/dsi/dsi_manager.c
+++ b/drivers/gpu/drm/msm/dsi/dsi_manager.c
@@ -88,6 +88,8 @@ static int dsi_mgr_setup_components(int id)
msm_dsi_phy_set_usecase(msm_dsi->phy, MSM_DSI_PHY_STANDALONE);
src_pll = msm_dsi_phy_get_pll(msm_dsi->phy);
+ if (IS_ERR(src_pll))
+ return PTR_ERR(src_pll);
ret = msm_dsi_host_set_src_pll(msm_dsi->host, src_pll);
} else if (!other_dsi) {
ret = 0;
@@ -116,6 +118,8 @@ static int dsi_mgr_setup_components(int id)
msm_dsi_phy_set_usecase(clk_slave_dsi->phy,
MSM_DSI_PHY_SLAVE);
src_pll = msm_dsi_phy_get_pll(clk_master_dsi->phy);
+ if (IS_ERR(src_pll))
+ return PTR_ERR(src_pll);
ret = msm_dsi_host_set_src_pll(msm_dsi->host, src_pll);
if (ret)
return ret;
diff --git a/drivers/gpu/drm/msm/dsi/phy/dsi_phy.c b/drivers/gpu/drm/msm/dsi/phy/dsi_phy.c
index 790ca28..c8bfaa7 100644
--- a/drivers/gpu/drm/msm/dsi/phy/dsi_phy.c
+++ b/drivers/gpu/drm/msm/dsi/phy/dsi_phy.c
@@ -503,10 +503,10 @@ static int dsi_phy_driver_probe(struct platform_device *pdev)
goto fail;
phy->pll = msm_dsi_pll_init(pdev, phy->cfg->type, phy->id);
- if (!phy->pll)
+ if (IS_ERR_OR_NULL(phy->pll))
dev_info(dev,
- "%s: pll init failed, need separate pll clk driver\n",
- __func__);
+ "%s: pll init failed: %ld, need separate pll clk driver\n",
+ __func__, PTR_ERR(phy->pll));
dsi_phy_disable_resource(phy);
diff --git a/drivers/gpu/drm/msm/dsi/pll/dsi_pll.c b/drivers/gpu/drm/msm/dsi/pll/dsi_pll.c
index bc289f5..491f08d 100644
--- a/drivers/gpu/drm/msm/dsi/pll/dsi_pll.c
+++ b/drivers/gpu/drm/msm/dsi/pll/dsi_pll.c
@@ -173,7 +173,7 @@ struct msm_dsi_pll *msm_dsi_pll_init(struct platform_device *pdev,
if (IS_ERR(pll)) {
dev_err(dev, "%s: failed to init DSI PLL\n", __func__);
- return NULL;
+ return pll;
}
pll->type = type;
--
QUALCOMM Canada, on behalf of Qualcomm Innovation Center, Inc. is a member
of Code Aurora Forum, hosted by The Linux Foundation
^ permalink raw reply related [flat|nested] 13+ messages in thread* [PATCH v2 2/3] drm/msm/dsi: correct DSI id bounds check during registration
2018-01-12 20:55 [PATCH 0/3] drm/msm/dsi: improve pointer validation checks Lloyd Atkinson
` (4 preceding siblings ...)
2018-01-16 21:26 ` [PATCH v2 1/3] drm/msm/dsi: check for failure on retrieving pll in dsi manager Lloyd Atkinson
@ 2018-01-16 21:26 ` Lloyd Atkinson
2018-01-16 21:26 ` [PATCH v2 3/3] drm/msm/dsi: check msm_dsi and dsi pointers before use Lloyd Atkinson
6 siblings, 0 replies; 13+ messages in thread
From: Lloyd Atkinson @ 2018-01-16 21:26 UTC (permalink / raw)
To: dri-devel; +Cc: linux-arm-msm, Lloyd Atkinson
Check DSI instance id argument against the proper boundary size
to protect against invalid configuration of the DSI id.
Signed-off-by: Lloyd Atkinson <latkinso@codeaurora.org>
---
drivers/gpu/drm/msm/dsi/dsi_manager.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/msm/dsi/dsi_manager.c b/drivers/gpu/drm/msm/dsi/dsi_manager.c
index 1a54fd6..4cb1cb6 100644
--- a/drivers/gpu/drm/msm/dsi/dsi_manager.c
+++ b/drivers/gpu/drm/msm/dsi/dsi_manager.c
@@ -862,7 +862,7 @@ int msm_dsi_manager_register(struct msm_dsi *msm_dsi)
int id = msm_dsi->id;
int ret;
- if (id > DSI_MAX) {
+ if (id >= DSI_MAX) {
pr_err("%s: invalid id %d\n", __func__, id);
return -EINVAL;
}
--
QUALCOMM Canada, on behalf of Qualcomm Innovation Center, Inc. is a member
of Code Aurora Forum, hosted by The Linux Foundation
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply related [flat|nested] 13+ messages in thread* [PATCH v2 3/3] drm/msm/dsi: check msm_dsi and dsi pointers before use
2018-01-12 20:55 [PATCH 0/3] drm/msm/dsi: improve pointer validation checks Lloyd Atkinson
` (5 preceding siblings ...)
2018-01-16 21:26 ` [PATCH v2 2/3] drm/msm/dsi: correct DSI id bounds check during registration Lloyd Atkinson
@ 2018-01-16 21:26 ` Lloyd Atkinson
6 siblings, 0 replies; 13+ messages in thread
From: Lloyd Atkinson @ 2018-01-16 21:26 UTC (permalink / raw)
To: dri-devel; +Cc: linux-arm-msm, Lloyd Atkinson
Move null checks of pointer arguments to the beginning of the
modeset init function since they are referenced immediately
instead of after they have already been used.
Signed-off-by: Lloyd Atkinson <latkinso@codeaurora.org>
---
drivers/gpu/drm/msm/dsi/dsi.c | 22 ++++++++++------------
1 file changed, 10 insertions(+), 12 deletions(-)
diff --git a/drivers/gpu/drm/msm/dsi/dsi.c b/drivers/gpu/drm/msm/dsi/dsi.c
index 98742d7..ee7e090 100644
--- a/drivers/gpu/drm/msm/dsi/dsi.c
+++ b/drivers/gpu/drm/msm/dsi/dsi.c
@@ -196,7 +196,7 @@ int msm_dsi_modeset_init(struct msm_dsi *msm_dsi, struct drm_device *dev,
struct drm_bridge *ext_bridge;
int ret;
- if (WARN_ON(!encoder))
+ if (WARN_ON(!encoder) || WARN_ON(!msm_dsi) || WARN_ON(!dev))
return -EINVAL;
msm_dsi->dev = dev;
@@ -245,19 +245,17 @@ int msm_dsi_modeset_init(struct msm_dsi *msm_dsi, struct drm_device *dev,
return 0;
fail:
- if (msm_dsi) {
- /* bridge/connector are normally destroyed by drm: */
- if (msm_dsi->bridge) {
- msm_dsi_manager_bridge_destroy(msm_dsi->bridge);
- msm_dsi->bridge = NULL;
- }
+ /* bridge/connector are normally destroyed by drm: */
+ if (msm_dsi->bridge) {
+ msm_dsi_manager_bridge_destroy(msm_dsi->bridge);
+ msm_dsi->bridge = NULL;
+ }
- /* don't destroy connector if we didn't make it */
- if (msm_dsi->connector && !msm_dsi->external_bridge)
- msm_dsi->connector->funcs->destroy(msm_dsi->connector);
+ /* don't destroy connector if we didn't make it */
+ if (msm_dsi->connector && !msm_dsi->external_bridge)
+ msm_dsi->connector->funcs->destroy(msm_dsi->connector);
- msm_dsi->connector = NULL;
- }
+ msm_dsi->connector = NULL;
return ret;
}
--
QUALCOMM Canada, on behalf of Qualcomm Innovation Center, Inc. is a member
of Code Aurora Forum, hosted by The Linux Foundation
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply related [flat|nested] 13+ messages in thread