netdev - Re: [net PATCH] hsr: fix a NULL pointer dereference in hsr_dev

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]

Date:   Mon, 2 Dec 2019 01:01:38 +0900
From:   Taehee Yoo <ap420073@...il.com>
To:     Cong Wang <xiyou.wangcong@...il.com>
Cc:     David Miller <davem@...emloft.net>,
        Linux Kernel Network Developers <netdev@...r.kernel.org>,
        treeze.taeung@...il.com
Subject: Re: [net PATCH] hsr: fix a NULL pointer dereference in hsr_dev_xmit()

On Sun, 1 Dec 2019 at 03:36, Cong Wang <xiyou.wangcong@...il.com> wrote:
>

Hi Cong,
Thank you for the review!

> On Sat, Nov 30, 2019 at 6:24 AM Taehee Yoo <ap420073@...il.com> wrote:
> >
> > hsr_dev_xmit() calls hsr_port_get_hsr() to find master node and that would
> > return NULL if master node is not existing in the list.
> > But hsr_dev_xmit() doesn't check return pointer so a NULL dereference
> > could occur.
>
> If you look at the git history, I made a same patch but reverted it later. :)
>

Yes, you already made the same patch and reverted.

>
> >
> > In the TX datapath, there is no rcu_read_lock() so this patch adds missing
> > rcu_read_lock() in the hsr_dev_xmit() too.
>
> This is wrong.
>

I thought hsr_dev_xmit() needs rcu_read_lock() because hsr_port_get_hsr() and
hsr_forward_skb() should be called under rcu_read_lock().
Could you let me know about the reason?

>
> >
> > Test commands:
> >     ip netns add nst
> >     ip link add v0 type veth peer name v1
> >     ip link add v2 type veth peer name v3
> >     ip link set v1 netns nst
> >     ip link set v3 netns nst
> >     ip link add hsr0 type hsr slave1 v0 slave2 v2
> >     ip a a 192.168.100.1/24 dev hsr0
> >     ip link set v0 up
> >     ip link set v2 up
> >     ip link set hsr0 up
> >     ip netns exec nst ip link add hsr1 type hsr slave1 v1 slave2 v3
> >     ip netns exec nst ip a a 192.168.100.2/24 dev hsr1
> >     ip netns exec nst ip link set v1 up
> >     ip netns exec nst ip link set v3 up
> >     ip netns exec nst ip link set hsr1 up
> >     hping3 192.168.100.2 -2 --flood &
> >     modprobe -rv hsr
>
> Looks like the master port got deleted without respecting RCU
> readers, let me look into it.
>
> Thanks!

Thank you!
Taehee