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] [day] [month] [year] [list]

Date:   Thu, 5 Dec 2019 16:00:06 +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 Thu, 5 Dec 2019 at 02:30, Cong Wang <xiyou.wangcong@...il.com> wrote:
>

Hi Cong,

> 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.
> >
> > In the TX datapath, there is no rcu_read_lock() so this patch adds missing
> > rcu_read_lock() in the hsr_dev_xmit() too.
>
> Just a correction:
> The TX path _has_ RCU read lock, but it harms nothing to take it again.
>
> [...]
> >
> > Fixes: 311633b60406 ("hsr: switch ->dellink() to ->ndo_uninit()")
> > Signed-off-by: Taehee Yoo <ap420073@...il.com>
>
> This fix is correct. There is no other way to workaround this RCU
> rule, checking against NULL is the only way to fix RCU reader
> races, so:
>
> Acked-by: Cong Wang <xiyou.wangcong@...il.com>
>
>
> Taehee, you might have to resend this with my Acked-by as David
> probably already drops it from patchwork.
>
> Thanks.

Thank you so much for your detailed reviews.

I will send a v2 patch that will not include unnecessary rcu_read_lock().

Thank you!
Taehee Yoo