Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1753408Ab2HMR1q (ORCPT ); Mon, 13 Aug 2012 13:27:46 -0400 Received: from mail-qa0-f53.google.com ([209.85.216.53]:34205 "EHLO mail-qa0-f53.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752434Ab2HMR1o (ORCPT ); Mon, 13 Aug 2012 13:27:44 -0400 MIME-Version: 1.0 Date: Mon, 13 Aug 2012 10:27:43 -0700 Message-ID: Subject: [PATCH 1/1]: leak of ctl_table_header structs in proc_sys_lookup From: Francesco Ruggeri To: "Eric W. Biederman" Cc: linux-kernel@vger.kernel.org Content-Type: text/plain; charset=ISO-8859-1 Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 4156 Lines: 99 Commit 076c3eed2c31773200b082568957fd8852ae93d7 in 3.4 changed some of the logic in proc_sys_lookup() by introducing lookup_entry(). On success lookup_entry() will return having invoked use_table() on its first parameter. In case of a failure later on in proc_sys_lookup() this reference has to be dropped. This patch fixes one such case when sysctl_follow_link() fails. When this happened the reference would never drop to 0, and would cause dev_change_net_namespace() to hang while holding the rtnl lock with this backtrace: [] schedule+0x5f/0x61 [] schedule_timeout+0x22/0xbb [] ? vprintk+0x376/0x3b1 [] wait_for_common+0x99/0x110 [] ? try_to_wake_up+0x1ae/0x1ae [] wait_for_completion+0x18/0x1a [] drop_sysctl_table+0x97/0x114 [] put_links+0x10a/0x173 [] drop_sysctl_table+0x29/0x114 [] drop_sysctl_table+0x109/0x114 [] ? local_bh_enable_ip+0x9/0xb [] unregister_sysctl_table+0x65/0x75 [] neigh_sysctl_unregister+0x22/0x3a [] inetdev_event+0x2c6/0x411 [] notifier_call_chain+0x32/0x5e [] raw_notifier_call_chain+0xf/0x11 [] call_netdevice_notifiers+0x45/0x4a [] dev_change_net_namespace+0xc6/0x18e [] do_setlink+0x77/0x766 [] ? nla_parse+0x4f/0xc3 [] rtnl_newlink+0x252/0x49f [] ? rtnl_newlink+0xc4/0x49f [] ? pmd_offset+0x14/0x3b [] rtnetlink_rcv_msg+0x248/0x25e [] ? read_tsc+0x9/0x19 [] ? timekeeping_get_ns+0x15/0x34 [] ? __rtnl_unlock+0x33/0x33 [] netlink_rcv_skb+0x40/0x8c [] rtnetlink_rcv+0x21/0x28 [] netlink_unicast+0xec/0x155 [] netlink_sendmsg+0x202/0x220 [] __sock_sendmsg+0x6e/0x7a [] sock_sendmsg+0xa3/0xbc [] ? sock_recvmsg+0xa6/0xbf [] ? get_page+0x29/0x2e [] ? __lru_cache_add+0x2f/0x56 [] ? lru_cache_add_lru+0x4e/0x55 [] ? page_add_new_anon_rmap+0x5b/0x6d [] ? move_addr_to_kernel+0x44/0x49 [] ? verify_compat_iovec+0x6d/0xb9 [] __sys_sendmsg+0x21c/0x2c3 [] ? inc_zone_page_state+0x28/0x2a [] ? pmd_offset+0x14/0x3b [] ? handle_mm_fault+0xeb/0x100 [] ? do_page_fault+0x2da/0x34c [] sys_sendmsg+0x3d/0x5b [] compat_sys_sendmsg+0xf/0x11 [] compat_sys_socketcall+0x152/0x19b [] sysenter_dispatch+0x7/0x21 This patch slightly changes the logic in proc_sys_lookup in that d_set_d_op(dentry, &proc_sys_dentry_operations); d_add(dentry, inode); will now execute before the ctl_table_header is freed. This should not be a problem, and it makes the code cleaner than just adding an extra call to sysctl_head_finish(h); Tested in Linux 3.4.4. Signed-off-by: Francesco Ruggeri Index: linux-3.4.x86_64/fs/proc/proc_sysctl.c =================================================================== --- linux-3.4.x86_64.orig/fs/proc/proc_sysctl.c +++ linux-3.4.x86_64/fs/proc/proc_sysctl.c @@ -462,9 +462,6 @@ static struct dentry *proc_sys_lookup(st err = ERR_PTR(-ENOMEM); inode = proc_sys_make_inode(dir->i_sb, h ? h : head, p); - if (h) - sysctl_head_finish(h); - if (!inode) goto out; @@ -473,6 +470,8 @@ static struct dentry *proc_sys_lookup(st d_add(dentry, inode); out: + if (h) + sysctl_head_finish(h); sysctl_head_finish(head); return err; } -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/