From b84681e7ab1e7f281ae42ba334fc77a2bb5ce007 Mon Sep 17 00:00:00 2001 From: luigi Date: Fri, 5 Mar 2010 17:53:28 +0000 Subject: [PATCH] plug a memory leak on pipe's reconfiguration --- sys/netinet/ipfw/ip_dummynet.c | 49 ++++++++++++++++++++-------------- 1 file changed, 29 insertions(+), 20 deletions(-) diff --git a/sys/netinet/ipfw/ip_dummynet.c b/sys/netinet/ipfw/ip_dummynet.c index 8e45705da8b8..842f17e7a92f 100644 --- a/sys/netinet/ipfw/ip_dummynet.c +++ b/sys/netinet/ipfw/ip_dummynet.c @@ -1320,12 +1320,13 @@ config_sched(struct dn_sch *_nsch, struct dn_id *arg) struct schk_new_arg a; /* argument for schk_new */ int i; struct dn_link p; /* copy of oldlink */ - struct dn_profile *pf; /* copy of old link profile */ + struct dn_profile *pf = NULL; /* copy of old link profile */ /* Used to preserv mask parameter */ struct ipfw_flow_id new_mask; int new_buckets = 0; int new_flags = 0; int pipe_cmd; + int err = ENOMEM; a.sch = _nsch; if (a.sch->oid.len != sizeof(*a.sch)) { @@ -1341,11 +1342,6 @@ config_sched(struct dn_sch *_nsch, struct dn_id *arg) 1, dn_cfg.max_hash_size, "sched buckets"); /* XXX other sanity checks */ bzero(&p, sizeof(p)); - pf = malloc(sizeof(struct dn_profile), M_DUMMYNET, M_NOWAIT | M_ZERO); - if (pf == NULL) { - D("Error allocating profile"); - return ENOMEM; - } pipe_cmd = a.sch->flags & DN_PIPE_CMD; a.sch->flags &= ~DN_PIPE_CMD; //XXX do it even if is not set? @@ -1390,27 +1386,33 @@ config_sched(struct dn_sch *_nsch, struct dn_id *arg) goto again; } } else { - DN_BH_WUNLOCK(); D("invalid scheduler type %d %s", a.sch->oid.subtype, a.sch->name); - return EINVAL; + err = EINVAL; + goto error; } /* normalize name and subtype */ a.sch->oid.subtype = a.fp->type; bzero(a.sch->name, sizeof(a.sch->name)); strlcpy(a.sch->name, a.fp->name, sizeof(a.sch->name)); if (s == NULL) { - DN_BH_WUNLOCK(); D("cannot allocate scheduler %d", i); - return ENOMEM; + goto error; } /* restore existing link if any */ if (p.link_nr) { s->link = p; - if (pf->link_nr == p.link_nr) /* Restore profile */ - s->profile = pf; - else + if (!pf || pf->link_nr != p.link_nr) { /* no saved value */ s->profile = NULL; /* XXX maybe not needed */ + } else { + s->profile = malloc(sizeof(struct dn_profile), + M_DUMMYNET, M_NOWAIT | M_ZERO); + if (s->profile == NULL) { + D("cannot allocate profile"); + goto error; //XXX + } + bcopy(pf, s->profile, sizeof(*pf)); + } } p.link_nr = 0; if (s->fp == NULL) { @@ -1426,8 +1428,13 @@ config_sched(struct dn_sch *_nsch, struct dn_id *arg) if (s->link.link_nr == 0) D("XXX WARNING link 0 for sched %d", i); p = s->link; /* preserve link */ - if (s->profile) /* preserve profile */ - bcopy(s->profile, pf, sizeof(struct dn_profile)); + if (s->profile) {/* preserve profile */ + if (!pf) + pf = malloc(sizeof(*pf), + M_DUMMYNET, M_NOWAIT | M_ZERO); + if (pf) /* XXX should issue a warning otherwise */ + bcopy(s->profile, pf, sizeof(*pf)); + } /* remove from the hash */ dn_ht_find(dn_cfg.schedhash, i, DNHT_REMOVE, NULL); /* Detach flowsets, preserve queues. */ @@ -1459,8 +1466,7 @@ config_sched(struct dn_sch *_nsch, struct dn_id *arg) if (!s->fs) { schk_delete_cb(s, (void *)DN_DESTROY); D("error creating internal fs for %d", i); - DN_BH_WUNLOCK(); - return ENOMEM; + goto error; } } /* call init function after the flowset is created */ @@ -1479,8 +1485,7 @@ config_sched(struct dn_sch *_nsch, struct dn_id *arg) /* sched config shouldn't modify the FIFO scheduler */ if (dn_ht_find(dn_cfg.schedhash, i, 0, &a) != NULL) { /* FIFO already exist, don't touch it */ - DN_BH_WUNLOCK(); - return 0; + goto error; } } a.sch->sched_nr = i; @@ -1488,8 +1493,12 @@ config_sched(struct dn_sch *_nsch, struct dn_id *arg) bzero(a.sch->name, sizeof(a.sch->name)); goto again; } + err = 0; +error: DN_BH_WUNLOCK(); - return 0; + if (pf) + free(pf, M_DUMMYNET); + return err; } /*