Page MenuHomeFreeBSD

Static analysis: Improve DAD deletion code
ClosedPublic

Authored by jtl on May 26 2017, 5:06 PM.
Tags
None
Referenced Files
Unknown Object (File)
Mar 12 2024, 10:58 PM
Unknown Object (File)
Dec 23 2023, 12:04 AM
Unknown Object (File)
Oct 17 2023, 11:09 PM
Unknown Object (File)
Aug 27 2023, 8:22 PM
Unknown Object (File)
Aug 13 2023, 12:54 AM
Unknown Object (File)
Aug 1 2023, 6:31 AM
Unknown Object (File)
Jul 9 2023, 11:50 PM
Unknown Object (File)
Jul 9 2023, 11:29 PM
Subscribers

Details

Summary

I ran clang's static analyzer over the kernel sources. It identified some concerns with the way DAD entries were removed from the DAD queue.

This change adds a flag to the DAD entry to indicate whether it is currently on the queue. This prevents accidentally doubly-removing a DAD entry from the queue, while also simplifying some of the logic in nd6_dad_stop().

Sponsored by: Netflix
MFC after: 2 weeks

Diff Detail

Repository
rS FreeBSD src repository - subversion
Lint
Lint Not Applicable
Unit
Tests Not Applicable

Event Timeline

Can someone please review this? This has been outstanding for a while...

This revision is now accepted and ready to land.Mar 22 2018, 11:17 AM
vangyzen added inline comments.
sys/netinet6/nd6_nbr.c
1149 ↗(On Diff #28874)

I don't like the refcount asymmetry between add() and del(), but add() is only called in one place, and this predates your change, so I'm just complaining in your general direction. :)

1307 ↗(On Diff #28874)

At first, I didn't understand whose reference this is releasing. Maybe it's worth a comment, like

/* Release this function's reference, acquired by nd6_dad_find(). */

On the other hand, anyone who works in this code will need to understand this behavior, so maybe the comment is superfluous. Your call.

sys/netinet6/nd6_nbr.c
1149 ↗(On Diff #28874)

Agreed. :-)

1307 ↗(On Diff #28874)

Seems reasonable. I can add the comment.

This revision was automatically updated to reflect the committed changes.