Skip to contentSkip to conversation

soc: qcom: pmic_glink_altmode: fix port sanity check

PATCH Applied Johan Hovold 5 participants 5 messages Nov 9, 2023 – Dec 8, 2023

Subsystems (3)
Stable branch Arm Qualcomm mailing list Arm Qualcomm support
lore
Share & tools
lore

Johan Hovold

author
#0 lore raw

PATCH 1/1 soc: qcom: pmic_glink_altmode: fix port sanity check

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 [email protected] # 6.3
Signed-off-by Johan Hovold <[email protected]>
---
 drivers/soc/qcom/pmic_glink_altmode.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)
drivers/soc/qcom/pmic_glink_altmode.c +2 -2
index 974c14d1e0bf..561d6ba005f4 100644
@@ -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;
 	}
Signature · 1 line
2.41.0
Reviewed-by on v1 1/1
#1 lore raw
Quoted 18 lines · 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 [email protected] # 6.3
Signed-off-by Johan Hovold <[email protected]>
---
  drivers/soc/qcom/pmic_glink_altmode.c | 4 ++--
  1 file changed, 2 insertions(+), 2 deletions(-)
drivers/soc/qcom/pmic_glink_altmode.c +0 -0
index 974c14d1e0bf..561d6ba005f4 100644

drivers/soc/qcom/pmic_glink_altmode.c static void pmic_glink_altmode_sc8180xp_notify(struct pmic_glink_altmode *altmod · line 288

Quoted 5 lines
drivers/soc/qcom/pmic_glink_altmode.c +1 -1
@@ -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 <[email protected]>

Konrad

#2 lore raw
Quoted 6 lines · Konrad Dybcio

Review

Quoted 2 lines · Johan Hovold
(quoted diff) +1 -1
-	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).

Quoted 1 line
Reviewed-by Konrad Dybcio <[email protected]>

Thanks for reviewing.

Johan

Reviewed-by
#3 lore raw
Quoted 11 lines · Johan Hovold
Quoted 6 lines · Konrad Dybcio

Review

Quoted 2 lines · Johan Hovold
(quoted diff) +1 -1
-   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 <[email protected]>
Quoted 7 lines
Quoted 2 lines
Reviewed-by Konrad Dybcio <[email protected]>

Thanks for reviewing.

Johan

Signature · 2 lines
With best wishes
Dmitry
maintainer Applied
#4 lore raw
In reply to Johan Hovold
Quoted 8 lines · 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.

[...]

Applied, thanks!

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

      commit: c4fb7d2eac9ff9bfc35a2e4d40c7169a332416e0

Best regards,

Signature · 1 line
Bjorn Andersson <[email protected]>