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:   Sat, 30 Nov 2019 10:34:11 -0800 (PST)
From:   David Miller <davem@...emloft.net>
To:     ap420073@...il.com
Cc:     xiyou.wangcong@...il.com, netdev@...r.kernel.org,
        treeze.taeung@...il.com
Subject: Re: [net PATCH] hsr: fix a NULL pointer dereference in
 hsr_dev_xmit()

From: Taehee Yoo <ap420073@...il.com>
Date: Sat, 30 Nov 2019 14:24:00 +0000

> @@ -226,9 +226,16 @@ static int hsr_dev_xmit(struct sk_buff *skb, struct net_device *dev)
>  	struct hsr_priv *hsr = netdev_priv(dev);
>  	struct hsr_port *master;
>  
> +	rcu_read_lock();
>  	master = hsr_port_get_hsr(hsr, HSR_PT_MASTER);

I don't want to distract from your bug fix but I had to audit and learn
how this hsr->ports stuff works while reading your patch.

hsr->ports has supposedly RCU protection...

But add and delete opertions to the port list only occur by newlink
netlink operations (the device isn't even visible yet at this point)
and network device teardown (all packet processing paths will quiesce
beforehand).

Therefore, the port list never changes from it's effectively static
configuration made at hsr_dev_finalize() time.

The whole driver very inconsistently accesses the hsr->port list,
and it all works only because of the above invariant.

So let's not try to fix the RCU protection issues here ok?  That
should be handled separately, and there are no real problems caused by
the lack of RCU protection here right now.

Thank you.