| Message ID | 20210322002637.3412657-1-olteanv@gmail.com (mailing list archive) |
|---|---|
| State | Accepted |
| Commit | a50a151e311bd3a793ebe4e5f233db8bfad0b78f |
| Delegated to: | Netdev Maintainers |
| Headers | show |
| Series | [net] net: ipconfig: ic_dev can be NULL in ic_close_devs | expand |
| Context | Check | Description |
|---|---|---|
| netdev/cover_letter | success | Link |
| netdev/fixes_present | success | Link |
| netdev/patch_count | success | Link |
| netdev/tree_selection | success | Clearly marked for net |
| netdev/subject_prefix | success | Link |
| netdev/cc_maintainers | success | CCed 8 of 8 maintainers |
| netdev/source_inline | success | Was 0 now: 0 |
| netdev/verify_signedoff | success | Link |
| netdev/module_param | success | Was 0 now: 0 |
| netdev/build_32bit | success | Errors and warnings before: 1 this patch: 1 |
| netdev/kdoc | success | Errors and warnings before: 0 this patch: 0 |
| netdev/verify_fixes | success | Link |
| netdev/checkpatch | success | total: 0 errors, 0 warnings, 0 checks, 31 lines checked |
| netdev/build_allmodconfig_warn | success | Errors and warnings before: 1 this patch: 1 |
| netdev/header_inline | success | Link |
Hi Vladimir, > ic_close_dev contains a generalization of the logic to not close a > network interface if it's the host port for a DSA switch. This logic is > disguised behind an iteration through the lowers of ic_dev in > ic_close_dev. > > When no interface for ipconfig can be found, ic_dev is NULL, and > ic_close_dev: > - dereferences a NULL pointer when assigning selected_dev > - would attempt to search through the lower interfaces of a NULL > net_device pointer > > So we should protect against that case. > > The "lower_dev" iterator variable was shortened to "lower" in order to > keep the 80 character limit. > > Fixes: f68cbaed67cb ("net: ipconfig: avoid use-after-free in ic_close_devs") > Fixes: 46acf7bdbc72 ("Revert "net: ipv4: handle DSA enabled master network devices"") > Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com> Tested-by: Heiko Thiery <heiko.thiery@gmail.com> > --- > net/ipv4/ipconfig.c | 14 ++++++++------ > 1 file changed, 8 insertions(+), 6 deletions(-) > > diff --git a/net/ipv4/ipconfig.c b/net/ipv4/ipconfig.c > index 47db1bfdaaa0..bc2f6ca97152 100644 > --- a/net/ipv4/ipconfig.c > +++ b/net/ipv4/ipconfig.c > @@ -309,7 +309,7 @@ static int __init ic_open_devs(void) > */ > static void __init ic_close_devs(void) > { > - struct net_device *selected_dev = ic_dev->dev; > + struct net_device *selected_dev = ic_dev ? ic_dev->dev : NULL; > struct ic_device *d, *next; > struct net_device *dev; > > @@ -317,16 +317,18 @@ static void __init ic_close_devs(void) > next = ic_first_dev; > while ((d = next)) { > bool bring_down = (d != ic_dev); > - struct net_device *lower_dev; > + struct net_device *lower; > struct list_head *iter; > > next = d->next; > dev = d->dev; > > - netdev_for_each_lower_dev(selected_dev, lower_dev, iter) { > - if (dev == lower_dev) { > - bring_down = false; > - break; > + if (selected_dev) { > + netdev_for_each_lower_dev(selected_dev, lower, iter) { > + if (dev == lower) { > + bring_down = false; > + break; > + } > } > } > if (bring_down) { > -- > 2.25.1 Thank you.
Hello: This patch was applied to netdev/net.git (refs/heads/master): On Mon, 22 Mar 2021 02:26:37 +0200 you wrote: > From: Vladimir Oltean <vladimir.oltean@nxp.com> > > ic_close_dev contains a generalization of the logic to not close a > network interface if it's the host port for a DSA switch. This logic is > disguised behind an iteration through the lowers of ic_dev in > ic_close_dev. > > [...] Here is the summary with links: - [net] net: ipconfig: ic_dev can be NULL in ic_close_devs https://git.kernel.org/netdev/net/c/a50a151e311b You are awesome, thank you! -- Deet-doot-dot, I am a bot. https://korg.docs.kernel.org/patchwork/pwbot.html
diff --git a/net/ipv4/ipconfig.c b/net/ipv4/ipconfig.c index 47db1bfdaaa0..bc2f6ca97152 100644 --- a/net/ipv4/ipconfig.c +++ b/net/ipv4/ipconfig.c @@ -309,7 +309,7 @@ static int __init ic_open_devs(void) */ static void __init ic_close_devs(void) { - struct net_device *selected_dev = ic_dev->dev; + struct net_device *selected_dev = ic_dev ? ic_dev->dev : NULL; struct ic_device *d, *next; struct net_device *dev; @@ -317,16 +317,18 @@ static void __init ic_close_devs(void) next = ic_first_dev; while ((d = next)) { bool bring_down = (d != ic_dev); - struct net_device *lower_dev; + struct net_device *lower; struct list_head *iter; next = d->next; dev = d->dev; - netdev_for_each_lower_dev(selected_dev, lower_dev, iter) { - if (dev == lower_dev) { - bring_down = false; - break; + if (selected_dev) { + netdev_for_each_lower_dev(selected_dev, lower, iter) { + if (dev == lower) { + bring_down = false; + break; + } } } if (bring_down) {