Skip to content

Commit

Permalink
net: sched: validate stab values
Browse files Browse the repository at this point in the history 
iproute2 package is well behaved, but malicious user space can
provide illegal shift values and trigger UBSAN reports.

Add stab parameter to red_check_params() to validate user input.

syzbot reported:

UBSAN: shift-out-of-bounds in ./include/net/red.h:312:18
shift exponent 111 is too large for 64-bit type 'long unsigned int'
CPU: 1 PID: 14662 Comm: syz-executor.3 Not tainted 5.12.0-rc2-syzkaller #0
Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS Google 01/01/2011
Call Trace:
 __dump_stack lib/dump_stack.c:79 [inline]
 dump_stack+0x141/0x1d7 lib/dump_stack.c:120
 ubsan_epilogue+0xb/0x5a lib/ubsan.c:148
 __ubsan_handle_shift_out_of_bounds.cold+0xb1/0x181 lib/ubsan.c:327
 red_calc_qavg_from_idle_time include/net/red.h:312 [inline]
 red_calc_qavg include/net/red.h:353 [inline]
 choke_enqueue.cold+0x18/0x3dd net/sched/sch_choke.c:221
 __dev_xmit_skb net/core/dev.c:3837 [inline]
 __dev_queue_xmit+0x1943/0x2e00 net/core/dev.c:4150
 neigh_hh_output include/net/neighbour.h:499 [inline]
 neigh_output include/net/neighbour.h:508 [inline]
 ip6_finish_output2+0x911/0x1700 net/ipv6/ip6_output.c:117
 __ip6_finish_output net/ipv6/ip6_output.c:182 [inline]
 __ip6_finish_output+0x4c1/0xe10 net/ipv6/ip6_output.c:161
 ip6_finish_output+0x35/0x200 net/ipv6/ip6_output.c:192
 NF_HOOK_COND include/linux/netfilter.h:290 [inline]
 ip6_output+0x1e4/0x530 net/ipv6/ip6_output.c:215
 dst_output include/net/dst.h:448 [inline]
 NF_HOOK include/linux/netfilter.h:301 [inline]
 NF_HOOK include/linux/netfilter.h:295 [inline]
 ip6_xmit+0x127e/0x1eb0 net/ipv6/ip6_output.c:320
 inet6_csk_xmit+0x358/0x630 net/ipv6/inet6_connection_sock.c:135
 dccp_transmit_skb+0x973/0x12c0 net/dccp/output.c:138
 dccp_send_reset+0x21b/0x2b0 net/dccp/output.c:535
 dccp_finish_passive_close net/dccp/proto.c:123 [inline]
 dccp_finish_passive_close+0xed/0x140 net/dccp/proto.c:118
 dccp_terminate_connection net/dccp/proto.c:958 [inline]
 dccp_close+0xb3c/0xe60 net/dccp/proto.c:1028
 inet_release+0x12e/0x280 net/ipv4/af_inet.c:431
 inet6_release+0x4c/0x70 net/ipv6/af_inet6.c:478
 __sock_release+0xcd/0x280 net/socket.c:599
 sock_close+0x18/0x20 net/socket.c:1258
 __fput+0x288/0x920 fs/file_table.c:280
 task_work_run+0xdd/0x1a0 kernel/task_work.c:140
 tracehook_notify_resume include/linux/tracehook.h:189 [inline]

Fixes: 8afa10c ("net_sched: red: Avoid illegal values")
Signed-off-by: Eric Dumazet <edumazet@google.com>
Reported-by: syzbot <syzkaller@googlegroups.com>
Signed-off-by: David S. Miller <davem@davemloft.net>
  • Loading branch information
Eric Dumazet authored and David S. Miller committed Mar 10, 2021
1 parent 1e1e73e commit e323d86
Show file tree
Hide file tree
Showing 5 changed files with 20 additions and 8 deletions.
10 changes: 9 additions & 1 deletion include/net/red.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -168,7 +168,8 @@ static inline void red_set_vars(struct red_vars *v)
v->qcount = -1;
}

static inline bool red_check_params(u32 qth_min, u32 qth_max, u8 Wlog, u8 Scell_log)
static inline bool red_check_params(u32 qth_min, u32 qth_max, u8 Wlog,
u8 Scell_log, u8 *stab)
{
if (fls(qth_min) + Wlog > 32)
return false;
Expand All @@ -178,6 +179,13 @@ static inline bool red_check_params(u32 qth_min, u32 qth_max, u8 Wlog, u8 Scell_
return false;
if (qth_max < qth_min)
return false;
if (stab) {
int i;

for (i = 0; i < RED_STAB_SIZE; i++)
if (stab[i] >= 32)
return false;
}
return true;
}

Expand Down
7 changes: 4 additions & 3 deletions net/sched/sch_choke.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -345,6 +345,7 @@ static int choke_change(struct Qdisc *sch, struct nlattr *opt,
struct sk_buff **old = NULL;
unsigned int mask;
u32 max_P;
u8 *stab;

if (opt == NULL)
return -EINVAL;
Expand All @@ -361,8 +362,8 @@ static int choke_change(struct Qdisc *sch, struct nlattr *opt,
max_P = tb[TCA_CHOKE_MAX_P] ? nla_get_u32(tb[TCA_CHOKE_MAX_P]) : 0;

ctl = nla_data(tb[TCA_CHOKE_PARMS]);

if (!red_check_params(ctl->qth_min, ctl->qth_max, ctl->Wlog, ctl->Scell_log))
stab = nla_data(tb[TCA_CHOKE_STAB]);
if (!red_check_params(ctl->qth_min, ctl->qth_max, ctl->Wlog, ctl->Scell_log, stab))
return -EINVAL;

if (ctl->limit > CHOKE_MAX_QUEUE)
Expand Down Expand Up @@ -412,7 +413,7 @@ static int choke_change(struct Qdisc *sch, struct nlattr *opt,

red_set_parms(&q->parms, ctl->qth_min, ctl->qth_max, ctl->Wlog,
ctl->Plog, ctl->Scell_log,
nla_data(tb[TCA_CHOKE_STAB]),
stab,
max_P);
red_set_vars(&q->vars);

Expand Down
2 changes: 1 addition & 1 deletion net/sched/sch_gred.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -480,7 +480,7 @@ static inline int gred_change_vq(struct Qdisc *sch, int dp,
struct gred_sched *table = qdisc_priv(sch);
struct gred_sched_data *q = table->tab[dp];

if (!red_check_params(ctl->qth_min, ctl->qth_max, ctl->Wlog, ctl->Scell_log)) {
if (!red_check_params(ctl->qth_min, ctl->qth_max, ctl->Wlog, ctl->Scell_log, stab)) {
NL_SET_ERR_MSG_MOD(extack, "invalid RED parameters");
return -EINVAL;
}
Expand Down
7 changes: 5 additions & 2 deletions net/sched/sch_red.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -242,6 +242,7 @@ static int __red_change(struct Qdisc *sch, struct nlattr **tb,
unsigned char flags;
int err;
u32 max_P;
u8 *stab;

if (tb[TCA_RED_PARMS] == NULL ||
tb[TCA_RED_STAB] == NULL)
Expand All @@ -250,7 +251,9 @@ static int __red_change(struct Qdisc *sch, struct nlattr **tb,
max_P = tb[TCA_RED_MAX_P] ? nla_get_u32(tb[TCA_RED_MAX_P]) : 0;

ctl = nla_data(tb[TCA_RED_PARMS]);
if (!red_check_params(ctl->qth_min, ctl->qth_max, ctl->Wlog, ctl->Scell_log))
stab = nla_data(tb[TCA_RED_STAB]);
if (!red_check_params(ctl->qth_min, ctl->qth_max, ctl->Wlog,
ctl->Scell_log, stab))
return -EINVAL;

err = red_get_flags(ctl->flags, TC_RED_HISTORIC_FLAGS,
Expand Down Expand Up @@ -288,7 +291,7 @@ static int __red_change(struct Qdisc *sch, struct nlattr **tb,
red_set_parms(&q->parms,
ctl->qth_min, ctl->qth_max, ctl->Wlog,
ctl->Plog, ctl->Scell_log,
nla_data(tb[TCA_RED_STAB]),
stab,
max_P);
red_set_vars(&q->vars);

Expand Down
2 changes: 1 addition & 1 deletion net/sched/sch_sfq.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -647,7 +647,7 @@ static int sfq_change(struct Qdisc *sch, struct nlattr *opt)
}

if (ctl_v1 && !red_check_params(ctl_v1->qth_min, ctl_v1->qth_max,
ctl_v1->Wlog, ctl_v1->Scell_log))
ctl_v1->Wlog, ctl_v1->Scell_log, NULL))
return -EINVAL;
if (ctl_v1 && ctl_v1->qth_min) {
p = kmalloc(sizeof(*p), GFP_KERNEL);
Expand Down

0 comments on commit e323d86

Please sign in to comment.