From 6049f2530cf2cb48a6fe8735309cc0b97aa7f700 Mon Sep 17 00:00:00 2001 From: Fernando Luis Vazquez Cao Date: Tue, 4 Feb 2014 19:35:02 +0900 Subject: rtnetlink: fix oops in rtnl_link_get_slave_info_data_size We should check whether rtnetlink link operations are defined before calling get_slave_size(). Without this, the following oops can occur when adding a tap device to OVS. [ 87.839553] BUG: unable to handle kernel NULL pointer dereference at 00000000000000a8 [ 87.839595] IP: [] if_nlmsg_size+0xf0/0x220 [...] [ 87.840651] Call Trace: [ 87.840664] [] ? rtmsg_ifinfo+0x2b/0x100 [ 87.840688] [] ? __netdev_adjacent_dev_insert+0x150/0x1a0 [ 87.840718] [] ? rtnetlink_event+0x30/0x40 [ 87.840742] [] ? notifier_call_chain+0x44/0x70 [ 87.840768] [] ? __netdev_upper_dev_link+0x3c6/0x3f0 [ 87.840798] [] ? netdev_create+0xcc/0x160 [openvswitch] [ 87.840828] [] ? ovs_vport_add+0x4a/0xd0 [openvswitch] [ 87.840857] [] ? new_vport+0x9/0x50 [openvswitch] [ 87.840884] [] ? ovs_vport_cmd_new+0x11e/0x210 [openvswitch] [ 87.840915] [] ? genl_family_rcv_msg+0x19a/0x360 [ 87.840941] [] ? genl_family_rcv_msg+0x360/0x360 [ 87.840967] [] ? genl_rcv_msg+0x79/0xc0 [ 87.840991] [] ? __kmalloc_reserve.isra.25+0x29/0x80 [ 87.841018] [] ? netlink_rcv_skb+0xa9/0xc0 [ 87.841042] [] ? genl_rcv+0x1f/0x30 [ 87.841064] [] ? netlink_unicast+0xe8/0x1e0 [ 87.841088] [] ? netlink_sendmsg+0x31a/0x750 [ 87.841113] [] ? sock_sendmsg+0x86/0xc0 [ 87.841136] [] ? __netdev_update_features+0x4d/0x200 [ 87.841163] [] ? ethtool_get_value+0x2e/0x50 [ 87.841188] [] ? ___sys_sendmsg+0x359/0x370 [ 87.841212] [] ? dev_ioctl+0x1a6/0x5c0 [ 87.841236] [] ? autoremove_wake_function+0x30/0x30 [ 87.841264] [] ? sock_do_ioctl+0x3d/0x50 [ 87.841288] [] ? sock_ioctl+0x1e8/0x2c0 [ 87.841312] [] ? do_vfs_ioctl+0x2cf/0x4b0 [ 87.841335] [] ? __sys_sendmsg+0x39/0x70 [ 87.841362] [] ? system_call_fastpath+0x16/0x1b [ 87.841386] Code: c0 74 10 48 89 ef ff d0 83 c0 07 83 e0 fc 48 98 49 01 c7 48 89 ef e8 d0 d6 fe ff 48 85 c0 0f 84 df 00 00 00 48 8b 90 08 07 00 00 <48> 8b 8a a8 00 00 00 31 d2 48 85 c9 74 0c 48 89 ee 48 89 c7 ff [ 87.841529] RIP [] if_nlmsg_size+0xf0/0x220 [ 87.841555] RSP [ 87.841569] CR2: 00000000000000a8 [ 87.851442] ---[ end trace e42ab217691b4fc2 ]--- Signed-off-by: Fernando Luis Vazquez Cao Acked-by: Jiri Pirko Signed-off-by: David S. Miller --- net/core/rtnetlink.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'net/core/rtnetlink.c') diff --git a/net/core/rtnetlink.c b/net/core/rtnetlink.c index 393b1bc9a618..048dc8d183aa 100644 --- a/net/core/rtnetlink.c +++ b/net/core/rtnetlink.c @@ -374,7 +374,7 @@ static size_t rtnl_link_get_slave_info_data_size(const struct net_device *dev) if (!master_dev) return 0; ops = master_dev->rtnl_link_ops; - if (!ops->get_slave_size) + if (!ops || !ops->get_slave_size) return 0; /* IFLA_INFO_SLAVE_DATA + nested data */ return nla_total_size(sizeof(struct nlattr)) + -- cgit From 0e0eee2465df77bcec2e8ff75432b8e57897b143 Mon Sep 17 00:00:00 2001 From: Cong Wang Date: Tue, 11 Feb 2014 15:51:30 -0800 Subject: net: correct error path in rtnl_newlink() I saw the following BUG when ->newlink() fails in rtnl_newlink(): [ 40.240058] kernel BUG at net/core/dev.c:6438! this is due to free_netdev() is not supposed to be called before netdev is completely unregistered, therefore it is not correct to call free_netdev() here, at least for ops->newlink!=NULL case, many drivers call it in ->destructor so that rtnl_unlock() will take care of it, we probably don't need to do anything here. Cc: David S. Miller Cc: Eric Dumazet Signed-off-by: Cong Wang Signed-off-by: Cong Wang Signed-off-by: David S. Miller --- net/core/rtnetlink.c | 19 ++++++++++++------- 1 file changed, 12 insertions(+), 7 deletions(-) (limited to 'net/core/rtnetlink.c') diff --git a/net/core/rtnetlink.c b/net/core/rtnetlink.c index 048dc8d183aa..1a0dac2ef9ad 100644 --- a/net/core/rtnetlink.c +++ b/net/core/rtnetlink.c @@ -1963,16 +1963,21 @@ replay: dev->ifindex = ifm->ifi_index; - if (ops->newlink) + if (ops->newlink) { err = ops->newlink(net, dev, tb, data); - else + /* Drivers should call free_netdev() in ->destructor + * and unregister it on failure so that device could be + * finally freed in rtnl_unlock. + */ + if (err < 0) + goto out; + } else { err = register_netdevice(dev); - - if (err < 0) { - free_netdev(dev); - goto out; + if (err < 0) { + free_netdev(dev); + goto out; + } } - err = rtnl_configure_link(dev, ifm); if (err < 0) unregister_netdevice(dev); -- cgit