public inbox for netdev@vger.kernel.org
 help / color / mirror / Atom feed
* [PATCH net] net: wan: framer: fix potential UAF in framer_provider_simple_of_xlate()
@ 2026-02-19 23:40 Dmitry Torokhov
  2026-02-21  0:25 ` Jakub Kicinski
  0 siblings, 1 reply; 4+ messages in thread
From: Dmitry Torokhov @ 2026-02-19 23:40 UTC (permalink / raw)
  To: Andrew Lunn
  Cc: Andrew Lunn, David S. Miller, Eric Dumazet, Jakub Kicinski,
	Paolo Abeni, Rafael J. Wysocki, Simon Horman, Zijun Hu, netdev,
	linux-kernel

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 <dmitry.torokhov@gmail.com>
---
 drivers/net/wan/framer/framer-core.c | 3 ---
 1 file changed, 3 deletions(-)

diff --git a/drivers/net/wan/framer/framer-core.c b/drivers/net/wan/framer/framer-core.c
index bf7ac7dd2804..397fabc3da4e 100644
--- a/drivers/net/wan/framer/framer-core.c
+++ b/drivers/net/wan/framer/framer-core.c
@@ -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);
-- 
2.53.0.345.g96ddfc5eaa-goog


-- 
Dmitry

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

* Re: [PATCH net] net: wan: framer: fix potential UAF in framer_provider_simple_of_xlate()
  2026-02-19 23:40 [PATCH net] net: wan: framer: fix potential UAF in framer_provider_simple_of_xlate() Dmitry Torokhov
@ 2026-02-21  0:25 ` Jakub Kicinski
  2026-02-21  0:56   ` Dmitry Torokhov
  0 siblings, 1 reply; 4+ messages in thread
From: Jakub Kicinski @ 2026-02-21  0:25 UTC (permalink / raw)
  To: Dmitry Torokhov
  Cc: Andrew Lunn, David S. Miller, Eric Dumazet, Paolo Abeni,
	Rafael J. Wysocki, Simon Horman, Zijun Hu, netdev, linux-kernel

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?

> Fixes: dcacb364772e ("net: wan: framer: Simplify API framer_provider_simple_of_xlate() implementation")
> Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
> ---
>  drivers/net/wan/framer/framer-core.c | 3 ---
>  1 file changed, 3 deletions(-)
> 
> diff --git a/drivers/net/wan/framer/framer-core.c b/drivers/net/wan/framer/framer-core.c
> index bf7ac7dd2804..397fabc3da4e 100644
> --- a/drivers/net/wan/framer/framer-core.c
> +++ b/drivers/net/wan/framer/framer-core.c
> @@ -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

>  	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.

>  }
>  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?
-- 
pw-bot: cr

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

* Re: [PATCH net] net: wan: framer: fix potential UAF in framer_provider_simple_of_xlate()
  2026-02-21  0:25 ` Jakub Kicinski
@ 2026-02-21  0:56   ` Dmitry Torokhov
  2026-02-21  1:13     ` Jakub Kicinski
  0 siblings, 1 reply; 4+ messages in thread
From: Dmitry Torokhov @ 2026-02-21  0:56 UTC (permalink / raw)
  To: Jakub Kicinski
  Cc: Andrew Lunn, David S. Miller, Eric Dumazet, Paolo Abeni,
	Rafael J. Wysocki, Simon Horman, Zijun Hu, netdev, linux-kernel

On Fri, Feb 20, 2026 at 04:25:53PM -0800, Jakub Kicinski wrote:
> 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.

> 
> > Fixes: dcacb364772e ("net: wan: framer: Simplify API framer_provider_simple_of_xlate() implementation")
> > Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
> > ---
> >  drivers/net/wan/framer/framer-core.c | 3 ---
> >  1 file changed, 3 deletions(-)
> > 
> > diff --git a/drivers/net/wan/framer/framer-core.c b/drivers/net/wan/framer/framer-core.c
> > index bf7ac7dd2804..397fabc3da4e 100644
> > --- a/drivers/net/wan/framer/framer-core.c
> > +++ b/drivers/net/wan/framer/framer-core.c
> > @@ -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

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.

> 
> >  	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?)

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

> 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.

> 
> >  }
> >  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.

-- 
Dmitry

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

* Re: [PATCH net] net: wan: framer: fix potential UAF in framer_provider_simple_of_xlate()
  2026-02-21  0:56   ` Dmitry Torokhov
@ 2026-02-21  1:13     ` Jakub Kicinski
  0 siblings, 0 replies; 4+ messages in thread
From: Jakub Kicinski @ 2026-02-21  1:13 UTC (permalink / raw)
  To: Dmitry Torokhov
  Cc: Andrew Lunn, David S. Miller, Eric Dumazet, Paolo Abeni,
	Rafael J. Wysocki, Simon Horman, Zijun Hu, netdev, linux-kernel,
	Herve Codina

On Fri, 20 Feb 2026 16:56:12 -0800 Dmitry Torokhov wrote:
> On Fri, Feb 20, 2026 at 04:25:53PM -0800, Jakub Kicinski wrote:
> > 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.

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

> > 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?

> > >  	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?)  
> 
> 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..

> > >  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 :/

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

end of thread, other threads:[~2026-02-21  1:13 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-02-19 23:40 [PATCH net] net: wan: framer: fix potential UAF in framer_provider_simple_of_xlate() Dmitry Torokhov
2026-02-21  0:25 ` Jakub Kicinski
2026-02-21  0:56   ` Dmitry Torokhov
2026-02-21  1:13     ` Jakub Kicinski

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