[PATCH] phy: exynos5-usbdrd: Use dynamic phy_cfg size to prevent OOB access

Linux-PHY Archive on lore.kernel.org
 help / color / mirror / Atom feed

* [PATCH] phy: exynos5-usbdrd: Use dynamic phy_cfg size to prevent OOB access
       [not found] <CGME20260831070349epcas5p4c62a4e46592dffe95723ddd51a6068e0@epcas5p4.samsung.com>
@ 2026-08-31  7:03 ` Selvarasu Ganesan
  2026-09-02  0:04   ` Łukasz Lebiedziński
  0 siblings, 1 reply; 3+ messages in thread
From: Selvarasu Ganesan @ 2026-08-31  7:03 UTC (permalink / raw)
  To: vkoul, neil.armstrong, krzk, peter.griffin, alim.akhtar,
	pritam.sutar, andre.draszik, kernel, linux-phy, linux-arm-kernel,
	linux-samsung-soc, linux-kernel
  Cc: jh0801.jung, dh10.jung, akash.m5, muhammed.ali, thiagu.r,
	Selvarasu Ganesan

The probe loop currently iterates using EXYNOS5_DRDPHYS_NUM (2),
creating both UTMI and PIPE3 PHY instances regardless of the SoC
capability. Several SoCs (Exynos2200, Exynos7870, Exynos850, Exynos990,
and ExynosAutoV920) provide phy_cfg arrays containing only a single
element.

On these SoCs, when the loop reaches index 1, the driver reads past the
end of the rodata array, populating the second PHY instance with garbage
data. Since the configuration structure contains critical function
pointers (phy_isol, phy_init, set_refclk), any subsequent access to this
PHY instance via exynos5_usbdrd_phy_xlate could result in a kernel oops.

Fix this by adding 'n_phy_cfg' to struct exynos5_usbdrd_phy_drvdata to
store the actual size of the phy_cfg array for each SoC. Update the
probe loop and the xlate function to bound their access against this
value instead of the hardcoded EXYNOS5_DRDPHYS_NUM.

Assisted-by: Claude:claude-sonnet-5
Signed-off-by: Selvarasu Ganesan <selvarasu.g@samsung.com>
---
 drivers/phy/samsung/phy-exynos5-usbdrd.c | 17 +++++++++++++++--
 1 file changed, 15 insertions(+), 2 deletions(-)

diff --git a/drivers/phy/samsung/phy-exynos5-usbdrd.c b/drivers/phy/samsung/phy-exynos5-usbdrd.c
index 8711a3b62c8e..311d1c25eb3e 100644
--- a/drivers/phy/samsung/phy-exynos5-usbdrd.c
+++ b/drivers/phy/samsung/phy-exynos5-usbdrd.c
@@ -477,6 +477,7 @@ struct exynos5_usbdrd_phy_config {
 
 struct exynos5_usbdrd_phy_drvdata {
 	const struct exynos5_usbdrd_phy_config *phy_cfg;
+	int n_phy_cfg;
 	const struct exynos5_usbdrd_phy_tuning **phy_tunes;
 	const struct phy_ops *phy_ops;
 	const char * const *clk_names;
@@ -1164,7 +1165,7 @@ static struct phy *exynos5_usbdrd_phy_xlate(struct device *dev,
 {
 	struct exynos5_usbdrd_phy *phy_drd = dev_get_drvdata(dev);
 
-	if (WARN_ON(args->args[0] >= EXYNOS5_DRDPHYS_NUM))
+	if (WARN_ON(args->args[0] >= phy_drd->drv_data->n_phy_cfg))
 		return ERR_PTR(-ENODEV);
 
 	return phy_drd->phys[args->args[0]].phy;
@@ -1993,6 +1994,7 @@ static const char * const exynos5_regulator_names[] = {
 
 static const struct exynos5_usbdrd_phy_drvdata exynos2200_usb32drd_phy = {
 	.phy_cfg		= phy_cfg_exynos2200,
+	.n_phy_cfg		= ARRAY_SIZE(phy_cfg_exynos2200),
 	.phy_ops		= &exynos2200_usbdrd_phy_ops,
 	.pmu_offset_usbdrd0_phy	= EXYNOS2200_PHY_CTRL_USB20,
 	.clk_names		= exynos5_clk_names,
@@ -2006,6 +2008,7 @@ static const struct exynos5_usbdrd_phy_drvdata exynos2200_usb32drd_phy = {
 
 static const struct exynos5_usbdrd_phy_drvdata exynos5420_usbdrd_phy = {
 	.phy_cfg		= phy_cfg_exynos5,
+	.n_phy_cfg		= ARRAY_SIZE(phy_cfg_exynos5),
 	.phy_ops		= &exynos5_usbdrd_phy_ops,
 	.pmu_offset_usbdrd0_phy	= EXYNOS5_USBDRD_PHY_CONTROL,
 	.pmu_offset_usbdrd1_phy	= EXYNOS5420_USBDRD1_PHY_CONTROL,
@@ -2019,6 +2022,7 @@ static const struct exynos5_usbdrd_phy_drvdata exynos5420_usbdrd_phy = {
 
 static const struct exynos5_usbdrd_phy_drvdata exynos5250_usbdrd_phy = {
 	.phy_cfg		= phy_cfg_exynos5,
+	.n_phy_cfg		= ARRAY_SIZE(phy_cfg_exynos5),
 	.phy_ops		= &exynos5_usbdrd_phy_ops,
 	.pmu_offset_usbdrd0_phy	= EXYNOS5_USBDRD_PHY_CONTROL,
 	.clk_names		= exynos5_clk_names,
@@ -2031,6 +2035,7 @@ static const struct exynos5_usbdrd_phy_drvdata exynos5250_usbdrd_phy = {
 
 static const struct exynos5_usbdrd_phy_drvdata exynos5433_usbdrd_phy = {
 	.phy_cfg		= phy_cfg_exynos5,
+	.n_phy_cfg		= ARRAY_SIZE(phy_cfg_exynos5),
 	.phy_ops		= &exynos5_usbdrd_phy_ops,
 	.pmu_offset_usbdrd0_phy	= EXYNOS5_USBDRD_PHY_CONTROL,
 	.pmu_offset_usbdrd1_phy	= EXYNOS5433_USBHOST30_PHY_CONTROL,
@@ -2044,6 +2049,7 @@ static const struct exynos5_usbdrd_phy_drvdata exynos5433_usbdrd_phy = {
 
 static const struct exynos5_usbdrd_phy_drvdata exynos7_usbdrd_phy = {
 	.phy_cfg		= phy_cfg_exynos5,
+	.n_phy_cfg		= ARRAY_SIZE(phy_cfg_exynos5),
 	.phy_ops		= &exynos5_usbdrd_phy_ops,
 	.pmu_offset_usbdrd0_phy	= EXYNOS5_USBDRD_PHY_CONTROL,
 	.clk_names		= exynos5_clk_names,
@@ -2056,6 +2062,7 @@ static const struct exynos5_usbdrd_phy_drvdata exynos7_usbdrd_phy = {
 
 static const struct exynos5_usbdrd_phy_drvdata exynos7870_usbdrd_phy = {
 	.phy_cfg		= phy_cfg_exynos7870,
+	.n_phy_cfg		= ARRAY_SIZE(phy_cfg_exynos7870),
 	.phy_tunes		= exynos7870_tunes,
 	.phy_ops		= &exynos7870_usbdrd_phy_ops,
 	.pmu_offset_usbdrd0_phy	= EXYNOS5_USBDRD_PHY_CONTROL,
@@ -2069,6 +2076,7 @@ static const struct exynos5_usbdrd_phy_drvdata exynos7870_usbdrd_phy = {
 
 static const struct exynos5_usbdrd_phy_drvdata exynos850_usbdrd_phy = {
 	.phy_cfg		= phy_cfg_exynos850,
+	.n_phy_cfg		= ARRAY_SIZE(phy_cfg_exynos850),
 	.phy_ops		= &exynos850_usbdrd_phy_ops,
 	.pmu_offset_usbdrd0_phy	= EXYNOS5_USBDRD_PHY_CONTROL,
 	.clk_names		= exynos5_clk_names,
@@ -2097,6 +2105,7 @@ static const struct exynos5_usbdrd_phy_tuning *exynos990_tunes[PTS_MAX] = {
 
 static const struct exynos5_usbdrd_phy_drvdata exynos990_usbdrd_phy = {
 	.phy_cfg		= phy_cfg_exynos850,
+	.n_phy_cfg		= ARRAY_SIZE(phy_cfg_exynos850),
 	.phy_ops		= &exynos850_usbdrd_phy_ops,
 	.phy_tunes		= exynos990_tunes,
 	.pmu_offset_usbdrd0_phy	= EXYNOS990_PHY_CTRL_USB20,
@@ -2629,6 +2638,7 @@ static const struct phy_ops exynosautov920_usb31drd_combo_ssphy_ops = {
 static const
 struct exynos5_usbdrd_phy_drvdata exynosautov920_usb31drd_combo_ssphy = {
 	.phy_cfg		= usb31drd_phy_cfg_exynosautov920,
+	.n_phy_cfg		= ARRAY_SIZE(usb31drd_phy_cfg_exynosautov920),
 	.phy_ops		= &exynosautov920_usb31drd_combo_ssphy_ops,
 	.pmu_offset_usbdrd0_phy	= EXYNOSAUTOV920_PHY_CTRL_USB31,
 	.clk_names		= exynos5_clk_names,
@@ -2659,6 +2669,7 @@ exynos5_usbdrd_phy_config usbdrd_hsphy_cfg_exynosautov920[] = {
 static const
 struct exynos5_usbdrd_phy_drvdata exynosautov920_usbdrd_combo_hsphy = {
 	.phy_cfg		= usbdrd_hsphy_cfg_exynosautov920,
+	.n_phy_cfg		= ARRAY_SIZE(usbdrd_hsphy_cfg_exynosautov920),
 	.phy_ops		= &exynosautov920_usbdrd_combo_hsphy_ops,
 	.pmu_offset_usbdrd0_phy	= EXYNOSAUTOV920_PHY_CTRL_USB20,
 	.clk_names		= exynos5_clk_names,
@@ -2687,6 +2698,7 @@ static const struct exynos5_usbdrd_phy_config phy_cfg_exynosautov920[] = {
 
 static const struct exynos5_usbdrd_phy_drvdata exynosautov920_usbdrd_phy = {
 	.phy_cfg		= phy_cfg_exynosautov920,
+	.n_phy_cfg		= ARRAY_SIZE(phy_cfg_exynosautov920),
 	.phy_ops		= &exynosautov920_usbdrd_phy_ops,
 	.pmu_offset_usbdrd0_phy	= EXYNOSAUTOV920_PHY_CTRL_USB20,
 	.clk_names		= exynos5_clk_names,
@@ -2863,6 +2875,7 @@ static const char * const gs101_regulator_names[] = {
 
 static const struct exynos5_usbdrd_phy_drvdata gs101_usbd31rd_phy = {
 	.phy_cfg			= phy_cfg_gs101,
+	.n_phy_cfg			= ARRAY_SIZE(phy_cfg_gs101),
 	.phy_tunes			= gs101_tunes,
 	.phy_ops			= &gs101_usbdrd_phy_ops,
 	.pmu_offset_usbdrd0_phy		= GS101_PHY_CTRL_USB20,
@@ -3020,7 +3033,7 @@ static int exynos5_usbdrd_phy_probe(struct platform_device *pdev)
 
 	dev_vdbg(dev, "Creating usbdrd_phy phy\n");
 
-	for (i = 0; i < EXYNOS5_DRDPHYS_NUM; i++) {
+	for (i = 0; i < drv_data->n_phy_cfg; i++) {
 		struct phy *phy = devm_phy_create(dev, NULL, drv_data->phy_ops);
 
 		if (IS_ERR(phy))
-- 
2.17.1


-- 
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy

^ permalink raw reply related	[flat|nested] 3+ messages in thread

* Re: [PATCH] phy: exynos5-usbdrd: Use dynamic phy_cfg size to prevent OOB access
  2026-08-31  7:03 ` [PATCH] phy: exynos5-usbdrd: Use dynamic phy_cfg size to prevent OOB access Selvarasu Ganesan
@ 2026-09-02  0:04   ` Łukasz Lebiedziński
  2026-09-03  3:43     ` Selvarasu Ganesan
  0 siblings, 1 reply; 3+ messages in thread
From: Łukasz Lebiedziński @ 2026-09-02  0:04 UTC (permalink / raw)
  To: selvarasu.g
  Cc: akash.m5, alim.akhtar, andre.draszik, dh10.jung, jh0801.jung,
	kernel, krzk, linux-arm-kernel, linux-kernel, linux-phy,
	linux-samsung-soc, muhammed.ali, neil.armstrong, peter.griffin,
	pritam.sutar, thiagu.r, vkoul

I tested this on my Samsung Galaxy A6 (Exynos7870) with this patch applied
on top of mainline. I wanted to verify the OOB read first before trusting
this fix, so I forced the device tree to request PHY index 1
(phys = <&usbdrd_phy 1>;) - this index does not exist for this SoC,
because the phy_cfg_exynos7870[] array contains only one entry. I added
a temporary pr_info() call after assigning phy_cfg in the probe loop to
display the phy_isol/phy_init values using %pS:

  i=0 phy_isol=exynos7870_usbdrd_phy_isol+0x0/0x60
      phy_init=exynos7870_usbdrd_utmi_init+0x0/0x260
  i=1 phy_isol=exynos5_usbdrd_phy_isol+0x0/0x50
      phy_init=exynos5_usbdrd_utmi_init+0x0/0xc8

For i=1, both function pointers resolve to the exynos5_* variants
instead of the exynos7870 ones - the OOB read lands right on
the adjacent phy_cfg_exynos5[] array in .rodata. No crash occurs, since
these garbage pointers happen to point to valid (though unrelated)
kernel functions rather than something invalid.

With the patch applied, the same debug print shows the loop correctly
stopping at n_phy_cfg=1; i=1 is never reached. Boot and USB both work
fine on the actual PHY index (0).

Tested-by: Łukasz Lebiedziński <kernel@lvkasz.us>

-- 
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy

^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [PATCH] phy: exynos5-usbdrd: Use dynamic phy_cfg size to prevent OOB access
  2026-09-02  0:04   ` Łukasz Lebiedziński
@ 2026-09-03  3:43     ` Selvarasu Ganesan
  0 siblings, 0 replies; 3+ messages in thread
From: Selvarasu Ganesan @ 2026-09-03  3:43 UTC (permalink / raw)
  To: Łukasz Lebiedziński
  Cc: akash.m5, alim.akhtar, andre.draszik, dh10.jung, jh0801.jung,
	krzk, linux-arm-kernel, linux-kernel, linux-phy,
	linux-samsung-soc, muhammed.ali, neil.armstrong, peter.griffin,
	pritam.sutar, thiagu.r, vkoul


On 9/2/2026 5:34 AM, Łukasz Lebiedziński wrote:
> I tested this on my Samsung Galaxy A6 (Exynos7870) with this patch applied
> on top of mainline. I wanted to verify the OOB read first before trusting
> this fix, so I forced the device tree to request PHY index 1
> (phys = <&usbdrd_phy 1>;) - this index does not exist for this SoC,
> because the phy_cfg_exynos7870[] array contains only one entry. I added
> a temporary pr_info() call after assigning phy_cfg in the probe loop to
> display the phy_isol/phy_init values using %pS:
>
>    i=0 phy_isol=exynos7870_usbdrd_phy_isol+0x0/0x60
>        phy_init=exynos7870_usbdrd_utmi_init+0x0/0x260
>    i=1 phy_isol=exynos5_usbdrd_phy_isol+0x0/0x50
>        phy_init=exynos5_usbdrd_utmi_init+0x0/0xc8
>
> For i=1, both function pointers resolve to the exynos5_* variants
> instead of the exynos7870 ones - the OOB read lands right on
> the adjacent phy_cfg_exynos5[] array in .rodata. No crash occurs, since
> these garbage pointers happen to point to valid (though unrelated)
> kernel functions rather than something invalid.
>
> With the patch applied, the same debug print shows the loop correctly
> stopping at n_phy_cfg=1; i=1 is never reached. Boot and USB both work
> fine on the actual PHY index (0).
>
> Tested-by: Łukasz Lebiedziński <kernel@lvkasz.us>

Thanks for your testing and update.

Thanks,
Selva
>

-- 
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy

^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2026-09-03  3:43 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
     [not found] <CGME20260831070349epcas5p4c62a4e46592dffe95723ddd51a6068e0@epcas5p4.samsung.com>
2026-08-31  7:03 ` [PATCH] phy: exynos5-usbdrd: Use dynamic phy_cfg size to prevent OOB access Selvarasu Ganesan
2026-09-02  0:04   ` Łukasz Lebiedziński
2026-09-03  3:43     ` Selvarasu Ganesan

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox