From patchwork Wed Aug 11 08:31:02 2010 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Jarek Poplawski X-Patchwork-Id: 61462 X-Patchwork-Delegate: davem@davemloft.net Return-Path: X-Original-To: patchwork-incoming@ozlabs.org Delivered-To: patchwork-incoming@ozlabs.org Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by ozlabs.org (Postfix) with ESMTP id ACB96B6F10 for ; Wed, 11 Aug 2010 18:31:18 +1000 (EST) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1756017Ab0HKIbM (ORCPT ); Wed, 11 Aug 2010 04:31:12 -0400 Received: from mail-bw0-f46.google.com ([209.85.214.46]:49993 "EHLO mail-bw0-f46.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1753245Ab0HKIbK (ORCPT ); Wed, 11 Aug 2010 04:31:10 -0400 Received: by bwz3 with SMTP id 3so2464316bwz.19 for ; Wed, 11 Aug 2010 01:31:09 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=gamma; h=domainkey-signature:received:received:date:from:to:cc:subject :message-id:mime-version:content-type:content-disposition:x-mutt-fcc :user-agent; bh=sPrDAVtzgx1owAf/63NHCyifMcgBfHpz4npTeI9LF38=; b=PpTdXOiJ+8IREttHeLWzkuWgj8W0PWd98Rd/vnSXxrdsz6aM5baF451NahfX7Syion t8anBwLSUSBU4RqRH11rQRBtK1rgDga/bfs3r602cGE6F7hHncqQ3wc4/ql6Xsqsr3Ba fRLcvYjC38a+RkiY+kHYjGka+J+nijYF1fxdg= DomainKey-Signature: a=rsa-sha1; c=nofws; d=gmail.com; s=gamma; h=date:from:to:cc:subject:message-id:mime-version:content-type :content-disposition:x-mutt-fcc:user-agent; b=k+tXpAp4wQJ+6D1LMYQFGpbw6quc1DYIKPais9ZcEwIwybt957Tx1Dme93MMOZVFS3 J2fjBI3DFgijbwvQFiBwtrILkCFBfhdlc39aegpQsSzPBOy0PigfEUeNBUfqhX9N4co+ LJ8MZffZ8+VF5N4w5nPAdUo48IyVWW2ZRBjZg= Received: by 10.204.69.200 with SMTP id a8mr12887118bkj.36.1281515469410; Wed, 11 Aug 2010 01:31:09 -0700 (PDT) Received: from ff.dom.local (bv170.internetdsl.tpnet.pl [80.53.205.170]) by mx.google.com with ESMTPS id y19sm2327696bkw.6.2010.08.11.01.31.06 (version=SSLv3 cipher=RC4-MD5); Wed, 11 Aug 2010 01:31:08 -0700 (PDT) Date: Wed, 11 Aug 2010 08:31:02 +0000 From: Jarek Poplawski To: David Miller Cc: netdev@vger.kernel.org, Stephen Hemminger , Patrick McHardy , Franchoze Eric Subject: [PATCH 1/2] pkt_sched: Fix sch_sfq vs tc_modify_qdisc oops Message-ID: <20100811083101.GA10126@ff.dom.local> MIME-Version: 1.0 Content-Disposition: inline X-Mutt-Fcc: =outbox User-Agent: Mutt/1.5.18 (2008-05-17) Sender: netdev-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: netdev@vger.kernel.org Hi, After more investigation I've found sch_sfq is still vulnerable because off its missing class handlers. Here is an oops as proof of the concept, and a patch below. This patch should be considered for stable (without following 2/2 patch). Jarek P. This oops can happen while doing tc qdisc replace on sfq leaf qdiscs under some traffic. BUG: unable to handle kernel NULL pointer dereference at (null) IP: [<(null)>] (null) *pde = 00000000 Oops: 0000 [#1] SMP last sysfs file: /sys/module/sch_htb/refcnt Modules linked in: sch_htb sch_sfq sch_prio snd_seq_dummy snd_seq_oss snd_seq_midi_event snd_seq snd_seq_device snd_pcm_oss snd_mixer_oss ipv6 ext3 jbd mbcache lp fuse ppdev snd_intel8x0 snd_ac97_codec ac97_bus rtc_cmos parport_pc uhci_hcd intel_agp snd_pcm e100 psmouse rtc_core mii i2c_i801 parport agpgart processor snd_timer thermal dcdbas serio_raw ehci_hcd rtc_lib shpchp thermal_sys button i2c_core evdev hwmon snd soundcore snd_page_alloc reiserfs [last unloaded: sch_htb] Pid: 2925, comm: tc Not tainted (2.6.32-smp #1) OptiPlex 170L EIP: 0060:[<00000000>] EFLAGS: 00010286 CPU: 0 EIP is at 0x0 EAX: cdb52000 EBX: c44dbc98 ECX: d041fe20 EDX: 00000028 ESI: c44dbc98 EDI: cdb52080 EBP: cdb52000 ESP: c44dbc14 DS: 007b ES: 007b FS: 00d8 GS: 0033 SS: 0068 Process tc (pid: 2925, ti=c44da000 task=ce819500 task.ti=c44da000) Stack: c12dfbe6 00000000 c44dbc98 00000028 cdb52080 cdb52000 c1353949 c44dbc98 <0> 00000028 d041f37a d041fdbd cdb52000 cf151410 cd8b7000 cf065800 c12e1dc1 <0> 00000000 00000000 cd9fd700 cf045000 cf151424 cf151400 cd9fd700 00010002 Call Trace: [] ? check_loop_fn+0x16/0xa0 [] ? printk+0x17/0x1e [] ? sfq_walk+0x8a/0xb0 [sch_sfq] [] ? tc_modify_qdisc+0x451/0x4f0 [] ? check_loop_fn+0x0/0xa0 [] ? tc_modify_qdisc+0x0/0x4f0 [] ? rtnetlink_rcv_msg+0x16d/0x210 [] ? sock_rmalloc+0x35/0x90 [] ? rtnetlink_rcv_msg+0x0/0x210 [] ? netlink_rcv_skb+0x66/0x90 [] ? rtnetlink_rcv+0x19/0x20 [] ? netlink_unicast+0x26e/0x280 [] ? netlink_sendmsg+0x1d5/0x2f0 [] ? sock_sendmsg+0x111/0x130 [] ? autoremove_wake_function+0x0/0x50 [] ? autoremove_wake_function+0x0/0x50 [] ? sys_sendmsg+0x174/0x280 [] ? sys_recvmsg+0x1a6/0x240 [] ? find_get_page+0x24/0xb0 [] ? filemap_fault+0x74/0x3a0 [] ? __do_fault+0x361/0x450 [] ? lock_sock_nested+0x90/0xb0 [] ? handle_mm_fault+0x144/0x800 [] ? sys_socketcall+0xbd/0x290 [] ? do_page_fault+0x13a/0x2a0 [] ? do_page_fault+0x0/0x2a0 [] ? syscall_call+0x7/0xb Code: Bad EIP value. EIP: [<00000000>] 0x0 SS:ESP 0068:c44dbc14 CR2: 0000000000000000 ------------> sch_sfq as a classful qdisc needs the .leaf handler. Otherwise, there is an oops possible in tc_modify_qdisc()/check_loop(). Fixes commit 7d2681a6ff4f9ab5e48d02550b4c6338f1638998 Signed-off-by: Jarek Poplawski --- -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html diff --git a/net/sched/sch_sfq.c b/net/sched/sch_sfq.c index b8bcb20..201cbac 100644 --- a/net/sched/sch_sfq.c +++ b/net/sched/sch_sfq.c @@ -508,6 +508,11 @@ nla_put_failure: return -1; } +static struct Qdisc *sfq_leaf(struct Qdisc *sch, unsigned long arg) +{ + return NULL; +} + static unsigned long sfq_get(struct Qdisc *sch, u32 classid) { return 0; @@ -575,6 +580,7 @@ static void sfq_walk(struct Qdisc *sch, struct qdisc_walker *arg) } static const struct Qdisc_class_ops sfq_class_ops = { + .leaf = sfq_leaf, .get = sfq_get, .put = sfq_put, .tcf_chain = sfq_find_tcf,