Re: CVS commit: src/sys/netinet

[Mihai Chelaru <mihai.chelaru%ngnetworks.ro@localhost>]

Masaru OKI wrote:

> Module Name:  src
> Committed By: oki
> Date:         Fri Mar 12 13:33:19 UTC 2010
> 
> Modified Files:
> src/sys/netinet: in.c
> 
> Log Message:
> Fixed a number of race conditions in the case of receiving ipv4 packet.
> found by iij seil team.
> 
> 
> To generate a diff of this commit:
> cvs rdiff -u -r1.136 -r1.137 src/sys/netinet/in.c
> 
> Please note that diffs are not public domain; they are subject to the
> copyright notices on the relevant files.

Hi,

Some questions and remarks:

* Can you detail a little bit what are the exactly race conditions this
patch is covering ?
* I think the locking is too exhaustive and the lock paths are too long.
Also, some code rearranging can be done in order to shorten these lock
paths.
* you bothered to splsoftnet() before in_ifinit that does splnet() anyway
* I don't think you need to splsoftnet for pfil_run_hooks
* splsoftnet for if_ioctl ? I think if_ioctls should be fixed instead -
although all I've seen until now are doing splnet for critical paths.
* probably only rtinit blocks - in in_control() and in_scrubprefix() - and
tail operations on ifaddrs should be locked.
* panics when trying to set an IP address for a gre tunnel - rip_usrreq
wants to splsoftnet, too
* it's been pulled-up in netbsd-5. At least the latter panic should be
solved before anything else.

-- 
Mihai