Re: [PATCH net] net: hsr: prevent NULL pointer dereference in hsr_proxy_announce()

[Lukasz Majewski <lukma@denx.de>]

[-- Attachment #1: Type: text/plain, Size: 1721 bytes --]

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@denx.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@denx.de

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]