netdev - Re: [PATCH net] net: hsr: prevent NULL pointer dereference in hsr_proxy

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: <20240911100007.31d600fc@wsk>
Date: Wed, 11 Sep 2024 10:00:07 +0200
From: Lukasz Majewski <lukma@...x.de>
To: Jakub Kicinski <kuba@...nel.org>
Cc: Jeongjun Park <aha310510@...il.com>, davem@...emloft.net,
 edumazet@...gle.com, pabeni@...hat.com, horms@...nel.org,
 ricardo@...liere.net, m-karicheri2@...com, n.zhandarovich@...tech.ru,
 netdev@...r.kernel.org, linux-kernel@...r.kernel.org,
 syzbot+02a42d9b1bd395cbcab4@...kaller.appspotmail.com
Subject: Re: [PATCH net] net: hsr: prevent NULL pointer dereference in
 hsr_proxy_announce()

Hi Jakub,

> On Mon, 9 Sep 2024 10:58:22 +0200 Lukasz Majewski wrote:
> > > In the function hsr_proxy_annouance() added in the previous
> > > commit 5f703ce5c981 ("net: hsr: Send supervisory frames to HSR
> > > network with ProxyNodeTable data"), the return value of the
> > > hsr_port_get_hsr() function is not checked to be a NULL pointer,
> > > which causes a NULL pointer dereference.    
> > 
> > Thank you for your patch.
> > 
> > The code in hsr_proxy_announcement() is _only_ executed (the timer
> > is configured to trigger this function) when hsr->redbox is set,
> > which means that somebody has called earlier iproute2 command:
> > 
> > ip link add name hsr1 type hsr slave1 lan4 slave2 lan5 interlink
> > lan3 supervision 45 version 1  
> 
> Are you trying to say the patch is correct or incorrect?

I'm just trying to explain that this code (i.e.
hsr_proxy_announcement()) shall NOT be trigger if the interlink port is
not configured.

Nonetheless the patch is correct - as it was pointed out that the return
value is not checked.

> The structs have no refcounting - should the timers be deleted with
> _sync() inside hsr_check_announce()?

The timers don't need to be conditionally enabled (and removed) as we
discussed it previously (as they only do useful work when they are
configured and almost take no resources when declared during the
driver probe).

Anyway:

Acked-by: Lukasz Majewski <lukma@...x.de>

Best regards,

Lukasz Majewski

--

DENX Software Engineering GmbH,      Managing Director: Erika Unter
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-59 Fax: (+49)-8142-66989-80 Email: lukma@...x.de

Content of type "application/pgp-signature" skipped