When executing usb_add_phy() and usb_add_phy_dev() it is possible that usb_add_extcon() fails (for example with -EPROBE_DEFER), in which case the usb_phy does not get added to phy_list via list_add_tail(&x->head, phy_list).
Then, when the driver that tried to add the phy receives the error propagated from usb_add_extcon() and calls into usb_remove_phy() to undo the partial registration there will be an unconditional call to list_del(&x->head) which is notinitialized and leads to a NULL pointer dereference.
Fix this by initializing x->head before usb_add_extcon() has a chance to fail.
Fixes: 7d21114dc6a2d53 ("usb: phy: Introduce one extcon device into usb phy") Cc: stable@vger.kernel.org Signed-off-by: Diogo Ivo diogo.ivo@tecnico.ulisboa.pt --- drivers/usb/phy/phy.c | 4 ++++ 1 file changed, 4 insertions(+)
diff --git a/drivers/usb/phy/phy.c b/drivers/usb/phy/phy.c index e1435bc59662..5a9b9353f343 100644 --- a/drivers/usb/phy/phy.c +++ b/drivers/usb/phy/phy.c @@ -646,6 +646,8 @@ int usb_add_phy(struct usb_phy *x, enum usb_phy_type type) return -EINVAL; }
+ INIT_LIST_HEAD(&x->head); + usb_charger_init(x); ret = usb_add_extcon(x); if (ret) @@ -696,6 +698,8 @@ int usb_add_phy_dev(struct usb_phy *x) return -EINVAL; }
+ INIT_LIST_HEAD(&x->head); + usb_charger_init(x); ret = usb_add_extcon(x); if (ret)
--- base-commit: 35d084745b3ea4af571ed421844f2bb1a99ad6e2 change-id: 20251113-diogo-smaug_typec-56b2059b892b
Best regards,
On Thu, Nov 13, 2025 at 02:59:06PM +0000, Diogo Ivo wrote:
When executing usb_add_phy() and usb_add_phy_dev() it is possible that usb_add_extcon() fails (for example with -EPROBE_DEFER), in which case the usb_phy does not get added to phy_list via list_add_tail(&x->head, phy_list).
Then, when the driver that tried to add the phy receives the error propagated from usb_add_extcon() and calls into usb_remove_phy() to undo the partial registration there will be an unconditional call to list_del(&x->head) which is notinitialized and leads to a NULL pointer dereference.
Fix this by initializing x->head before usb_add_extcon() has a chance to fail.
Fixes: 7d21114dc6a2d53 ("usb: phy: Introduce one extcon device into usb phy") Cc: stable@vger.kernel.org Signed-off-by: Diogo Ivo diogo.ivo@tecnico.ulisboa.pt
drivers/usb/phy/phy.c | 4 ++++ 1 file changed, 4 insertions(+)
diff --git a/drivers/usb/phy/phy.c b/drivers/usb/phy/phy.c index e1435bc59662..5a9b9353f343 100644 --- a/drivers/usb/phy/phy.c +++ b/drivers/usb/phy/phy.c @@ -646,6 +646,8 @@ int usb_add_phy(struct usb_phy *x, enum usb_phy_type type) return -EINVAL; }
- INIT_LIST_HEAD(&x->head);
- usb_charger_init(x); ret = usb_add_extcon(x); if (ret)
@@ -696,6 +698,8 @@ int usb_add_phy_dev(struct usb_phy *x) return -EINVAL; }
- INIT_LIST_HEAD(&x->head);
- usb_charger_init(x); ret = usb_add_extcon(x); if (ret)
Shouldn't you be also removing an existing call to INIT_LIST_HEAD() somewhere? This is not "moving" the code, it is adding it.
thanks,
greg k-h
On 11/21/25 14:09, Greg Kroah-Hartman wrote:
On Thu, Nov 13, 2025 at 02:59:06PM +0000, Diogo Ivo wrote:
When executing usb_add_phy() and usb_add_phy_dev() it is possible that usb_add_extcon() fails (for example with -EPROBE_DEFER), in which case the usb_phy does not get added to phy_list via list_add_tail(&x->head, phy_list).
Then, when the driver that tried to add the phy receives the error propagated from usb_add_extcon() and calls into usb_remove_phy() to undo the partial registration there will be an unconditional call to list_del(&x->head) which is notinitialized and leads to a NULL pointer dereference.
Fix this by initializing x->head before usb_add_extcon() has a chance to fail.
Fixes: 7d21114dc6a2d53 ("usb: phy: Introduce one extcon device into usb phy") Cc: stable@vger.kernel.org Signed-off-by: Diogo Ivo diogo.ivo@tecnico.ulisboa.pt
drivers/usb/phy/phy.c | 4 ++++ 1 file changed, 4 insertions(+)
diff --git a/drivers/usb/phy/phy.c b/drivers/usb/phy/phy.c index e1435bc59662..5a9b9353f343 100644 --- a/drivers/usb/phy/phy.c +++ b/drivers/usb/phy/phy.c @@ -646,6 +646,8 @@ int usb_add_phy(struct usb_phy *x, enum usb_phy_type type) return -EINVAL; }
- INIT_LIST_HEAD(&x->head);
- usb_charger_init(x); ret = usb_add_extcon(x); if (ret)
@@ -696,6 +698,8 @@ int usb_add_phy_dev(struct usb_phy *x) return -EINVAL; }
- INIT_LIST_HEAD(&x->head);
- usb_charger_init(x); ret = usb_add_extcon(x); if (ret)
Shouldn't you be also removing an existing call to INIT_LIST_HEAD() somewhere? This is not "moving" the code, it is adding it.
From my understanding that's exactly the problem, currently there is no call to INIT_LIST_HEAD() anywhere on these code paths, meaning that if we do not reach the point of calling list_add_tail() at the end of usb_add_phy() and usb_phy_add_dev() then x->head will remain uninitialized and fault when running usb_remove_phy().
Best regards, Diogo
thanks,
greg k-h
On Fri, Nov 21, 2025 at 02:55:35PM +0000, Diogo Ivo wrote:
On 11/21/25 14:09, Greg Kroah-Hartman wrote:
On Thu, Nov 13, 2025 at 02:59:06PM +0000, Diogo Ivo wrote:
When executing usb_add_phy() and usb_add_phy_dev() it is possible that usb_add_extcon() fails (for example with -EPROBE_DEFER), in which case the usb_phy does not get added to phy_list via list_add_tail(&x->head, phy_list).
Then, when the driver that tried to add the phy receives the error propagated from usb_add_extcon() and calls into usb_remove_phy() to undo the partial registration there will be an unconditional call to list_del(&x->head) which is notinitialized and leads to a NULL pointer dereference.
Fix this by initializing x->head before usb_add_extcon() has a chance to fail.
Fixes: 7d21114dc6a2d53 ("usb: phy: Introduce one extcon device into usb phy") Cc: stable@vger.kernel.org Signed-off-by: Diogo Ivo diogo.ivo@tecnico.ulisboa.pt
drivers/usb/phy/phy.c | 4 ++++ 1 file changed, 4 insertions(+)
diff --git a/drivers/usb/phy/phy.c b/drivers/usb/phy/phy.c index e1435bc59662..5a9b9353f343 100644 --- a/drivers/usb/phy/phy.c +++ b/drivers/usb/phy/phy.c @@ -646,6 +646,8 @@ int usb_add_phy(struct usb_phy *x, enum usb_phy_type type) return -EINVAL; }
- INIT_LIST_HEAD(&x->head);
- usb_charger_init(x); ret = usb_add_extcon(x); if (ret)
@@ -696,6 +698,8 @@ int usb_add_phy_dev(struct usb_phy *x) return -EINVAL; }
- INIT_LIST_HEAD(&x->head);
- usb_charger_init(x); ret = usb_add_extcon(x); if (ret)
Shouldn't you be also removing an existing call to INIT_LIST_HEAD() somewhere? This is not "moving" the code, it is adding it.
From my understanding that's exactly the problem, currently there is no call to INIT_LIST_HEAD() anywhere on these code paths, meaning that if we do not reach the point of calling list_add_tail() at the end of usb_add_phy() and usb_phy_add_dev() then x->head will remain uninitialized and fault when running usb_remove_phy().
Then how does this work at all if the list is never initialized?
thanks,
greg k-h
On 11/21/25 15:03, Greg Kroah-Hartman wrote:
On Fri, Nov 21, 2025 at 02:55:35PM +0000, Diogo Ivo wrote:
On 11/21/25 14:09, Greg Kroah-Hartman wrote:
On Thu, Nov 13, 2025 at 02:59:06PM +0000, Diogo Ivo wrote:
When executing usb_add_phy() and usb_add_phy_dev() it is possible that usb_add_extcon() fails (for example with -EPROBE_DEFER), in which case the usb_phy does not get added to phy_list via list_add_tail(&x->head, phy_list).
Then, when the driver that tried to add the phy receives the error propagated from usb_add_extcon() and calls into usb_remove_phy() to undo the partial registration there will be an unconditional call to list_del(&x->head) which is notinitialized and leads to a NULL pointer dereference.
Fix this by initializing x->head before usb_add_extcon() has a chance to fail.
Fixes: 7d21114dc6a2d53 ("usb: phy: Introduce one extcon device into usb phy") Cc: stable@vger.kernel.org Signed-off-by: Diogo Ivo diogo.ivo@tecnico.ulisboa.pt
drivers/usb/phy/phy.c | 4 ++++ 1 file changed, 4 insertions(+)
diff --git a/drivers/usb/phy/phy.c b/drivers/usb/phy/phy.c index e1435bc59662..5a9b9353f343 100644 --- a/drivers/usb/phy/phy.c +++ b/drivers/usb/phy/phy.c @@ -646,6 +646,8 @@ int usb_add_phy(struct usb_phy *x, enum usb_phy_type type) return -EINVAL; }
- INIT_LIST_HEAD(&x->head);
- usb_charger_init(x); ret = usb_add_extcon(x); if (ret)
@@ -696,6 +698,8 @@ int usb_add_phy_dev(struct usb_phy *x) return -EINVAL; }
- INIT_LIST_HEAD(&x->head);
- usb_charger_init(x); ret = usb_add_extcon(x); if (ret)
Shouldn't you be also removing an existing call to INIT_LIST_HEAD() somewhere? This is not "moving" the code, it is adding it.
From my understanding that's exactly the problem, currently there is no call to INIT_LIST_HEAD() anywhere on these code paths, meaning that if we do not reach the point of calling list_add_tail() at the end of usb_add_phy() and usb_phy_add_dev() then x->head will remain uninitialized and fault when running usb_remove_phy().
Then how does this work at all if the list is never initialized?
In this case in drivers/usb/phy/phy.c a static LIST_HEAD(phy_list) is declared and then for each new 'struct usb_phy *x' the x->head entry will get added to this list by calling list_add_tail(&x->head, &phy_list).
Best regards, Diogo
thanks,
greg k-h
On Fri, Nov 21, 2025 at 04:39:58PM +0000, Diogo Ivo wrote:
On 11/21/25 15:03, Greg Kroah-Hartman wrote:
On Fri, Nov 21, 2025 at 02:55:35PM +0000, Diogo Ivo wrote:
On 11/21/25 14:09, Greg Kroah-Hartman wrote:
On Thu, Nov 13, 2025 at 02:59:06PM +0000, Diogo Ivo wrote:
When executing usb_add_phy() and usb_add_phy_dev() it is possible that usb_add_extcon() fails (for example with -EPROBE_DEFER), in which case the usb_phy does not get added to phy_list via list_add_tail(&x->head, phy_list).
Then, when the driver that tried to add the phy receives the error propagated from usb_add_extcon() and calls into usb_remove_phy() to undo the partial registration there will be an unconditional call to list_del(&x->head) which is notinitialized and leads to a NULL pointer dereference.
Fix this by initializing x->head before usb_add_extcon() has a chance to fail.
Fixes: 7d21114dc6a2d53 ("usb: phy: Introduce one extcon device into usb phy") Cc: stable@vger.kernel.org Signed-off-by: Diogo Ivo diogo.ivo@tecnico.ulisboa.pt
drivers/usb/phy/phy.c | 4 ++++ 1 file changed, 4 insertions(+)
diff --git a/drivers/usb/phy/phy.c b/drivers/usb/phy/phy.c index e1435bc59662..5a9b9353f343 100644 --- a/drivers/usb/phy/phy.c +++ b/drivers/usb/phy/phy.c @@ -646,6 +646,8 @@ int usb_add_phy(struct usb_phy *x, enum usb_phy_type type) return -EINVAL; }
- INIT_LIST_HEAD(&x->head);
- usb_charger_init(x); ret = usb_add_extcon(x); if (ret)
@@ -696,6 +698,8 @@ int usb_add_phy_dev(struct usb_phy *x) return -EINVAL; }
- INIT_LIST_HEAD(&x->head);
- usb_charger_init(x); ret = usb_add_extcon(x); if (ret)
Shouldn't you be also removing an existing call to INIT_LIST_HEAD() somewhere? This is not "moving" the code, it is adding it.
From my understanding that's exactly the problem, currently there is no call to INIT_LIST_HEAD() anywhere on these code paths, meaning that if we do not reach the point of calling list_add_tail() at the end of usb_add_phy() and usb_phy_add_dev() then x->head will remain uninitialized and fault when running usb_remove_phy().
Then how does this work at all if the list is never initialized?
In this case in drivers/usb/phy/phy.c a static LIST_HEAD(phy_list) is declared and then for each new 'struct usb_phy *x' the x->head entry will get added to this list by calling list_add_tail(&x->head, &phy_list).
Great, can you document this in the changelog text so that it makes more sense (and properly quote the fixes: sha1, I think you have too many digits there...) and resend a v2.
thanks,
greg k-h
On 11/21/25 16:48, Greg Kroah-Hartman wrote:
On Fri, Nov 21, 2025 at 04:39:58PM +0000, Diogo Ivo wrote:
On 11/21/25 15:03, Greg Kroah-Hartman wrote:
On Fri, Nov 21, 2025 at 02:55:35PM +0000, Diogo Ivo wrote:
On 11/21/25 14:09, Greg Kroah-Hartman wrote:
On Thu, Nov 13, 2025 at 02:59:06PM +0000, Diogo Ivo wrote:
When executing usb_add_phy() and usb_add_phy_dev() it is possible that usb_add_extcon() fails (for example with -EPROBE_DEFER), in which case the usb_phy does not get added to phy_list via list_add_tail(&x->head, phy_list).
Then, when the driver that tried to add the phy receives the error propagated from usb_add_extcon() and calls into usb_remove_phy() to undo the partial registration there will be an unconditional call to list_del(&x->head) which is notinitialized and leads to a NULL pointer dereference.
Fix this by initializing x->head before usb_add_extcon() has a chance to fail.
Fixes: 7d21114dc6a2d53 ("usb: phy: Introduce one extcon device into usb phy") Cc: stable@vger.kernel.org Signed-off-by: Diogo Ivo diogo.ivo@tecnico.ulisboa.pt
drivers/usb/phy/phy.c | 4 ++++ 1 file changed, 4 insertions(+)diff --git a/drivers/usb/phy/phy.c b/drivers/usb/phy/phy.c index e1435bc59662..5a9b9353f343 100644 --- a/drivers/usb/phy/phy.c +++ b/drivers/usb/phy/phy.c @@ -646,6 +646,8 @@ int usb_add_phy(struct usb_phy *x, enum usb_phy_type type) return -EINVAL; }
- INIT_LIST_HEAD(&x->head);
usb_charger_init(x); ret = usb_add_extcon(x); if (ret)@@ -696,6 +698,8 @@ int usb_add_phy_dev(struct usb_phy *x) return -EINVAL; }
- INIT_LIST_HEAD(&x->head);
usb_charger_init(x); ret = usb_add_extcon(x); if (ret)Shouldn't you be also removing an existing call to INIT_LIST_HEAD() somewhere? This is not "moving" the code, it is adding it.
From my understanding that's exactly the problem, currently there is no call to INIT_LIST_HEAD() anywhere on these code paths, meaning that if we do not reach the point of calling list_add_tail() at the end of usb_add_phy() and usb_phy_add_dev() then x->head will remain uninitialized and fault when running usb_remove_phy().
Then how does this work at all if the list is never initialized?
In this case in drivers/usb/phy/phy.c a static LIST_HEAD(phy_list) is declared and then for each new 'struct usb_phy *x' the x->head entry will get added to this list by calling list_add_tail(&x->head, &phy_list).
Great, can you document this in the changelog text so that it makes more sense (and properly quote the fixes: sha1, I think you have too many digits there...) and resend a v2.
Yes, I'll fix it and send out a v2.
Thanks, Diogo
thanks,
greg k-h
linux-stable-mirror@lists.linaro.org
-
Diogo Ivo -
Greg Kroah-Hartman