| Message ID | 20231017074257.3389177-3-idosch@nvidia.com |
|---|---|
| State | New |
| Headers | show |
| Series | mlxsw: Add support for new reset flow | expand |
Tue, Oct 17, 2023 at 09:42:47AM CEST, idosch@nvidia.com wrote: >Each devlink instance is associated with a parent device and a pointer >to this device is stored in the devlink structure, but devlink does not >hold a reference on this device. > >This is going to be a problem in the next patch where - among other >things - devlink will acquire the device lock during netns dismantle, >before the reload operation. Since netns dismantle is performed >asynchronously and since a reference is not held on the parent device, >it will be possible to hit a use-after-free. > >Prepare for the upcoming change by holding a reference on the parent >device. > Just a note, I'm currently pushing the same patch as a part of my patchset: https://lore.kernel.org/all/20231013121029.353351-4-jiri@resnulli.us/ >Signed-off-by: Ido Schimmel <idosch@nvidia.com> >Reviewed-by: Jiri Pirko <jiri@nvidia.com> >--- > net/devlink/core.c | 3 +++ > 1 file changed, 3 insertions(+) > >diff --git a/net/devlink/core.c b/net/devlink/core.c >index bcbbb952569f..5b8b692b8c76 100644 >--- a/net/devlink/core.c >+++ b/net/devlink/core.c >@@ -4,6 +4,7 @@ > * Copyright (c) 2016 Jiri Pirko <jiri@mellanox.com> > */ > >+#include <linux/device.h> > #include <net/genetlink.h> > #define CREATE_TRACE_POINTS > #include <trace/events/devlink.h> >@@ -310,6 +311,7 @@ static void devlink_release(struct work_struct *work) > > mutex_destroy(&devlink->lock); > lockdep_unregister_key(&devlink->lock_key); >+ put_device(devlink->dev); > kfree(devlink); > } > >@@ -425,6 +427,7 @@ struct devlink *devlink_alloc_ns(const struct devlink_ops *ops, > if (ret < 0) > goto err_xa_alloc; > >+ get_device(dev); > devlink->dev = dev; Nit: devlink->dev = get_device(dev); > devlink->ops = ops; > xa_init_flags(&devlink->ports, XA_FLAGS_ALLOC); >-- >2.40.1 > >
On Tue, Oct 17, 2023 at 09:56:18AM +0200, Jiri Pirko wrote: > Tue, Oct 17, 2023 at 09:42:47AM CEST, idosch@nvidia.com wrote: > >Each devlink instance is associated with a parent device and a pointer > >to this device is stored in the devlink structure, but devlink does not > >hold a reference on this device. > > > >This is going to be a problem in the next patch where - among other > >things - devlink will acquire the device lock during netns dismantle, > >before the reload operation. Since netns dismantle is performed > >asynchronously and since a reference is not held on the parent device, > >it will be possible to hit a use-after-free. > > > >Prepare for the upcoming change by holding a reference on the parent > >device. > > > > Just a note, I'm currently pushing the same patch as a part > of my patchset: > https://lore.kernel.org/all/20231013121029.353351-4-jiri@resnulli.us/ Then you probably need patch #1 as well: https://lore.kernel.org/netdev/20231017074257.3389177-2-idosch@nvidia.com/
Tue, Oct 17, 2023 at 10:11:24AM CEST, idosch@nvidia.com wrote: >On Tue, Oct 17, 2023 at 09:56:18AM +0200, Jiri Pirko wrote: >> Tue, Oct 17, 2023 at 09:42:47AM CEST, idosch@nvidia.com wrote: >> >Each devlink instance is associated with a parent device and a pointer >> >to this device is stored in the devlink structure, but devlink does not >> >hold a reference on this device. >> > >> >This is going to be a problem in the next patch where - among other >> >things - devlink will acquire the device lock during netns dismantle, >> >before the reload operation. Since netns dismantle is performed >> >asynchronously and since a reference is not held on the parent device, >> >it will be possible to hit a use-after-free. >> > >> >Prepare for the upcoming change by holding a reference on the parent >> >device. >> > >> >> Just a note, I'm currently pushing the same patch as a part >> of my patchset: >> https://lore.kernel.org/all/20231013121029.353351-4-jiri@resnulli.us/ > >Then you probably need patch #1 as well: > >https://lore.kernel.org/netdev/20231017074257.3389177-2-idosch@nvidia.com/ Correct.
diff --git a/net/devlink/core.c b/net/devlink/core.c index bcbbb952569f..5b8b692b8c76 100644 --- a/net/devlink/core.c +++ b/net/devlink/core.c @@ -4,6 +4,7 @@ * Copyright (c) 2016 Jiri Pirko <jiri@mellanox.com> */ +#include <linux/device.h> #include <net/genetlink.h> #define CREATE_TRACE_POINTS #include <trace/events/devlink.h> @@ -310,6 +311,7 @@ static void devlink_release(struct work_struct *work) mutex_destroy(&devlink->lock); lockdep_unregister_key(&devlink->lock_key); + put_device(devlink->dev); kfree(devlink); } @@ -425,6 +427,7 @@ struct devlink *devlink_alloc_ns(const struct devlink_ops *ops, if (ret < 0) goto err_xa_alloc; + get_device(dev); devlink->dev = dev; devlink->ops = ops; xa_init_flags(&devlink->ports, XA_FLAGS_ALLOC);