| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-Id: <20180710212227.17839-1-jacob.e.keller@intel.com>
Date: Tue, 10 Jul 2018 14:22:27 -0700
From: Jacob Keller <jacob.e.keller@...el.com>
To: netdev@...r.kernel.org
Cc: Cong Wang <xiyou.wangcong@...il.com>,
Eric Dumazet <edumazet@...gle.com>,
Jacob Keller <jacob.e.keller@...el.com>
Subject: [net v2] sch_fq_codel: zero q->flows_cnt when fq_codel_init fails
When fq_codel_init fails, qdisc_create_dflt will cleanup by using
qdisc_destroy. This function calls the ->reset() op prior to calling the
->destroy() op.
Unfortunately, during the failure flow for sch_fq_codel, the ->flows
parameter is not initialized, so the fq_codel_reset function will null
pointer dereference.
kernel: BUG: unable to handle kernel NULL pointer dereference at 0000000000000008
kernel: IP: fq_codel_reset+0x58/0xd0 [sch_fq_codel]
kernel: PGD 0 P4D 0
kernel: Oops: 0000 [#1] SMP PTI
kernel: Modules linked in: i40iw i40e(OE) xt_CHECKSUM iptable_mangle ipt_MASQUERADE nf_nat_masquerade_ipv4 iptable_nat nf_nat_ipv4 nf_nat nf_conntrack_ipv4 nf_defrag_ipv4 xt_conntrack nf_conntrack tun bridge stp llc devlink ebtable_filter ebtables ip6table_filter ip6_tables rpcrdma ib_isert iscsi_target_mod sunrpc ib_iser libiscsi scsi_transport_iscsi ib_srpt target_core_mod ib_srp scsi_transport_srp ib_ipoib rdma_ucm ib_ucm ib_uverbs ib_umad rdma_cm ib_cm iw_cm intel_rapl sb_edac x86_pkg_temp_thermal intel_powerclamp coretemp kvm irqbypass crct10dif_pclmul crc32_pclmul ghash_clmulni_intel intel_cstate iTCO_wdt iTCO_vendor_support intel_uncore ib_core intel_rapl_perf mei_me mei joydev i2c_i801 lpc_ich ioatdma shpchp wmi sch_fq_codel xfs libcrc32c mgag200 ixgbe drm_kms_helper isci ttm firewire_ohci
kernel: mdio drm igb libsas crc32c_intel firewire_core ptp pps_core scsi_transport_sas crc_itu_t dca i2c_algo_bit ipmi_si ipmi_devintf ipmi_msghandler [last unloaded: i40e]
kernel: CPU: 10 PID: 4219 Comm: ip Tainted: G OE 4.16.13custom-fq-codel-test+ #3
kernel: Hardware name: Intel Corporation S2600CO/S2600CO, BIOS SE5C600.86B.02.05.0004.051120151007 05/11/2015
kernel: RIP: 0010:fq_codel_reset+0x58/0xd0 [sch_fq_codel]
kernel: RSP: 0018:ffffbfbf4c1fb620 EFLAGS: 00010246
kernel: RAX: 0000000000000400 RBX: 0000000000000000 RCX: 00000000000005b9
kernel: RDX: 0000000000000000 RSI: ffff9d03264a60c0 RDI: ffff9cfd17b31c00
kernel: RBP: 0000000000000001 R08: 00000000000260c0 R09: ffffffffb679c3e9
kernel: R10: fffff1dab06a0e80 R11: ffff9cfd163af800 R12: ffff9cfd17b31c00
kernel: R13: 0000000000000001 R14: ffff9cfd153de600 R15: 0000000000000001
kernel: FS: 00007fdec2f92800(0000) GS:ffff9d0326480000(0000) knlGS:0000000000000000
kernel: CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
kernel: CR2: 0000000000000008 CR3: 0000000c1956a006 CR4: 00000000000606e0
kernel: Call Trace:
kernel: qdisc_destroy+0x56/0x140
kernel: qdisc_create_dflt+0x8b/0xb0
kernel: mq_init+0xc1/0xf0
kernel: qdisc_create_dflt+0x5a/0xb0
kernel: dev_activate+0x205/0x230
kernel: __dev_open+0xf5/0x160
kernel: __dev_change_flags+0x1a3/0x210
kernel: dev_change_flags+0x21/0x60
kernel: do_setlink+0x660/0xdf0
kernel: ? down_trylock+0x25/0x30
kernel: ? xfs_buf_trylock+0x1a/0xd0 [xfs]
kernel: ? rtnl_newlink+0x816/0x990
kernel: ? _xfs_buf_find+0x327/0x580 [xfs]
kernel: ? _cond_resched+0x15/0x30
kernel: ? kmem_cache_alloc+0x20/0x1b0
kernel: ? rtnetlink_rcv_msg+0x200/0x2f0
kernel: ? rtnl_calcit.isra.30+0x100/0x100
kernel: ? netlink_rcv_skb+0x4c/0x120
kernel: ? netlink_unicast+0x19e/0x260
kernel: ? netlink_sendmsg+0x1ff/0x3c0
kernel: ? sock_sendmsg+0x36/0x40
kernel: ? ___sys_sendmsg+0x295/0x2f0
kernel: ? ebitmap_cmp+0x6d/0x90
kernel: ? dev_get_by_name_rcu+0x73/0x90
kernel: ? skb_dequeue+0x52/0x60
kernel: ? __inode_wait_for_writeback+0x7f/0xf0
kernel: ? bit_waitqueue+0x30/0x30
kernel: ? fsnotify_grab_connector+0x3c/0x60
kernel: ? __sys_sendmsg+0x51/0x90
kernel: ? do_syscall_64+0x74/0x180
kernel: ? entry_SYSCALL_64_after_hwframe+0x3d/0xa2
kernel: Code: 00 00 48 89 87 00 02 00 00 8b 87 a0 01 00 00 85 c0 0f 84 84 00 00 00 31 ed 48 63 dd 83 c5 01 48 c1 e3 06 49 03 9c 24 90 01 00 00 <48> 8b 73 08 48 8b 3b e8 6c 9a 4f f6 48 8d 43 10 48 c7 03 00 00
kernel: RIP: fq_codel_reset+0x58/0xd0 [sch_fq_codel] RSP: ffffbfbf4c1fb620
kernel: CR2: 0000000000000008
kernel: ---[ end trace e81a62bede66274e ]---
This is caused because flows_cnt is non-zero, but flows hasn't been
initialized. fq_codel_init has left the private data in a partially
initialized state.
To fix this, reset flows_cnt to 0 when we fail to initialize.
Additionally, to make the state more consistent, also cleanup the flows
pointer when the allocation of backlogs fails.
This fixes the NULL pointer dereference, since both the for-loop and
memset in fq_codel_reset will be no-ops when flow_cnt is zero.
Signed-off-by: Jacob Keller <jacob.e.keller@...el.com>
---
net/sched/sch_fq_codel.c | 25 ++++++++++++++++++-------
1 file changed, 18 insertions(+), 7 deletions(-)
diff --git a/net/sched/sch_fq_codel.c b/net/sched/sch_fq_codel.c
index cd2e0e342fb6..6c0a9d5dbf94 100644
--- a/net/sched/sch_fq_codel.c
+++ b/net/sched/sch_fq_codel.c
@@ -479,24 +479,28 @@ static int fq_codel_init(struct Qdisc *sch, struct nlattr *opt,
q->cparams.mtu = psched_mtu(qdisc_dev(sch));
if (opt) {
- int err = fq_codel_change(sch, opt, extack);
+ err = fq_codel_change(sch, opt, extack);
if (err)
- return err;
+ goto init_failure;
}
err = tcf_block_get(&q->block, &q->filter_list, sch, extack);
if (err)
- return err;
+ goto init_failure;
if (!q->flows) {
q->flows = kvcalloc(q->flows_cnt,
sizeof(struct fq_codel_flow),
GFP_KERNEL);
- if (!q->flows)
- return -ENOMEM;
+ if (!q->flows) {
+ err = -ENOMEM;
+ goto init_failure;
+ }
q->backlogs = kvcalloc(q->flows_cnt, sizeof(u32), GFP_KERNEL);
- if (!q->backlogs)
- return -ENOMEM;
+ if (!q->backlogs) {
+ err = -ENOMEM;
+ goto alloc_failure;
+ }
for (i = 0; i < q->flows_cnt; i++) {
struct fq_codel_flow *flow = q->flows + i;
@@ -509,6 +513,13 @@ static int fq_codel_init(struct Qdisc *sch, struct nlattr *opt,
else
sch->flags &= ~TCQ_F_CAN_BYPASS;
return 0;
+
+alloc_failure:
+ kvfree(q->flows);
+ q->flows = NULL;
+init_failure:
+ q->flows_cnt = 0;
+ return err;
}
static int fq_codel_dump(struct Qdisc *sch, struct sk_buff *skb)
--
2.18.0.409.g86b1da64138f
Powered by blists - more mailing lists