Skip to content

Commit 1aff640

Browse files
NicolasDichtelgregkh
NicolasDichtel
authored and
gregkh
committed
UPSTREAM: xfrm interface: fix memory leak on creation
The following commands produce a backtrace and return an error but the xfrm interface is created (in the wrong netns): $ ip netns add foo $ ip netns add bar $ ip -n foo netns set bar 0 $ ip -n foo link add xfrmi0 link-netnsid 0 type xfrm dev lo if_id 23 RTNETLINK answers: Invalid argument $ ip -n bar link ls xfrmi0 2: xfrmi0@lo: <NOARP,M-DOWN> mtu 1500 qdisc noop state DOWN mode DEFAULT group default qlen 1000 link/none 00:00:00:00:00:00 brd 00:00:00:00:00:00 Here is the backtrace: [ 79.879174] WARNING: CPU: 0 PID: 1178 at net/core/dev.c:8172 rollback_registered_many+0x86/0x3c1 [ 79.880260] Modules linked in: xfrm_interface nfsv3 nfs_acl auth_rpcgss nfsv4 nfs lockd grace sunrpc fscache button parport_pc parport serio_raw evdev pcspkr loop ext4 crc16 mbcache jbd2 crc32c_generic ide_cd_mod ide_gd_mod cdrom ata_$ eneric ata_piix libata scsi_mod 8139too piix psmouse i2c_piix4 ide_core 8139cp mii i2c_core floppy [ 79.883698] CPU: 0 PID: 1178 Comm: ip Not tainted 5.2.0-rc6+ torvalds#106 [ 79.884462] Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS 1.10.2-1 04/01/2014 [ 79.885447] RIP: 0010:rollback_registered_many+0x86/0x3c1 [ 79.886120] Code: 01 e8 d7 7d c6 ff 0f 0b 48 8b 45 00 4c 8b 20 48 8d 58 90 49 83 ec 70 48 8d 7b 70 48 39 ef 74 44 8a 83 d0 04 00 00 84 c0 75 1f <0f> 0b e8 61 cd ff ff 48 b8 00 01 00 00 00 00 ad de 48 89 43 70 66 [ 79.888667] RSP: 0018:ffffc900015ab740 EFLAGS: 00010246 [ 79.889339] RAX: ffff8882353e5700 RBX: ffff8882353e56a0 RCX: ffff8882353e5710 [ 79.890174] RDX: ffffc900015ab7e0 RSI: ffffc900015ab7e0 RDI: ffff8882353e5710 [ 79.891029] RBP: ffffc900015ab7e0 R08: ffffc900015ab7e0 R09: ffffc900015ab7e0 [ 79.891866] R10: ffffc900015ab7a0 R11: ffffffff82233fec R12: ffffc900015ab770 [ 79.892728] R13: ffffffff81eb7ec0 R14: ffff88822ed6cf00 R15: 00000000ffffffea [ 79.893557] FS: 00007ff350f31740(0000) GS:ffff888237a00000(0000) knlGS:0000000000000000 [ 79.894581] CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033 [ 79.895317] CR2: 00000000006c8580 CR3: 000000022c272000 CR4: 00000000000006f0 [ 79.896137] Call Trace: [ 79.896464] unregister_netdevice_many+0x12/0x6c [ 79.896998] __rtnl_newlink+0x6e2/0x73b [ 79.897446] ? __kmalloc_node_track_caller+0x15e/0x185 [ 79.898039] ? pskb_expand_head+0x5f/0x1fe [ 79.898556] ? stack_access_ok+0xd/0x2c [ 79.899009] ? deref_stack_reg+0x12/0x20 [ 79.899462] ? stack_access_ok+0xd/0x2c [ 79.899927] ? stack_access_ok+0xd/0x2c [ 79.900404] ? __module_text_address+0x9/0x4f [ 79.900910] ? is_bpf_text_address+0x5/0xc [ 79.901390] ? kernel_text_address+0x67/0x7b [ 79.901884] ? __kernel_text_address+0x1a/0x25 [ 79.902397] ? unwind_get_return_address+0x12/0x23 [ 79.903122] ? __cmpxchg_double_slab.isra.37+0x46/0x77 [ 79.903772] rtnl_newlink+0x43/0x56 [ 79.904217] rtnetlink_rcv_msg+0x200/0x24c In fact, each time a xfrm interface was created, a netdev was allocated by __rtnl_newlink()/rtnl_create_link() and then another one by xfrmi_newlink()/xfrmi_create(). Only the second one was registered, it's why the previous commands produce a backtrace: dev_change_net_namespace() was called on a netdev with reg_state set to NETREG_UNINITIALIZED (the first one). CC: Lorenzo Colitti <lorenzo@google.com> CC: Benedict Wong <benedictwong@google.com> CC: Steffen Klassert <steffen.klassert@secunet.com> CC: Shannon Nelson <shannon.nelson@oracle.com> CC: Antony Antony <antony@phenome.org> CC: Eyal Birger <eyal.birger@gmail.com> Fixes: f203b76 ("xfrm: Add virtual xfrm interfaces") Reported-by: Julien Floret <julien.floret@6wind.com> Signed-off-by: Nicolas Dichtel <nicolas.dichtel@6wind.com> Signed-off-by: Steffen Klassert <steffen.klassert@secunet.com> (cherry picked from commit 56c5ee1) Signed-off-by: Greg Kroah-Hartman <gregkh@google.com> Change-Id: I479074a3e99d5771854485be20563d818f3c5f6b
1 parent ea3c56a commit 1aff640

File tree

1 file changed

+28
-70
lines changed

1 file changed

+28
-70
lines changed

‎net/xfrm/xfrm_interface.c

+28-70
Original file line numberDiff line numberDiff line change
@@ -119,7 +119,7 @@ static void xfrmi_dev_free(struct net_device *dev)
119119
free_percpu(dev->tstats);
120120
}
121121

122-
static int xfrmi_create2(struct net_device *dev)
122+
static int xfrmi_create(struct net_device *dev)
123123
{
124124
struct xfrm_if *xi = netdev_priv(dev);
125125
struct net *net = dev_net(dev);
@@ -142,72 +142,19 @@ static int xfrmi_create2(struct net_device *dev)
142142
return err;
143143
}
144144

145-
static struct xfrm_if *xfrmi_create(struct net *net, struct xfrm_if_parms *p)
146-
{
147-
struct net_device *dev;
148-
struct xfrm_if *xi;
149-
char name[IFNAMSIZ];
150-
int err;
151-
152-
if (p->name[0]) {
153-
strlcpy(name, p->name, IFNAMSIZ);
154-
} else {
155-
err = -EINVAL;
156-
goto failed;
157-
}
158-
159-
dev = alloc_netdev(sizeof(*xi), name, NET_NAME_UNKNOWN, xfrmi_dev_setup);
160-
if (!dev) {
161-
err = -EAGAIN;
162-
goto failed;
163-
}
164-
165-
dev_net_set(dev, net);
166-
167-
xi = netdev_priv(dev);
168-
xi->p = *p;
169-
xi->net = net;
170-
xi->dev = dev;
171-
xi->phydev = dev_get_by_index(net, p->link);
172-
if (!xi->phydev) {
173-
err = -ENODEV;
174-
goto failed_free;
175-
}
176-
177-
err = xfrmi_create2(dev);
178-
if (err < 0)
179-
goto failed_dev_put;
180-
181-
return xi;
182-
183-
failed_dev_put:
184-
dev_put(xi->phydev);
185-
failed_free:
186-
free_netdev(dev);
187-
failed:
188-
return ERR_PTR(err);
189-
}
190-
191-
static struct xfrm_if *xfrmi_locate(struct net *net, struct xfrm_if_parms *p,
192-
int create)
145+
static struct xfrm_if *xfrmi_locate(struct net *net, struct xfrm_if_parms *p)
193146
{
194147
struct xfrm_if __rcu **xip;
195148
struct xfrm_if *xi;
196149
struct xfrmi_net *xfrmn = net_generic(net, xfrmi_net_id);
197150

198151
for (xip = &xfrmn->xfrmi[0];
199152
(xi = rtnl_dereference(*xip)) != NULL;
200-
xip = &xi->next) {
201-
if (xi->p.if_id == p->if_id) {
202-
if (create)
203-
return ERR_PTR(-EEXIST);
204-
153+
xip = &xi->next)
154+
if (xi->p.if_id == p->if_id)
205155
return xi;
206-
}
207-
}
208-
if (!create)
209-
return ERR_PTR(-ENODEV);
210-
return xfrmi_create(net, p);
156+
157+
return NULL;
211158
}
212159

213160
static void xfrmi_dev_uninit(struct net_device *dev)
@@ -675,21 +622,33 @@ static int xfrmi_newlink(struct net *src_net, struct net_device *dev,
675622
struct netlink_ext_ack *extack)
676623
{
677624
struct net *net = dev_net(dev);
678-
struct xfrm_if_parms *p;
625+
struct xfrm_if_parms p;
679626
struct xfrm_if *xi;
627+
int err;
680628

681-
xi = netdev_priv(dev);
682-
p = &xi->p;
683-
684-
xfrmi_netlink_parms(data, p);
629+
xfrmi_netlink_parms(data, &p);
685630

686631
if (!tb[IFLA_IFNAME])
687632
return -EINVAL;
688633

689-
nla_strlcpy(p->name, tb[IFLA_IFNAME], IFNAMSIZ);
634+
nla_strlcpy(p.name, tb[IFLA_IFNAME], IFNAMSIZ);
690635

691-
xi = xfrmi_locate(net, p, 1);
692-
return PTR_ERR_OR_ZERO(xi);
636+
xi = xfrmi_locate(net, &p);
637+
if (xi)
638+
return -EEXIST;
639+
640+
xi = netdev_priv(dev);
641+
xi->p = p;
642+
xi->net = net;
643+
xi->dev = dev;
644+
xi->phydev = dev_get_by_index(net, p.link);
645+
if (!xi->phydev)
646+
return -ENODEV;
647+
648+
err = xfrmi_create(dev);
649+
if (err < 0)
650+
dev_put(xi->phydev);
651+
return err;
693652
}
694653

695654
static void xfrmi_dellink(struct net_device *dev, struct list_head *head)
@@ -706,9 +665,8 @@ static int xfrmi_changelink(struct net_device *dev, struct nlattr *tb[],
706665

707666
xfrmi_netlink_parms(data, &xi->p);
708667

709-
xi = xfrmi_locate(net, &xi->p, 0);
710-
711-
if (IS_ERR_OR_NULL(xi)) {
668+
xi = xfrmi_locate(net, &xi->p);
669+
if (!xi) {
712670
xi = netdev_priv(dev);
713671
} else {
714672
if (xi->dev != dev)

0 commit comments

Comments
 (0)