[prev in list] [next in list] [prev in thread] [next in thread]
List: openbsd-tech
Subject: Re: test null before free in relayd
From: sven falempin <sven.falempin () gmail ! com>
Date: 2015-05-27 12:09:22
Message-ID: CA++fYEgvoO6DBz=DZ6K=13ECqXP+GwR7V96wP5p9CmkGbAL6yg () mail ! gmail ! com
[Download RAW message or body]
Unmangled diff:
http://pastebin.com/p9Qi9rZX
On Mon, May 25, 2015 at 4:24 PM, sven falempin <sven.falempin@gmail.com>
wrote:
>
>
> On Mon, May 25, 2015 at 2:55 PM, Gleydson Soares <gsoares@gmail.com>
> wrote:
> >
> >
> > your diff is incomplete...
> > well, why just this occurrence? there is others check null before
> > free(3) in same file.
> >
> > and please, be more specific adding more details about your changes: eg.:
> > (I am removing the null check here, because free(3) itself already
> > check against null).
> >
>
> Index: config.c
> ===================================================================
> RCS file: /cvs/src/usr.sbin/./relayd/config.c,v
> retrieving revision 1.25
> diff -u -p -r1.25 config.c
> --- config.c 2 May 2015 13:15:24 -0000 1.25
> +++ config.c 25 May 2015 20:18:34 -0000
> @@ -178,10 +178,8 @@ config_purge(struct relayd *env, u_int r
> if (what & CONFIG_PROTOS && env->sc_protos != NULL) {
> while ((proto = TAILQ_FIRST(env->sc_protos)) != NULL) {
> TAILQ_REMOVE(env->sc_protos, proto, entry);
> - if (proto->style != NULL)
> - free(proto->style);
> - if (proto->tlscapass != NULL)
> - free(proto->tlscapass);
> + free(proto->style);
> + free(proto->tlscapass);
> free(proto);
> }
> env->sc_protocount = 0;
> @@ -949,12 +947,9 @@ config_getrelay(struct relayd *env, stru
> return (0);
>
> fail:
> - if (rlay->rl_tls_cert)
> - free(rlay->rl_tls_cert);
> - if (rlay->rl_tls_key)
> - free(rlay->rl_tls_key);
> - if (rlay->rl_tls_ca)
> - free(rlay->rl_tls_ca);
> + free(rlay->rl_tls_cert);
> + free(rlay->rl_tls_key);
> + free(rlay->rl_tls_ca);
> close(rlay->rl_s);
> free(rlay);
> return (-1);
> @@ -998,7 +993,6 @@ config_getrelaytable(struct relayd *env,
> return (0);
>
> fail:
> - if (rlt != NULL)
> - free(rlt);
> + free(rlt);
> return (-1);
> }
> Index: parse.y
> ===================================================================
> RCS file: /cvs/src/usr.sbin/./relayd/parse.y,v
> retrieving revision 1.204
> diff -u -p -r1.204 parse.y
> --- parse.y 2 May 2015 13:15:24 -0000 1.204
> +++ parse.y 25 May 2015 20:18:34 -0000
> @@ -1275,8 +1275,7 @@ ruleopts : METHOD STRING
> {
> rule->rule_kv[keytype].kv_value == NULL)
> fatal("out of memory");
> free($3);
> - if ($4)
> - free($4);
> + free($4);
> rule->rule_kv[keytype].kv_type = keytype;
> }
> > COOKIE key_option {
> @@ -1296,8 +1295,7 @@ ruleopts : METHOD STRING
> {
> rule->rule_kv[keytype].kv_value == NULL)
> fatal("out of memory");
> free($3);
> - if ($4)
> - free($4);
> + free($4);
> rule->rule_kv[keytype].kv_type = keytype;
> }
> > HEADER key_option {
> @@ -1315,8 +1313,7 @@ ruleopts : METHOD STRING
> {
> rule->rule_kv[keytype].kv_value == NULL)
> fatal("out of memory");
> free($3);
> - if ($4)
> - free($4);
> + free($4);
> rule->rule_kv[keytype].kv_type = keytype;
> }
> > PATH key_option {
> @@ -1332,8 +1329,7 @@ ruleopts : METHOD STRING
> {
> yyerror("combining query type and the
> given "
> "option is not supported");
> free($3);
> - if ($4)
> - free($4);
> + free($4);
> YYERROR;
> break;
> }
> @@ -1346,8 +1342,7 @@ ruleopts : METHOD STRING
> {
> rule->rule_kv[keytype].kv_value == NULL)
> fatal("out of memory");
> free($3);
> - if ($4)
> - free($4);
> + free($4);
> rule->rule_kv[keytype].kv_type = keytype;
> }
> > QUERYSTR key_option {
> @@ -1386,8 +1381,7 @@ ruleopts : METHOD STRING
> {
> rule->rule_kv[keytype].kv_value == NULL)
> fatal("out of memory");
> free($3.digest);
> - if ($4)
> - free($4);
> + free($4);
> rule->rule_kv[keytype].kv_type = keytype;
> }
> > URL key_option {
> Index: relay.c
> ===================================================================
> RCS file: /cvs/src/usr.sbin/./relayd/relay.c,v
> retrieving revision 1.194
> diff -u -p -r1.194 relay.c
> --- relay.c 18 May 2015 16:57:20 -0000 1.194
> +++ relay.c 25 May 2015 20:18:34 -0000
> @@ -1616,15 +1616,13 @@ relay_close(struct rsession *con, const
> con->se_tag != 0 ? tag_id2name(con->se_tag) : "0",
> ibuf,
> obuf, ntohs(con->se_out.port), msg, ptr == NULL ? "" :
> ",",
> ptr == NULL ? "" : ptr);
> - if (ptr != NULL)
> - free(ptr);
> + free(ptr);
> }
>
> if (proto->close != NULL)
> (*proto->close)(con);
>
> - if (con->se_priv != NULL)
> - free(con->se_priv);
> + free(con->se_priv);
> if (con->se_in.bev != NULL)
> bufferevent_free(con->se_in.bev);
> else if (con->se_in.output != NULL)
> @@ -1649,8 +1647,7 @@ relay_close(struct rsession *con, const
> __func__, relay_inflight);
> }
> }
> - if (con->se_in.buf != NULL)
> - free(con->se_in.buf);
> + free(con->se_in.buf);
>
> if (con->se_out.bev != NULL)
> bufferevent_free(con->se_out.bev);
> @@ -1674,8 +1671,7 @@ relay_close(struct rsession *con, const
> }
> }
>
> - if (con->se_out.buf != NULL)
> - free(con->se_out.buf);
> + free(con->se_out.buf);
>
> if (con->se_log != NULL)
> evbuffer_free(con->se_log);
> @@ -2612,8 +2608,7 @@ relay_load_file(const char *name, off_t
> return (buf);
>
> fail:
> - if (buf != NULL)
> - free(buf);
> + free(buf);
> close(fd);
> return (NULL);
> }
> Index: relay_http.c
> ===================================================================
> RCS file: /cvs/src/usr.sbin/./relayd/relay_http.c,v
> retrieving revision 1.47
> diff -u -p -r1.47 relay_http.c
> --- relay_http.c 22 May 2015 01:34:13 -0000 1.47
> +++ relay_http.c 25 May 2015 20:18:34 -0000
> @@ -602,8 +602,7 @@ relay_read_httpchunks(struct bufferevent
> case 0:
> /* Chunk is terminated by an empty newline */
> line = evbuffer_readline(src);
> - if (line != NULL)
> - free(line);
> + free(line);
> if (relay_bufferevent_print(cre->dst, "\r\n") == -1)
> goto fail;
> cre->toread = TOREAD_HTTP_CHUNK_LENGTH;
> @@ -687,8 +686,7 @@ _relay_lookup_url(struct ctl_relay_event
>
> ret = RES_PASS;
> fail:
> - if (md != NULL)
> - free(md);
> + free(md);
> free(val);
> return (ret);
> }
> Index: relay_udp.c
> ===================================================================
> RCS file: /cvs/src/usr.sbin/./relayd/relay_udp.c,v
> retrieving revision 1.39
> diff -u -p -r1.39 relay_udp.c
> --- relay_udp.c 22 Jan 2015 17:42:09 -0000 1.39
> +++ relay_udp.c 25 May 2015 20:18:34 -0000
> @@ -191,8 +191,7 @@ relay_udp_response(int fd, short sig, vo
> return;
>
> relay_close(con, "unknown response");
> - if (priv != NULL)
> - free(priv);
> + free(priv);
> }
>
> void
> @@ -290,8 +289,7 @@ relay_udp_server(int fd, short sig, void
> /* Save the received data */
> if (evbuffer_add(con->se_out.output, buf, len) == -1) {
> relay_close(con, "failed to store buffer");
> - if (cnl != NULL)
> - free(cnl);
> + free(cnl);
> return;
> }
>
> Index: relayd.c
> ===================================================================
> RCS file: /cvs/src/usr.sbin/./relayd/relayd.c,v
> retrieving revision 1.139
> diff -u -p -r1.139 relayd.c
> --- relayd.c 2 May 2015 13:15:24 -0000 1.139
> +++ relayd.c 25 May 2015 20:18:34 -0000
> @@ -447,8 +447,7 @@ parent_dispatch_pfe(int fd, struct privs
> if (IMSG_DATA_SIZE(imsg) > 0)
> str = get_string(imsg->data, IMSG_DATA_SIZE(imsg));
> parent_reload(env, CONFIG_RELOAD, str);
> - if (str != NULL)
> - free(str);
> + free(str);
> break;
> case IMSG_CTL_SHUTDOWN:
> parent_shutdown(env);
> @@ -563,8 +562,7 @@ purge_table(struct relayd *env, struct t
> SSL_free(host->cte.ssl);
> free(host);
> }
> - if (table->sendbuf != NULL)
> - free(table->sendbuf);
> + free(table->sendbuf);
> if (table->conf.flags & F_TLS)
> SSL_CTX_free(table->ssl_ctx);
>
> @@ -693,8 +691,7 @@ kv_set(struct kv *kv, char *fmt, ...)
> }
>
> /* Set the new value */
> - if (kv->kv_value != NULL)
> - free(kv->kv_value);
> + free(kv->kv_value);
> kv->kv_value = value;
>
> return (0);
> @@ -711,8 +708,7 @@ kv_setkey(struct kv *kv, char *fmt, ...)
> return (-1);
> va_end(ap);
>
> - if (kv->kv_key != NULL)
> - free(kv->kv_key);
> + free(kv->kv_key);
> kv->kv_key = key;
>
> return (0);
> @@ -769,13 +765,9 @@ kv_free(struct kv *kv)
> {
> if (kv->kv_type == KEY_TYPE_NONE)
> return;
> - if (kv->kv_key != NULL) {
> - free(kv->kv_key);
> - }
> + free(kv->kv_key);
> kv->kv_key = NULL;
> - if (kv->kv_value != NULL) {
> - free(kv->kv_value);
> - }
> + free(kv->kv_value);
> kv->kv_value = NULL;
> kv->kv_matchtree = NULL;
> kv->kv_match = NULL;
> @@ -929,8 +921,7 @@ rule_add(struct protocol *proto, struct
> kv = &r->rule_kv[i];
> if (kv->kv_type != i)
> continue;
> - if (kv->kv_key != NULL)
> - free(kv->kv_key);
> + free(kv->kv_key);
> if ((kv->kv_key = strdup(buf)) == NULL) {
> rule_free(r);
> free(r);
>
> Also , but more questionable:
>
> Index: relay_http.c
> ===================================================================
> RCS file: /cvs/src/usr.sbin/./relayd/relay_http.c,v
> retrieving revision 1.47
> diff -u -p -r1.47 relay_http.c
> --- relay_http.c 22 May 2015 01:34:13 -0000 1.47
> +++ relay_http.c 25 May 2015 20:23:58 -0000
> @@ -128,28 +128,13 @@ relay_httpdesc_init(struct ctl_relay_eve
> void
> relay_httpdesc_free(struct http_descriptor *desc)
> {
> - if (desc->http_path != NULL) {
> - free(desc->http_path);
> - desc->http_path = NULL;
> - }
> - if (desc->http_query != NULL) {
> - free(desc->http_query);
> - desc->http_query = NULL;
> - }
> - if (desc->http_version != NULL) {
> - free(desc->http_version);
> - desc->http_version = NULL;
> - }
> - if (desc->query_key != NULL) {
> - free(desc->query_key);
> - desc->query_key = NULL;
> - }
> - if (desc->query_val != NULL) {
> - free(desc->query_val);
> - desc->query_val = NULL;
> - }
> + free(desc->http_path);
> + free(desc->http_query);
> + free(desc->http_version);
> + free(desc->query_key);
> + free(desc->query_val);
> kv_purge(&desc->http_headers);
> - desc->http_lastheader = NULL;
> + bzero(desc,sizeof(*desc));
> }
>
> --
>
> ---------------------------------------------------------------------------------------------------------------------
> () ascii ribbon campaign - against html e-mail
> /\
>
--
---------------------------------------------------------------------------------------------------------------------
() ascii ribbon campaign - against html e-mail
/\
[prev in list] [next in list] [prev in thread] [next in thread]
Configure |
About |
News |
Add a list |
Sponsored by KoreLogic