Re: iked(8): fix some leaks in parse.y
Tobias Heider(tobias.hei...@stusta.de) on 2019.09.25 22:34:53 +0200:
> > Your fix reads ok, but:
> >
> > please write null-pointer checks explicitly, i.e. != NULL or == NULL.
>
> Sure.
>
> > and a suggestion: how about putting a
> >
> > if (head == NULL)
> > return;
> >
> > at the top of iaw_free(), so it works like free()?
> >
> > Then you can skip the if (...) infront of all the calls.
>
> Thanks, makes sense. Not sure how I didn't think of this.
> Here is a cleaned up version:
ok benno@
>
> Index: parse.y
> ===
> RCS file: /cvs/src/sbin/iked/parse.y,v
> retrieving revision 1.83
> diff -u -p -u -r1.83 parse.y
> --- parse.y 26 Aug 2019 16:41:08 - 1.83
> +++ parse.y 25 Sep 2019 20:28:31 -
> @@ -354,10 +354,13 @@ int get_id_type(char *);
> uint8_t x2i(unsigned char *);
> int parsekey(unsigned char *, size_t, struct iked_auth *);
> int parsekeyfile(char *, struct iked_auth *);
> +void iaw_free(struct ipsec_addr_wrap *);
>
> struct ipsec_transforms *ipsec_transforms;
> struct ipsec_filters *ipsec_filters;
> struct ipsec_mode *ipsec_mode;
> +/* interface lookup routintes */
> +struct ipsec_addr_wrap *iftab;
>
> typedef struct {
> union {
> @@ -1630,6 +1633,9 @@ parse_config(const char *filename, struc
> free(sym);
> }
>
> + iaw_free(iftab);
> + iftab = NULL;
> +
> return (errors ? -1 : 0);
> }
>
> @@ -2184,10 +2190,6 @@ host_any(void)
> return (ipa);
> }
>
> -/* interface lookup routintes */
> -
> -struct ipsec_addr_wrap *iftab;
> -
> void
> ifa_load(void)
> {
> @@ -3040,7 +3042,17 @@ done:
> free(p->prop_xforms);
> free(p);
> }
> -
> + if (peers != NULL) {
> + iaw_free(peers->src);
> + iaw_free(peers->dst);
> + /* peers is static, cannot be freed */
> + }
> + if (hosts != NULL) {
> + iaw_free(hosts->src);
> + iaw_free(hosts->dst);
> + free(hosts);
> + }
> + iaw_free(ikecfg);
> return (ret);
> }
>
> @@ -3066,4 +3078,24 @@ create_user(const char *user, const char
>
> rules++;
> return (0);
> +}
> +
> +void
> +iaw_free(struct ipsec_addr_wrap *head)
> +{
> + struct ipsec_addr_wrap *n, *cur;
> +
> + if (head == NULL)
> + return;
> +
> + for (n = head; n != NULL; ) {
> + cur = n;
> + n = n->next;
> + if (cur->srcnat != NULL) {
> + free(cur->srcnat->name);
> + free(cur->srcnat);
> + }
> + free(cur->name);
> + free(cur);
> + }
> }
>
Re: iked(8): fix some leaks in parse.y
On Wed, Sep 25, 2019 at 10:34:53PM +0200, Tobias Heider wrote:
> Thanks, makes sense. Not sure how I didn't think of this.
> Here is a cleaned up version:
OK bluhm@
> Index: parse.y
> ===
> RCS file: /cvs/src/sbin/iked/parse.y,v
> retrieving revision 1.83
> diff -u -p -u -r1.83 parse.y
> --- parse.y 26 Aug 2019 16:41:08 - 1.83
> +++ parse.y 25 Sep 2019 20:28:31 -
> @@ -354,10 +354,13 @@ int get_id_type(char *);
> uint8_t x2i(unsigned char *);
> int parsekey(unsigned char *, size_t, struct iked_auth *);
> int parsekeyfile(char *, struct iked_auth *);
> +void iaw_free(struct ipsec_addr_wrap *);
>
> struct ipsec_transforms *ipsec_transforms;
> struct ipsec_filters *ipsec_filters;
> struct ipsec_mode *ipsec_mode;
> +/* interface lookup routintes */
> +struct ipsec_addr_wrap *iftab;
>
> typedef struct {
> union {
> @@ -1630,6 +1633,9 @@ parse_config(const char *filename, struc
> free(sym);
> }
>
> + iaw_free(iftab);
> + iftab = NULL;
> +
> return (errors ? -1 : 0);
> }
>
> @@ -2184,10 +2190,6 @@ host_any(void)
> return (ipa);
> }
>
> -/* interface lookup routintes */
> -
> -struct ipsec_addr_wrap *iftab;
> -
> void
> ifa_load(void)
> {
> @@ -3040,7 +3042,17 @@ done:
> free(p->prop_xforms);
> free(p);
> }
> -
> + if (peers != NULL) {
> + iaw_free(peers->src);
> + iaw_free(peers->dst);
> + /* peers is static, cannot be freed */
> + }
> + if (hosts != NULL) {
> + iaw_free(hosts->src);
> + iaw_free(hosts->dst);
> + free(hosts);
> + }
> + iaw_free(ikecfg);
> return (ret);
> }
>
> @@ -3066,4 +3078,24 @@ create_user(const char *user, const char
>
> rules++;
> return (0);
> +}
> +
> +void
> +iaw_free(struct ipsec_addr_wrap *head)
> +{
> + struct ipsec_addr_wrap *n, *cur;
> +
> + if (head == NULL)
> + return;
> +
> + for (n = head; n != NULL; ) {
> + cur = n;
> + n = n->next;
> + if (cur->srcnat != NULL) {
> + free(cur->srcnat->name);
> + free(cur->srcnat);
> + }
> + free(cur->name);
> + free(cur);
> + }
> }
Re: iked(8): fix some leaks in parse.y
> Your fix reads ok, but:
>
> please write null-pointer checks explicitly, i.e. != NULL or == NULL.
Sure.
> and a suggestion: how about putting a
>
> if (head == NULL)
> return;
>
> at the top of iaw_free(), so it works like free()?
>
> Then you can skip the if (...) infront of all the calls.
Thanks, makes sense. Not sure how I didn't think of this.
Here is a cleaned up version:
Index: parse.y
===
RCS file: /cvs/src/sbin/iked/parse.y,v
retrieving revision 1.83
diff -u -p -u -r1.83 parse.y
--- parse.y 26 Aug 2019 16:41:08 - 1.83
+++ parse.y 25 Sep 2019 20:28:31 -
@@ -354,10 +354,13 @@ intget_id_type(char *);
uint8_t x2i(unsigned char *);
int parsekey(unsigned char *, size_t, struct iked_auth *);
int parsekeyfile(char *, struct iked_auth *);
+voidiaw_free(struct ipsec_addr_wrap *);
struct ipsec_transforms *ipsec_transforms;
struct ipsec_filters *ipsec_filters;
struct ipsec_mode *ipsec_mode;
+/* interface lookup routintes */
+struct ipsec_addr_wrap *iftab;
typedef struct {
union {
@@ -1630,6 +1633,9 @@ parse_config(const char *filename, struc
free(sym);
}
+ iaw_free(iftab);
+ iftab = NULL;
+
return (errors ? -1 : 0);
}
@@ -2184,10 +2190,6 @@ host_any(void)
return (ipa);
}
-/* interface lookup routintes */
-
-struct ipsec_addr_wrap *iftab;
-
void
ifa_load(void)
{
@@ -3040,7 +3042,17 @@ done:
free(p->prop_xforms);
free(p);
}
-
+ if (peers != NULL) {
+ iaw_free(peers->src);
+ iaw_free(peers->dst);
+ /* peers is static, cannot be freed */
+ }
+ if (hosts != NULL) {
+ iaw_free(hosts->src);
+ iaw_free(hosts->dst);
+ free(hosts);
+ }
+ iaw_free(ikecfg);
return (ret);
}
@@ -3066,4 +3078,24 @@ create_user(const char *user, const char
rules++;
return (0);
+}
+
+void
+iaw_free(struct ipsec_addr_wrap *head)
+{
+ struct ipsec_addr_wrap *n, *cur;
+
+ if (head == NULL)
+ return;
+
+ for (n = head; n != NULL; ) {
+ cur = n;
+ n = n->next;
+ if (cur->srcnat != NULL) {
+ free(cur->srcnat->name);
+ free(cur->srcnat);
+ }
+ free(cur->name);
+ free(cur);
+ }
}
Re: iked(8): fix some leaks in parse.y
Tobias Heider(tobias.hei...@stusta.de) on 2019.09.25 21:29:44 +0200:
> As the subject says this diff fixes a few leaks in the config parser.
Your fix reads ok, but:
please write null-pointer checks explicitly, i.e. != NULL or == NULL.
and a suggestion: how about putting a
if (head == NULL)
return;
at the top of iaw_free(), so it works like free()?
Then you can skip the if (...) infront of all the calls.
/B.
> ok?
>
> Index: parse.y
> ===
> RCS file: /cvs/src/sbin/iked/parse.y,v
> retrieving revision 1.83
> diff -u -p -u -r1.83 parse.y
> --- parse.y 26 Aug 2019 16:41:08 - 1.83
> +++ parse.y 25 Sep 2019 19:28:24 -
> @@ -354,10 +354,13 @@ int get_id_type(char *);
> uint8_t x2i(unsigned char *);
> int parsekey(unsigned char *, size_t, struct iked_auth *);
> int parsekeyfile(char *, struct iked_auth *);
> +void iaw_free(struct ipsec_addr_wrap *);
>
> struct ipsec_transforms *ipsec_transforms;
> struct ipsec_filters *ipsec_filters;
> struct ipsec_mode *ipsec_mode;
> +/* interface lookup routintes */
> +struct ipsec_addr_wrap *iftab;
>
> typedef struct {
> union {
> @@ -1630,6 +1633,9 @@ parse_config(const char *filename, struc
> free(sym);
> }
>
> + iaw_free(iftab);
> + iftab = NULL;
> +
> return (errors ? -1 : 0);
> }
>
> @@ -2184,10 +2190,6 @@ host_any(void)
> return (ipa);
> }
>
> -/* interface lookup routintes */
> -
> -struct ipsec_addr_wrap *iftab;
> -
> void
> ifa_load(void)
> {
> @@ -3040,7 +3042,22 @@ done:
> free(p->prop_xforms);
> free(p);
> }
> -
> + if (peers) {
> + if (peers->src)
> + iaw_free(peers->src);
> + if (peers->dst)
> + iaw_free(peers->dst);
> + /* peers is static, cannot be freed */
> + }
> + if (hosts) {
> + if (hosts->src)
> + iaw_free(hosts->src);
> + if (hosts->dst)
> + iaw_free(hosts->dst);
> + free(hosts);
> + }
> + if (ikecfg)
> + iaw_free(ikecfg);
> return (ret);
> }
>
> @@ -3066,4 +3083,21 @@ create_user(const char *user, const char
>
> rules++;
> return (0);
> +}
> +
> +void
> +iaw_free(struct ipsec_addr_wrap *head)
> +{
> + struct ipsec_addr_wrap *n, *cur;
> +
> + for (n = head; n; ) {
> + cur = n;
> + n = n->next;
> + if (cur->srcnat) {
> + free(cur->srcnat->name);
> + free(cur->srcnat);
> + }
> + free(cur->name);
> + free(cur);
> + }
> }
>
