Skip to contentSkip to conversation

net: wan: framer: fix potential UAF in framer_provider_simple_of_xlate()

PATCH In review Dmitry Torokhov 2 participants 4 messages Feb 19 – Feb 21

Subsystems (3)
Networking Network drivers Lantiq PEF2256 driver
lore
Share & tools
lore

Dmitry Torokhov

author
#0 lore raw

PATCH 1/1 net: wan: framer: fix potential UAF in framer_provider_simple_of_xlate()

The implementation put_device()s located device and then uses container_of() on the pointer. The device may disappear by that time, resulting in UAF.

Fix the problem by keeping the reference to the framer device, and avoid getting an extra reference to it in framer_get().

Fixes dcacb364772e ("net: wan: framer: Simplify API framer_provider_simple_of_xlate() implementation")
Signed-off-by Dmitry Torokhov <[email protected]>
---
 drivers/net/wan/framer/framer-core.c | 3 ---
 1 file changed, 3 deletions(-)
drivers/net/wan/framer/framer-core.c +0 -3
index bf7ac7dd2804..397fabc3da4e 100644
@@ -482,8 +482,6 @@ struct framer *framer_get(struct device *dev, const char *con_id)
 	if (IS_ERR(framer))
 		return framer;
 
-	get_device(&framer->dev);
-
 	if (!try_module_get(framer->ops->owner)) {
 		ret = -EPROBE_DEFER;
 		goto err_put_device;

@@ -749,7 +747,6 @@ struct framer *framer_provider_simple_of_xlate(struct device *dev,
 	if (!target_dev)
 		return ERR_PTR(-ENODEV);
 
-	put_device(target_dev);
 	return dev_to_framer(target_dev);
 }
 EXPORT_SYMBOL_GPL(framer_provider_simple_of_xlate);
Signature · 5 lines
2.53.0.345.g96ddfc5eaa-goog


-- 
Dmitry
maintainer
#1 lore raw
Quoted 6 lines · On Thu, 19 Feb 2026 15:40:57 -0800 Dmitry Torokhov wrote:

The implementation put_device()s located device and then uses container_of() on the pointer. The device may disappear by that time, resulting in UAF.

Fix the problem by keeping the reference to the framer device, and avoid getting an extra reference to it in framer_get().

I have failed to understand what you are talking about after looking at this for 15min :S Please write better commit messages?

Quoted 11 lines
Fixes dcacb364772e ("net: wan: framer: Simplify API framer_provider_simple_of_xlate() implementation")
Signed-off-by Dmitry Torokhov <[email protected]>
---
 drivers/net/wan/framer/framer-core.c | 3 ---
 1 file changed, 3 deletions(-)
drivers/net/wan/framer/framer-core.c +0 -0
index bf7ac7dd2804..397fabc3da4e 100644

drivers/net/wan/framer/framer-core.c struct framer *framer_get(struct device *dev, const char *con_id) · line 485

Quoted 4 lines
drivers/net/wan/framer/framer-core.c +0 -1
@@ -482,8 +482,6 @@ struct framer *framer_get(struct device *dev, const char *con_id)
 	if (IS_ERR(framer))
 		return framer;
 
-	get_device(&framer->dev);

AFAICT this get_device() does not pair with the put_device() you are removing

drivers/net/wan/framer/framer-core.c struct framer *framer_provider_simple_of_xlate(struct device *dev, · line 750

Quoted 9 lines
drivers/net/wan/framer/framer-core.c +0 -1
 	if (!try_module_get(framer->ops->owner)) {
 		ret = -EPROBE_DEFER;
 		goto err_put_device;

@@ -749,7 +747,6 @@ struct framer *framer_provider_simple_of_xlate(struct device *dev,
 	if (!target_dev)
 		return ERR_PTR(-ENODEV);
 
-	put_device(target_dev);
 	return dev_to_framer(target_dev);

The only caller of this function does not dereference the pointer (no idea why it even calls it, for some setup validation?) Calling container_of() on a stale pointer is safe.

drivers/net/wan/framer/framer-core.c struct framer *framer_provider_simple_of_xlate(struct device *dev, · line 752

Quoted 2 lines
drivers/net/wan/framer/framer-core.c +0 -0
 }
 EXPORT_SYMBOL_GPL(framer_provider_simple_of_xlate);

I'm kinda curious about the backstory for this patch.. What made you look at this code?

Signature · 1 line
pw-bot: cr
author
#2 lore raw
Quoted 10 lines · Jakub Kicinski
Quoted 6 lines · On Thu, 19 Feb 2026 15:40:57 -0800 Dmitry Torokhov wrote:

The implementation put_device()s located device and then uses container_of() on the pointer. The device may disappear by that time, resulting in UAF.

Fix the problem by keeping the reference to the framer device, and avoid getting an extra reference to it in framer_get().

I have failed to understand what you are talking about after looking at this for 15min :S Please write better commit messages?

Yeah, I should probably rephrase it, container_of() is not that important.

The core of the issue that once you do put_device() it may disappear, so when you do

	return dev_to_framer(target_dev);

the returned pointer may no longer point to the valid framer device. The memory may get used for something else entirely.

You have to hold on to the reference until you are completely done with the device.

Quoted 15 lines
Quoted 15 lines
Fixes dcacb364772e ("net: wan: framer: Simplify API framer_provider_simple_of_xlate() implementation")
Signed-off-by Dmitry Torokhov <[email protected]>
---
 drivers/net/wan/framer/framer-core.c | 3 ---
 1 file changed, 3 deletions(-)
drivers/net/wan/framer/framer-core.c +0 -0
index bf7ac7dd2804..397fabc3da4e 100644
@@ -482,8 +482,6 @@ struct framer *framer_get(struct device *dev, const char *con_id)
 	if (IS_ERR(framer))
 		return framer;
Quoted 4 lines

drivers/net/wan/framer/framer-core.c struct framer *framer_get(struct device *dev, const char *con_id) · line 485

Quoted 1 line
drivers/net/wan/framer/framer-core.c +0 -1
-	get_device(&framer->dev);

AFAICT this get_device() does not pair with the put_device() you are removing

It does not "pair", but it tries to bump up a reference to the device we just did "put" on in framer_provider_simple_of_xlate(). If we remove put there as I propose then we do not need to do it here, or we'll end up with an extra reference.

Quoted 8 lines
Quoted 8 lines
drivers/net/wan/framer/framer-core.c +0 -0
 	if (!try_module_get(framer->ops->owner)) {
 		ret = -EPROBE_DEFER;
 		goto err_put_device;

@@ -749,7 +747,6 @@ struct framer *framer_provider_simple_of_xlate(struct device *dev,
 	if (!target_dev)
 		return ERR_PTR(-ENODEV);
Quoted 5 lines

drivers/net/wan/framer/framer-core.c struct framer *framer_provider_simple_of_xlate(struct device *dev, · line 750

Quoted 2 lines
drivers/net/wan/framer/framer-core.c +0 -1
-	put_device(target_dev);
 	return dev_to_framer(target_dev);

The only caller of this function does not dereference the pointer (no idea why it even calls it, for some setup validation?)

The returned pointer ends up in framer_get() through a few layers.

Quoted 1 line

Calling container_of() on a stale pointer is safe.

But using what it returns isn't. And I am sure there is an arcane C rule that disallows container_of and gives compiler a license to kill a kitten.

Quoted 6 lines

drivers/net/wan/framer/framer-core.c struct framer *framer_provider_simple_of_xlate(struct device *dev, · line 752

Quoted 3 lines
drivers/net/wan/framer/framer-core.c +0 -0
 }
 EXPORT_SYMBOL_GPL(framer_provider_simple_of_xlate);

I'm kinda curious about the backstory for this patch.. What made you look at this code?

I want to remove class_find_device_by_of_node() in favor of class_find_device_by_fwnode() so I happened to look at the code.

Thanks.

Signature · 1 line
Dmitry
maintainer
#3 lore raw
Quoted 10 lines · On Fri, 20 Feb 2026 16:56:12 -0800 Dmitry Torokhov wrote:
Quoted 2 lines · Jakub Kicinski

I have failed to understand what you are talking about after looking at this for 15min :S Please write better commit messages?

Yeah, I should probably rephrase it, container_of() is not that important.

The core of the issue that once you do put_device() it may disappear, so when you do

Quoted 7 lines
	return dev_to_framer(target_dev);

the returned pointer may no longer point to the valid framer device. The memory may get used for something else entirely.

You have to hold on to the reference until you are completely done with the device.

I meant you should explain the code paths that are involved.

Quoted 7 lines
Quoted 2 lines

AFAICT this get_device() does not pair with the put_device() you are removing

It does not "pair", but it tries to bump up a reference to the device we just did "put" on in framer_provider_simple_of_xlate(). If we remove put there as I propose then we do not need to do it here, or we'll end up with an extra reference.

Yes but there seem to be other callers to framer_get() which pair with framer_put() and no involvement of framer_provider_simple_of_xlate(). framer_codec_probe() for example?

Quoted 7 lines
Quoted 7 lines
Quoted 7 lines
drivers/net/wan/framer/framer-core.c +0 -0
 	if (!try_module_get(framer->ops->owner)) {
 		ret = -EPROBE_DEFER;
 		goto err_put_device;

@@ -749,7 +747,6 @@ struct framer *framer_provider_simple_of_xlate(struct device *dev,
 	if (!target_dev)
 		return ERR_PTR(-ENODEV);
Quoted 7 lines
Quoted 5 lines

drivers/net/wan/framer/framer-core.c struct framer *framer_provider_simple_of_xlate(struct device *dev, · line 750

Quoted 2 lines
drivers/net/wan/framer/framer-core.c +0 -1
-	put_device(target_dev);
 	return dev_to_framer(target_dev);

The only caller of this function does not dereference the pointer (no idea why it even calls it, for some setup validation?)

The returned pointer ends up in framer_get() through a few layers.

Ack, I think I see it now, thru the ->of_xlate() saved in the provider. This is the kind of basic detail that should be in the commit msg..

Quoted 7 lines
Quoted 4 lines

drivers/net/wan/framer/framer-core.c struct framer *framer_provider_simple_of_xlate(struct device *dev, · line 752

Quoted 1 line
drivers/net/wan/framer/framer-core.c +0 -0
 EXPORT_SYMBOL_GPL(framer_provider_simple_of_xlate);

I'm kinda curious about the backstory for this patch.. What made you look at this code?

I want to remove class_find_device_by_of_node() in favor of class_find_device_by_fwnode() so I happened to look at the code.

Good luck :)

BTW when you repost please make sure you CC Herve, looks like the MAINTAINERS entry for framer only covers one driver :/