[PATCH] soc: qcom: pmic_glink_altmode: fix port sanity check

[PATCH] soc: qcom: pmic_glink_altmode: fix port sanity check

[Johan Hovold]

The PMIC GLINK altmode driver currently supports at most two ports.

Fix the incomplete port sanity check on notifications to avoid
accessing and corrupting memory beyond the port array if we ever get a
notification for an unsupported port.

Fixes: 080b4e24852b ("soc: qcom: pmic_glink: Introduce altmode support")
Cc: stable@vger.kernel.org	# 6.3
Signed-off-by: Johan Hovold <johan+linaro@kernel.org>
---
 drivers/soc/qcom/pmic_glink_altmode.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/soc/qcom/pmic_glink_altmode.c b/drivers/soc/qcom/pmic_glink_altmode.c
index 974c14d1e0bf..561d6ba005f4 100644
--- a/drivers/soc/qcom/pmic_glink_altmode.c
+++ b/drivers/soc/qcom/pmic_glink_altmode.c
@@ -285,7 +285,7 @@ static void pmic_glink_altmode_sc8180xp_notify(struct pmic_glink_altmode *altmod
 
 	svid = mux == 2 ? USB_TYPEC_DP_SID : 0;
 
-	if (!altmode->ports[port].altmode) {
+	if (port >= ARRAY_SIZE(altmode->ports) || !altmode->ports[port].altmode) {
 		dev_dbg(altmode->dev, "notification on undefined port %d\n", port);
 		return;
 	}
@@ -328,7 +328,7 @@ static void pmic_glink_altmode_sc8280xp_notify(struct pmic_glink_altmode *altmod
 	hpd_state = FIELD_GET(SC8280XP_HPD_STATE_MASK, notify->payload[8]);
 	hpd_irq = FIELD_GET(SC8280XP_HPD_IRQ_MASK, notify->payload[8]);
 
-	if (!altmode->ports[port].altmode) {
+	if (port >= ARRAY_SIZE(altmode->ports) || !altmode->ports[port].altmode) {
 		dev_dbg(altmode->dev, "notification on undefined port %d\n", port);
 		return;
 	}
-- 
2.41.0

Re: [PATCH] soc: qcom: pmic_glink_altmode: fix port sanity check

[Konrad Dybcio]

On 11/9/23 10:31, Johan Hovold wrote:
> The PMIC GLINK altmode driver currently supports at most two ports.
> 
> Fix the incomplete port sanity check on notifications to avoid
> accessing and corrupting memory beyond the port array if we ever get a
> notification for an unsupported port.
> 
> Fixes: 080b4e24852b ("soc: qcom: pmic_glink: Introduce altmode support")
> Cc: stable@vger.kernel.org	# 6.3
> Signed-off-by: Johan Hovold <johan+linaro@kernel.org>
> ---
>   drivers/soc/qcom/pmic_glink_altmode.c | 4 ++--
>   1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/soc/qcom/pmic_glink_altmode.c b/drivers/soc/qcom/pmic_glink_altmode.c
> index 974c14d1e0bf..561d6ba005f4 100644
> --- a/drivers/soc/qcom/pmic_glink_altmode.c
> +++ b/drivers/soc/qcom/pmic_glink_altmode.c
> @@ -285,7 +285,7 @@ static void pmic_glink_altmode_sc8180xp_notify(struct pmic_glink_altmode *altmod
>   
>   	svid = mux == 2 ? USB_TYPEC_DP_SID : 0;
>   
> -	if (!altmode->ports[port].altmode) {
> +	if (port >= ARRAY_SIZE(altmode->ports) || !altmode->ports[port].altmode) {
I'd personally use PMIC_GLINK_MAX_PORTS directly but it's the same

Reviewed-by: Konrad Dybcio <konrad.dybcio@linaro.org>

Konrad

Re: [PATCH] soc: qcom: pmic_glink_altmode: fix port sanity check

[Johan Hovold]

On Thu, Nov 09, 2023 at 02:28:59PM +0100, Konrad Dybcio wrote:
> On 11/9/23 10:31, Johan Hovold wrote:
  
> > -	if (!altmode->ports[port].altmode) {
> > +	if (port >= ARRAY_SIZE(altmode->ports) || !altmode->ports[port].altmode) {

> I'd personally use PMIC_GLINK_MAX_PORTS directly but it's the same

That's what I'd generally do as well, but here I followed the style of
this driver (and using ARRAY_SIZE() is arguable more safe).

> Reviewed-by: Konrad Dybcio <konrad.dybcio@linaro.org>

Thanks for reviewing.

Johan

Re: [PATCH] soc: qcom: pmic_glink_altmode: fix port sanity check

[Dmitry Baryshkov]

On Thu, 9 Nov 2023 at 15:47, Johan Hovold <johan@kernel.org> wrote:
>
> On Thu, Nov 09, 2023 at 02:28:59PM +0100, Konrad Dybcio wrote:
> > On 11/9/23 10:31, Johan Hovold wrote:
>
> > > -   if (!altmode->ports[port].altmode) {
> > > +   if (port >= ARRAY_SIZE(altmode->ports) || !altmode->ports[port].altmode) {
>
> > I'd personally use PMIC_GLINK_MAX_PORTS directly but it's the same
>
> That's what I'd generally do as well, but here I followed the style of
> this driver (and using ARRAY_SIZE() is arguable more safe).

I'd prefer ARRAY_SIZE here too.

Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>

>
> > Reviewed-by: Konrad Dybcio <konrad.dybcio@linaro.org>
>
> Thanks for reviewing.
>
> Johan
>


-- 
With best wishes
Dmitry

Re: [PATCH] soc: qcom: pmic_glink_altmode: fix port sanity check

[Bjorn Andersson]

On Thu, 09 Nov 2023 10:31:00 +0100, Johan Hovold wrote:
> The PMIC GLINK altmode driver currently supports at most two ports.
> 
> Fix the incomplete port sanity check on notifications to avoid
> accessing and corrupting memory beyond the port array if we ever get a
> notification for an unsupported port.
> 
> 
> [...]

Applied, thanks!

[1/1] soc: qcom: pmic_glink_altmode: fix port sanity check
      commit: c4fb7d2eac9ff9bfc35a2e4d40c7169a332416e0

Best regards,
-- 
Bjorn Andersson <andersson@kernel.org>