netdev - Re: [PATCH net] net/sched: act_pedit: fix dump of extended layered op

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

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <CAM_iQpV37=Mp1YRvXMM2R=jioNrXog_Wftu2XpOX7eSU6nHtOA@mail.gmail.com>
Date:   Tue, 28 Aug 2018 09:48:19 -0700
From:   Cong Wang <xiyou.wangcong@...il.com>
To:     Davide Caratti <dcaratti@...hat.com>
Cc:     Jamal Hadi Salim <jhs@...atatu.com>,
        David Miller <davem@...emloft.net>,
        Linux Kernel Network Developers <netdev@...r.kernel.org>,
        Amir Vadai <amir@...ai.me>
Subject: Re: [PATCH net] net/sched: act_pedit: fix dump of extended layered op

On Mon, Aug 27, 2018 at 1:56 PM Davide Caratti <dcaratti@...hat.com> wrote:
>
> in the (rare) case of failure in nla_nest_start(), missing NULL checks in
> tcf_pedit_key_ex_dump() can make the following command
>
>  # tc action add action pedit ex munge ip ttl set 64
>
> dereference a NULL pointer:
>
>  BUG: unable to handle kernel NULL pointer dereference at 0000000000000000
>  PGD 800000007d1cd067 P4D 800000007d1cd067 PUD 7acd3067 PMD 0
>  Oops: 0002 [#1] SMP PTI
>  CPU: 0 PID: 3336 Comm: tc Tainted: G            E     4.18.0.pedit+ #425
>  Hardware name: Red Hat KVM, BIOS 0.5.1 01/01/2011
>  RIP: 0010:tcf_pedit_dump+0x19d/0x358 [act_pedit]
>  Code: be 02 00 00 00 48 89 df 66 89 44 24 20 e8 9b b1 fd e0 85 c0 75 46 8b 83 c8 00 00 00 49 83 c5 08 48 03 83 d0 00 00 00 4d 39 f5 <66> 89 04 25 00 00 00 00 0f 84 81 01 00 00 41 8b 45 00 48 8d 4c 24
>  RSP: 0018:ffffb5d4004478a8 EFLAGS: 00010246
>  RAX: ffff8880fcda2070 RBX: ffff8880fadd2900 RCX: 0000000000000000
>  RDX: 0000000000000002 RSI: ffffb5d4004478ca RDI: ffff8880fcda206e
>  RBP: ffff8880fb9cb900 R08: 0000000000000008 R09: ffff8880fcda206e
>  R10: ffff8880fadd2900 R11: 0000000000000000 R12: ffff8880fd26cf40
>  R13: ffff8880fc957430 R14: ffff8880fc957430 R15: ffff8880fb9cb988
>  FS:  00007f75a537a740(0000) GS:ffff8880fda00000(0000) knlGS:0000000000000000
>  CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
>  CR2: 0000000000000000 CR3: 000000007a2fa005 CR4: 00000000001606f0
>  Call Trace:
>   ? __nla_reserve+0x38/0x50
>   tcf_action_dump_1+0xd2/0x130
>   tcf_action_dump+0x6a/0xf0
>   tca_get_fill.constprop.31+0xa3/0x120
>   tcf_action_add+0xd1/0x170
>   tc_ctl_action+0x137/0x150
>   rtnetlink_rcv_msg+0x263/0x2d0
>   ? _cond_resched+0x15/0x40
>   ? rtnl_calcit.isra.30+0x110/0x110
>   netlink_rcv_skb+0x4d/0x130
>   netlink_unicast+0x1a3/0x250
>   netlink_sendmsg+0x2ae/0x3a0
>   sock_sendmsg+0x36/0x40
>   ___sys_sendmsg+0x26f/0x2d0
>   ? do_wp_page+0x8e/0x5f0
>   ? handle_pte_fault+0x6c3/0xf50
>   ? __handle_mm_fault+0x38e/0x520
>   ? __sys_sendmsg+0x5e/0xa0
>   __sys_sendmsg+0x5e/0xa0
>   do_syscall_64+0x5b/0x180
>   entry_SYSCALL_64_after_hwframe+0x44/0xa9
>  RIP: 0033:0x7f75a4583ba0
>  Code: c3 48 8b 05 f2 62 2c 00 f7 db 64 89 18 48 83 cb ff eb dd 0f 1f 80 00 00 00 00 83 3d fd c3 2c 00 00 75 10 b8 2e 00 00 00 0f 05 <48> 3d 01 f0 ff ff 73 31 c3 48 83 ec 08 e8 ae cc 00 00 48 89 04 24
>  RSP: 002b:00007fff60ee7418 EFLAGS: 00000246 ORIG_RAX: 000000000000002e
>  RAX: ffffffffffffffda RBX: 00007fff60ee7540 RCX: 00007f75a4583ba0
>  RDX: 0000000000000000 RSI: 00007fff60ee7490 RDI: 0000000000000003
>  RBP: 000000005b842d3e R08: 0000000000000002 R09: 0000000000000000
>  R10: 00007fff60ee6ea0 R11: 0000000000000246 R12: 0000000000000000
>  R13: 00007fff60ee7554 R14: 0000000000000001 R15: 000000000066c100
>  Modules linked in: act_pedit(E) ip6table_filter ip6_tables iptable_filter binfmt_misc crct10dif_pclmul ext4 crc32_pclmul mbcache ghash_clmulni_intel jbd2 pcbc snd_hda_codec_generic snd_hda_intel snd_hda_codec snd_hda_core snd_hwdep snd_seq snd_seq_device snd_pcm aesni_intel crypto_simd snd_timer cryptd glue_helper snd joydev pcspkr soundcore virtio_balloon i2c_piix4 nfsd auth_rpcgss nfs_acl lockd grace sunrpc ip_tables xfs libcrc32c ata_generic pata_acpi virtio_net net_failover virtio_blk virtio_console failover qxl crc32c_intel drm_kms_helper syscopyarea serio_raw sysfillrect sysimgblt fb_sys_fops ttm drm ata_piix virtio_pci libata virtio_ring i2c_core virtio floppy dm_mirror dm_region_hash dm_log dm_mod [last unloaded: act_pedit]
>  CR2: 0000000000000000
>
> Like it's done for other TC actions, give up dumping pedit rules and return
> an error if nla_nest_start() returns NULL.

Looks good to me,

Acked-by: Cong Wang <xiyou.wangcong@...il.com>

While you are at it, please fix act_tunnel_key too.

Thanks.