• Davide Caratti's avatar
    net/sched: act_gate: fix NULL dereference in tcf_gate_init() · 7024339a
    Davide Caratti authored
    it is possible to see a KASAN use-after-free, immediately followed by a
    NULL dereference crash, with the following command:
    
     # tc action add action gate index 3 cycle-time 100000000ns \
     > cycle-time-ext 100000000ns clockid CLOCK_TAI
    
     BUG: KASAN: use-after-free in tcf_action_init_1+0x8eb/0x960
     Write of size 1 at addr ffff88810a5908bc by task tc/883
    
     CPU: 0 PID: 883 Comm: tc Not tainted 5.7.0+ #188
     Hardware name: Red Hat KVM, BIOS 1.11.1-4.module+el8.1.0+4066+0f1aadab 04/01/2014
     Call Trace:
      dump_stack+0x75/0xa0
      print_address_description.constprop.6+0x1a/0x220
      kasan_report.cold.9+0x37/0x7c
      tcf_action_init_1+0x8eb/0x960
      tcf_action_init+0x157/0x2a0
      tcf_action_add+0xd9/0x2f0
      tc_ctl_action+0x2a3/0x39d
      rtnetlink_rcv_msg+0x5f3/0x920
      netlink_rcv_skb+0x120/0x380
      netlink_unicast+0x439/0x630
      netlink_sendmsg+0x714/0xbf0
      sock_sendmsg+0xe2/0x110
      ____sys_sendmsg+0x5b4/0x890
      ___sys_sendmsg+0xe9/0x160
      __sys_sendmsg+0xd3/0x170
      do_syscall_64+0x9a/0x370
      entry_SYSCALL_64_after_hwframe+0x44/0xa9
    
    [...]
    
     KASAN: null-ptr-deref in range [0x0000000000000070-0x0000000000000077]
     CPU: 0 PID: 883 Comm: tc Tainted: G    B             5.7.0+ #188
     Hardware name: Red Hat KVM, BIOS 1.11.1-4.module+el8.1.0+4066+0f1aadab 04/01/2014
     RIP: 0010:tcf_action_fill_size+0xa3/0xf0
     [....]
     RSP: 0018:ffff88813a48f250 EFLAGS: 00010212
     RAX: dffffc0000000000 RBX: 0000000000000094 RCX: ffffffffa47c3eb6
     RDX: 000000000000000e RSI: 0000000000000008 RDI: 0000000000000070
     RBP: ffff88810a590800 R08: 0000000000000004 R09: ffffed1027491e03
     R10: 0000000000000003 R11: ffffed1027491e03 R12: 0000000000000000
     R13: 0000000000000000 R14: dffffc0000000000 R15: ffff88810a590800
     FS:  00007f62cae8ce40(0000) GS:ffff888147c00000(0000) knlGS:0000000000000000
     CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
     CR2: 00007f62c9d20a10 CR3: 000000013a52a000 CR4: 0000000000340ef0
     Call Trace:
      tcf_action_init+0x172/0x2a0
      tcf_action_add+0xd9/0x2f0
      tc_ctl_action+0x2a3/0x39d
      rtnetlink_rcv_msg+0x5f3/0x920
      netlink_rcv_skb+0x120/0x380
      netlink_unicast+0x439/0x630
      netlink_sendmsg+0x714/0xbf0
      sock_sendmsg+0xe2/0x110
      ____sys_sendmsg+0x5b4/0x890
      ___sys_sendmsg+0xe9/0x160
      __sys_sendmsg+0xd3/0x170
      do_syscall_64+0x9a/0x370
      entry_SYSCALL_64_after_hwframe+0x44/0xa9
    
    this is caused by the test on 'cycletime_ext', that is still unassigned
    when the action is newly created. This makes the action .init() return 0
    without calling tcf_idr_insert(), hence the UAF + crash.
    
    rework the logic that prevents zero values of cycle-time, as follows:
    
    1) 'tcfg_cycletime_ext' seems to be unused in the action software path,
       and it was already possible by other means to obtain non-zero
       cycletime and zero cycletime-ext. So, removing that test should not
       cause any damage.
    2) while at it, we must prevent overwriting configuration data with wrong
       ones: use a temporary variable for 'tcfg_cycletime', and validate it
       preserving the original semantic (that allowed computing the cycle
       time as the sum of all intervals, when not specified by
       TCA_GATE_CYCLE_TIME).
    3) remove the test on 'tcfg_cycletime', no more useful, and avoid
       returning -EFAULT, which did not seem an appropriate return value for
       a wrong netlink attribute.
    
    v3: fix uninitialized 'cycletime' (thanks to Vladimir Oltean)
    v2: remove useless 'return;' at the end of void gate_get_start_time()
    
    Fixes: a51c328d ("net: qos: introduce a gate control flow action")
    CC: Ivan Vecera <ivecera@redhat.com>
    Signed-off-by: default avatarDavide Caratti <dcaratti@redhat.com>
    Acked-by: default avatarVladimir Oltean <vladimir.oltean@nxp.com>
    Tested-by: default avatarVladimir Oltean <vladimir.oltean@nxp.com>
    Signed-off-by: default avatarDavid S. Miller <davem@davemloft.net>
    7024339a
act_gate.c 14.4 KB