Re: memory leak in net/core/neighbour.c

A.N.Kuznetsov (kuznet@ms2.inr.ac.ru)
Thu, 15 Jan 1998 16:32:35 +0300


In article <34BD3803.F14E9F90@star.net> you wrote:
: There seems to be a small memory leak related to allocating
: neigh_sysctl_table structures. A structure is allocated for every call
: to neigh_sysctl_register and returned as a void *, but in a couple of
: places the returned value is discarded (in devinet.c and
: ipv6/addrconf.c). There would then be no way to free the table.

It is not discarded. Seems, you did not noticed that the line is splitted.
Right?

: I've corrected this by saving the structure in the neigh_parms
: sysctl_table field, and have added a test to make sure the field isn't
: already filled. When allocating a neigh_parms structure the sysctl_table
: field is initialized to NULL.
and
: memcpy(p, &tbl->parms, sizeof(*p));
: + p->sysctl_table = NULL;
: p->reachable_time = neigh_rand_reach_time(p->base_reachable_time);

It is not essential. p->sysctl_table will be reinitialized either
to a real value or to NULL automatically. Though, you are right,
it would make code more transparent.

: The attached patch also corrects a race condition in
: neigh_parms_release, where I think you want the start_bh_atomic
: exclusion before walking the parms list.

Nope. This list is never modified by any interrupts,
so that bh protection is not necessary.
Now only list change is "protected":

: start_bh_atomic();
: *p = parms->next;
: end_bh_atomic();

And it is overkill i.e. it is made only to mark potentially
dangerous place. Assignment is atomic in any case.

Alexey Kuznetsov