| Message ID | 20220814112758.3088655-1-jhs@mojatatu.com (mailing list archive) |
|---|---|
| State | Accepted |
| Commit | 02799571714dc5dd6948824b9d080b44a295f695 |
| Delegated to: | Netdev Maintainers |
| Headers | show |
| Series | [net,1/1] net_sched: cls_route: disallow handle of 0 | expand |
On Sun, 14 Aug 2022 11:27:58 +0000 Jamal Hadi Salim <jhs@mojatatu.com> wrote: > Follows up on: > https://lore.kernel.org/all/20220809170518.164662-1-cascardo@canonical.com/ > > handle of 0 implies from/to of universe realm which is not very > sensible. > > Lets see what this patch will do: > $sudo tc qdisc add dev $DEV root handle 1:0 prio > > //lets manufacture a way to insert handle of 0 > $sudo tc filter add dev $DEV parent 1:0 protocol ip prio 100 \ > route to 0 from 0 classid 1:10 action ok > > //gets rejected... > Error: handle of 0 is not valid. > We have an error talking to the kernel, -1 > > //lets create a legit entry.. > sudo tc filter add dev $DEV parent 1:0 protocol ip prio 100 route from 10 \ > classid 1:10 action ok > > //what did the kernel insert? > $sudo tc filter ls dev $DEV parent 1:0 > filter protocol ip pref 100 route chain 0 > filter protocol ip pref 100 route chain 0 fh 0x000a8000 flowid 1:10 from 10 > action order 1: gact action pass > random type none pass val 0 > index 1 ref 1 bind 1 > > //Lets try to replace that legit entry with a handle of 0 > $ sudo tc filter replace dev $DEV parent 1:0 protocol ip prio 100 \ > handle 0x000a8000 route to 0 from 0 classid 1:10 action drop > > Error: Replacing with handle of 0 is invalid. > We have an error talking to the kernel, -1 > > And last, lets run Cascardo's POC: > $ ./poc > 0 > 0 > -22 > -22 > -22 > > Signed-off-by: Jamal Hadi Salim <jhs@mojatatu.com> Acked-by: Stephen Hemminger <stephen@networkplumber.org>
Hello: This patch was applied to netdev/net.git (master) by David S. Miller <davem@davemloft.net>: On Sun, 14 Aug 2022 11:27:58 +0000 you wrote: > Follows up on: > https://lore.kernel.org/all/20220809170518.164662-1-cascardo@canonical.com/ > > handle of 0 implies from/to of universe realm which is not very > sensible. > > Lets see what this patch will do: > $sudo tc qdisc add dev $DEV root handle 1:0 prio > > [...] Here is the summary with links: - [net,1/1] net_sched: cls_route: disallow handle of 0 https://git.kernel.org/netdev/net/c/02799571714d You are awesome, thank you!
On Sun, 14 Aug 2022 11:27:58 +0000 Jamal Hadi Salim wrote: > Follows up on: > https://lore.kernel.org/all/20220809170518.164662-1-cascardo@canonical.com/ > > handle of 0 implies from/to of universe realm which is not very > sensible. Heh, I was gonna say, but then you acked the other fix :)
The earlier discussion was to let the other fix in to plug the CVE hole (I had proposed disallowing handle of 0 in that discussion). cheers, jamal On Mon, Aug 15, 2022 at 1:44 PM Jakub Kicinski <kuba@kernel.org> wrote: > > On Sun, 14 Aug 2022 11:27:58 +0000 Jamal Hadi Salim wrote: > > Follows up on: > > https://lore.kernel.org/all/20220809170518.164662-1-cascardo@canonical.com/ > > > > handle of 0 implies from/to of universe realm which is not very > > sensible. > > Heh, I was gonna say, but then you acked the other fix :)
Jamal Hadi Salim <jhs@mojatatu.com> writes: > Follows up on: > https://lore.kernel.org/all/20220809170518.164662-1-cascardo@canonical.com/ > > handle of 0 implies from/to of universe realm which is not very > sensible. Hi, This was posted two weeks ago and now that it's merged, could you please post to oss-security? I don't think there was an actual embargo on this one. Regards, Anthony Liguori
On Mon, Aug 29, 2022 at 11:26 AM Anthony Liguori <aliguori@amazon.com> wrote: > > Jamal Hadi Salim <jhs@mojatatu.com> writes: > > > Follows up on: > > https://lore.kernel.org/all/20220809170518.164662-1-cascardo@canonical.com/ > > > > handle of 0 implies from/to of universe realm which is not very > > sensible. > > Hi, > > This was posted two weeks ago and now that it's merged, could you please > post to oss-security? > > I don't think there was an actual embargo on this one. > Apologies, not familiar with the process and wasnt sure if this was addressed to me. Are you asking for this to be cross-posted to oss-security? cheers, jamal
diff --git a/net/sched/cls_route.c b/net/sched/cls_route.c index 3f935cbbaff6..48712bc51bda 100644 --- a/net/sched/cls_route.c +++ b/net/sched/cls_route.c @@ -424,6 +424,11 @@ static int route4_set_parms(struct net *net, struct tcf_proto *tp, return -EINVAL; } + if (!nhandle) { + NL_SET_ERR_MSG(extack, "Replacing with handle of 0 is invalid"); + return -EINVAL; + } + h1 = to_hash(nhandle); b = rtnl_dereference(head->table[h1]); if (!b) { @@ -477,6 +482,11 @@ static int route4_change(struct net *net, struct sk_buff *in_skb, int err; bool new = true; + if (!handle) { + NL_SET_ERR_MSG(extack, "Creating with handle of 0 is invalid"); + return -EINVAL; + } + if (opt == NULL) return handle ? -EINVAL : 0;
Follows up on: https://lore.kernel.org/all/20220809170518.164662-1-cascardo@canonical.com/ handle of 0 implies from/to of universe realm which is not very sensible. Lets see what this patch will do: $sudo tc qdisc add dev $DEV root handle 1:0 prio //lets manufacture a way to insert handle of 0 $sudo tc filter add dev $DEV parent 1:0 protocol ip prio 100 \ route to 0 from 0 classid 1:10 action ok //gets rejected... Error: handle of 0 is not valid. We have an error talking to the kernel, -1 //lets create a legit entry.. sudo tc filter add dev $DEV parent 1:0 protocol ip prio 100 route from 10 \ classid 1:10 action ok //what did the kernel insert? $sudo tc filter ls dev $DEV parent 1:0 filter protocol ip pref 100 route chain 0 filter protocol ip pref 100 route chain 0 fh 0x000a8000 flowid 1:10 from 10 action order 1: gact action pass random type none pass val 0 index 1 ref 1 bind 1 //Lets try to replace that legit entry with a handle of 0 $ sudo tc filter replace dev $DEV parent 1:0 protocol ip prio 100 \ handle 0x000a8000 route to 0 from 0 classid 1:10 action drop Error: Replacing with handle of 0 is invalid. We have an error talking to the kernel, -1 And last, lets run Cascardo's POC: $ ./poc 0 0 -22 -22 -22 Signed-off-by: Jamal Hadi Salim <jhs@mojatatu.com> --- net/sched/cls_route.c | 10 ++++++++++ 1 file changed, 10 insertions(+)