stable.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
To: stable@vger.kernel.org
Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
	patches@lists.linux.dev, Frederick Lawler <fred@cloudflare.com>,
	Jakub Sitnicki <jakub@cloudflare.com>,
	Jakub Kicinski <kuba@kernel.org>
Subject: [PATCH 6.1 10/10] net: sched: disallow noqueue for qdisc classes
Date: Thu, 12 Jan 2023 14:56:31 +0100	[thread overview]
Message-ID: <20230112135327.411324794@linuxfoundation.org> (raw)
In-Reply-To: <20230112135326.981869724@linuxfoundation.org>

From: Frederick Lawler <fred@cloudflare.com>

commit 96398560f26aa07e8f2969d73c8197e6a6d10407 upstream.

While experimenting with applying noqueue to a classful queue discipline,
we discovered a NULL pointer dereference in the __dev_queue_xmit()
path that generates a kernel OOPS:

    # dev=enp0s5
    # tc qdisc replace dev $dev root handle 1: htb default 1
    # tc class add dev $dev parent 1: classid 1:1 htb rate 10mbit
    # tc qdisc add dev $dev parent 1:1 handle 10: noqueue
    # ping -I $dev -w 1 -c 1 1.1.1.1

[    2.172856] BUG: kernel NULL pointer dereference, address: 0000000000000000
[    2.173217] #PF: supervisor instruction fetch in kernel mode
...
[    2.178451] Call Trace:
[    2.178577]  <TASK>
[    2.178686]  htb_enqueue+0x1c8/0x370
[    2.178880]  dev_qdisc_enqueue+0x15/0x90
[    2.179093]  __dev_queue_xmit+0x798/0xd00
[    2.179305]  ? _raw_write_lock_bh+0xe/0x30
[    2.179522]  ? __local_bh_enable_ip+0x32/0x70
[    2.179759]  ? ___neigh_create+0x610/0x840
[    2.179968]  ? eth_header+0x21/0xc0
[    2.180144]  ip_finish_output2+0x15e/0x4f0
[    2.180348]  ? dst_output+0x30/0x30
[    2.180525]  ip_push_pending_frames+0x9d/0xb0
[    2.180739]  raw_sendmsg+0x601/0xcb0
[    2.180916]  ? _raw_spin_trylock+0xe/0x50
[    2.181112]  ? _raw_spin_unlock_irqrestore+0x16/0x30
[    2.181354]  ? get_page_from_freelist+0xcd6/0xdf0
[    2.181594]  ? sock_sendmsg+0x56/0x60
[    2.181781]  sock_sendmsg+0x56/0x60
[    2.181958]  __sys_sendto+0xf7/0x160
[    2.182139]  ? handle_mm_fault+0x6e/0x1d0
[    2.182366]  ? do_user_addr_fault+0x1e1/0x660
[    2.182627]  __x64_sys_sendto+0x1b/0x30
[    2.182881]  do_syscall_64+0x38/0x90
[    2.183085]  entry_SYSCALL_64_after_hwframe+0x63/0xcd
...
[    2.187402]  </TASK>

Previously in commit d66d6c3152e8 ("net: sched: register noqueue
qdisc"), NULL was set for the noqueue discipline on noqueue init
so that __dev_queue_xmit() falls through for the noqueue case. This
also sets a bypass of the enqueue NULL check in the
register_qdisc() function for the struct noqueue_disc_ops.

Classful queue disciplines make it past the NULL check in
__dev_queue_xmit() because the discipline is set to htb (in this case),
and then in the call to __dev_xmit_skb(), it calls into htb_enqueue()
which grabs a leaf node for a class and then calls qdisc_enqueue() by
passing in a queue discipline which assumes ->enqueue() is not set to NULL.

Fix this by not allowing classes to be assigned to the noqueue
discipline. Linux TC Notes states that classes cannot be set to
the noqueue discipline. [1] Let's enforce that here.

Links:
1. https://linux-tc-notes.sourceforge.net/tc/doc/sch_noqueue.txt

Fixes: d66d6c3152e8 ("net: sched: register noqueue qdisc")
Cc: stable@vger.kernel.org
Signed-off-by: Frederick Lawler <fred@cloudflare.com>
Reviewed-by: Jakub Sitnicki <jakub@cloudflare.com>
Link: https://lore.kernel.org/r/20230109163906.706000-1-fred@cloudflare.com
Signed-off-by: Jakub Kicinski <kuba@kernel.org>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
---
 net/sched/sch_api.c |    5 +++++
 1 file changed, 5 insertions(+)

--- a/net/sched/sch_api.c
+++ b/net/sched/sch_api.c
@@ -1132,6 +1132,11 @@ skip:
 			return -ENOENT;
 		}
 
+		if (new && new->ops == &noqueue_qdisc_ops) {
+			NL_SET_ERR_MSG(extack, "Cannot assign noqueue to a class");
+			return -EINVAL;
+		}
+
 		err = cops->graft(parent, cl, new, &old, extack);
 		if (err)
 			return err;



  parent reply	other threads:[~2023-01-12 13:57 UTC|newest]

Thread overview: 23+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-01-12 13:56 [PATCH 6.1 00/10] 6.1.6-rc1 review Greg Kroah-Hartman
2023-01-12 13:56 ` [PATCH 6.1 01/10] parisc: Align parisc MADV_XXX constants with all other architectures Greg Kroah-Hartman
2023-01-12 13:56 ` [PATCH 6.1 02/10] x86/fpu: Take task_struct* in copy_sigframe_from_user_to_xstate() Greg Kroah-Hartman
2023-01-12 13:56 ` [PATCH 6.1 03/10] x86/fpu: Add a pkru argument to copy_uabi_from_kernel_to_xstate() Greg Kroah-Hartman
2023-01-12 13:56 ` [PATCH 6.1 04/10] x86/fpu: Add a pkru argument to copy_uabi_to_xstate() Greg Kroah-Hartman
2023-01-12 13:56 ` [PATCH 6.1 05/10] x86/fpu: Allow PKRU to be (once again) written by ptrace Greg Kroah-Hartman
2023-01-12 13:56 ` [PATCH 6.1 06/10] x86/fpu: Emulate XRSTORs behavior if the xfeatures PKRU bit is not set Greg Kroah-Hartman
2023-01-12 13:56 ` [PATCH 6.1 07/10] selftests/vm/pkeys: Add a regression test for setting PKRU through ptrace Greg Kroah-Hartman
2023-01-12 13:56 ` [PATCH 6.1 08/10] Revert "SUNRPC: Use RMW bitops in single-threaded hot paths" Greg Kroah-Hartman
2023-01-12 13:56 ` [PATCH 6.1 09/10] gcc: disable -Warray-bounds for gcc-11 too Greg Kroah-Hartman
2023-01-16  5:44   ` Jiri Slaby
2023-01-12 13:56 ` Greg Kroah-Hartman [this message]
2023-01-12 18:45 ` [PATCH 6.1 00/10] 6.1.6-rc1 review Conor Dooley
2023-01-13  0:27 ` Shuah Khan
2023-01-13  1:37 ` Kelsey Steele
2023-01-13  3:02 ` ogasawara takeshi
2023-01-13  5:51 ` Guenter Roeck
2023-01-13  8:58 ` Bagas Sanjaya
2023-01-13  9:40 ` Ron Economos
2023-01-13 12:30 ` Sudip Mukherjee
2023-01-13 17:32 ` Naresh Kamboju
2023-01-13 18:06 ` Allen Pais
2023-01-14  5:00 ` Rudi Heitbaum

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20230112135327.411324794@linuxfoundation.org \
    --to=gregkh@linuxfoundation.org \
    --cc=fred@cloudflare.com \
    --cc=jakub@cloudflare.com \
    --cc=kuba@kernel.org \
    --cc=patches@lists.linux.dev \
    --cc=stable@vger.kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).