On Tue, Aug 08, 2000 at 04:57:06PM -0700, Timothy Roscoe wrote:
> This is a replacement of the IGMPv2 code in net/ipv4/ with IGMPv3.
[...]
I started to look over your code. First it is a lot harder to review
than necessary, because you were changing white space of unchanged code
all over (please do not do that). Also the 2.4 patch seems to recreate
net/ipv4/af_inet.c which does not look correct.
More comments on the 2.4 version:
You do not seem to have any SMP locks on the srclist (you're actually
removing the existing locking). This could lead to SMP data corruption.
Please readd the im->lock spinlocking and the reference counting for the
object. Other data structures seem to have the same problem.
igmp_send_state_report is called from a timer, but uses GFP_KERNEL allocation.
IP_MULTICAST_FILTER: you do not check mfilt.imsf_numsrc is reasonable,
so an user could allocate 128K of kernel memory. SIOCSIPMSFILTER seems
to have the same problem. The memory should probably be accounted to the
socket buffer anyways (sock_kmalloc)
UDP: linear list search looks slow, can't you use a simple closed hash table
for that?
Only small parts of e.g. the filter code seems to be covered by
CONFIG_IP_MULTICAST, all should.
robustness should probably be a per device variable, not global. It also
needs a atomic updates or a lock.
im->tm_running looks very SMP racy.
-Andi
|