Re: [Dnsmasq-discuss] [PATCH] Fix potential memory leak
On Mon, Mar 18, 2024 at 04:12:44PM +, Dan Schaper via Dnsmasq-discuss wrote: > From "Brian Haley" Date 3/18/2024 6:59:21 AM > > > > As an attempt to express that proposed patches get human attention. > > > > I'm not sure what that means... The 'As an attempt to express that proposed patches get human attention' means something like The patch has been seen, it is not completely ignored. > It means Geert doesn't think Simon is running `dnsmasq` to his (Geert's) > liking so Geert is doing yet another passive aggressive attack on the list. > You'll notice how Geert likes to change the email address he sends from with > what he thinks are witty and creative names, like his monthly rules to post > reminders. > > Would be nice to just blanket ban anything from @stappers.nl but until then > it's easier to just set a rule to send anything from that address to spam. > > Geert has no authority to do anything in the `dnsmasq` project and this > latest game with him setting up a patch repository is just a game. Simon is > the sole gateway to getting any code in, much to Geert's dismay. Yeah, the price of doing what is possible. Shoving all and every dnsmasq task to Simon feels wrong. Leaving proposed patches unanswered for days _is_ wrong. In now three weeks am I the only one who made some effort to respond to patch https://lists.thekelleys.org.uk/pipermail/dnsmasq-discuss/2024q1/017464.html and https://lists.thekelleys.org.uk/pipermail/dnsmasq-discuss/2024q1/017473.html Patches being ignored happened before and most likely will happen again. Hence my experiment with dnsmasqmlpc, dnsmasq mailing list patch collector. https://lists.sr.ht/~stappers/dnsmasqmlpc/patches Groeten Geert Stappers -- Silence is hard to parse ___ Dnsmasq-discuss mailing list Dnsmasq-discuss@lists.thekelleys.org.uk https://lists.thekelleys.org.uk/cgi-bin/mailman/listinfo/dnsmasq-discuss
Re: [Dnsmasq-discuss] [PATCH] Fix potential memory leak
On Sun, Mar 17, 2024 at 01:09:36PM -0400, Brian Haley wrote: > Nak. Acknowledge -- Silence is hard to parse ___ Dnsmasq-discuss mailing list Dnsmasq-discuss@lists.thekelleys.org.uk https://lists.thekelleys.org.uk/cgi-bin/mailman/listinfo/dnsmasq-discuss
Re: [Dnsmasq-discuss] [PATCH] Fix potential memory leak
-- Original Message -- From "Brian Haley" To "Geert Stappers" ; dnsmasq-discuss@lists.thekelleys.org.uk Date 3/18/2024 6:59:21 AM Subject Re: [Dnsmasq-discuss] [PATCH] Fix potential memory leak As an attempt to express that proposed patches get human attention. I'm not sure what that means... It means Geert doesn't think Simon is running `dnsmasq` to his (Geert's) liking so Geert is doing yet another passive aggressive attack on the list. You'll notice how Geert likes to change the email address he sends from with what he thinks are witty and creative names, like his monthly rules to post reminders. Would be nice to just blanket ban anything from @stappers.nl but until then it's easier to just set a rule to send anything from that address to spam. Geert has no authority to do anything in the `dnsmasq` project and this latest game with him setting up a patch repository is just a game. Simon is the sole gateway to getting any code in, much to Geert's dismay. Best, Dan ___ Dnsmasq-discuss mailing list Dnsmasq-discuss@lists.thekelleys.org.uk https://lists.thekelleys.org.uk/cgi-bin/mailman/listinfo/dnsmasq-discuss
Re: [Dnsmasq-discuss] [PATCH] Fix potential memory leak
Hi,
On 3/16/24 6:07 AM, Geert Stappers wrote:
On Sat, Mar 02, 2024 at 05:03:01PM +0100, Geert Stappers wrote:
On Fri, Mar 01, 2024 at 04:43:20PM -0500, Brian Haley wrote:
When a new IPv6 address is being added to a dhcp_config
struct, if there is anything invalid regarding the prefix
it looks like there is a potential memory leak.
ret_err_free() should be used to free it.
Signed-off-by: Brian Haley
---
src/option.c | 6 +++---
} 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/src/option.c b/src/option.c
index f4ff7c0..02be995 100644
--- a/src/option.c
+++ b/src/option.c
} @@ -4034,7 +4034,7 @@ static int one_opt(int option, char *arg, char *errstr,
char *gen_err, int comma
u64)1<<(128-new_addr->prefixlen))-1) &
addrpart) != 0)
{
dhcp_config_free(new);
- ret_err(_("bad IPv6 prefix"));
+ ret_err_free(_("bad IPv6 prefix"), new_addr);
}
new_addr->flags |= ADDRLIST_PREFIX;
Looks good to me
New version of the patch is planned.
Again, just wanted to emphasize that I did not agree with the new
version and want this one merged instead.
As an attempt to express that proposed patches get human attention.
I'm not sure what that means...
-Brian
___
Dnsmasq-discuss mailing list
Dnsmasq-discuss@lists.thekelleys.org.uk
https://lists.thekelleys.org.uk/cgi-bin/mailman/listinfo/dnsmasq-discuss
Re: [Dnsmasq-discuss] [PATCH] Fix potential memory leak
Hi,
On 3/17/24 9:38 AM, Geert Stappers wrote:
From: Brian Haley
When a new IPv6 address is being added to a dhcp_config
struct, if there is anything invalid regarding the prefix
it looks like there is a potential memory leak.
ret_err_free() should be used to free it.
Signed-off-by: Brian Haley
---
src/option.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/src/option.c b/src/option.c
index f4ff7c0..db758ce 100644
--- a/src/option.c
+++ b/src/option.c
@@ -4034,7 +4034,7 @@ static int one_opt(int option, char *arg, char *errstr,
char *gen_err, int comma
u64)1<<(128-new_addr->prefixlen))-1) &
addrpart) != 0)
{
dhcp_config_free(new);
- ret_err(_("bad IPv6 prefix"));
+ ret_err_free(_("bad IPv6 prefix"), new_addr);
}
new_addr->flags |= ADDRLIST_PREFIX;
Nak.
I don't believe this stands on it's own - new_addr has been linked into
the list by this point, so freeing it could cause other issues like
invalid memory references. My original submission moved that operation
until later in the function to avoid this, so I would rather see that
version merged.
Thanks,
-Brian
___
Dnsmasq-discuss mailing list
Dnsmasq-discuss@lists.thekelleys.org.uk
https://lists.thekelleys.org.uk/cgi-bin/mailman/listinfo/dnsmasq-discuss
[Dnsmasq-discuss] [PATCH] Fix potential memory leak
From: Brian Haley
When a new IPv6 address is being added to a dhcp_config
struct, if there is anything invalid regarding the prefix
it looks like there is a potential memory leak.
ret_err_free() should be used to free it.
Signed-off-by: Brian Haley
---
src/option.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/src/option.c b/src/option.c
index f4ff7c0..db758ce 100644
--- a/src/option.c
+++ b/src/option.c
@@ -4034,7 +4034,7 @@ static int one_opt(int option, char *arg, char *errstr,
char *gen_err, int comma
u64)1<<(128-new_addr->prefixlen))-1) &
addrpart) != 0)
{
dhcp_config_free(new);
- ret_err(_("bad IPv6 prefix"));
+ ret_err_free(_("bad IPv6 prefix"), new_addr);
}
new_addr->flags |= ADDRLIST_PREFIX;
--
2.30.2
___
Dnsmasq-discuss mailing list
Dnsmasq-discuss@lists.thekelleys.org.uk
https://lists.thekelleys.org.uk/cgi-bin/mailman/listinfo/dnsmasq-discuss
Re: [Dnsmasq-discuss] [PATCH] Fix potential memory leak
On Sat, Mar 02, 2024 at 05:03:01PM +0100, Geert Stappers wrote:
> On Fri, Mar 01, 2024 at 04:43:20PM -0500, Brian Haley wrote:
> > When a new IPv6 address is being added to a dhcp_config
> > struct, if there is anything invalid regarding the prefix
> > it looks like there is a potential memory leak.
> > ret_err_free() should be used to free it.
> >
> > Signed-off-by: Brian Haley
> > ---
> > src/option.c | 6 +++---
> } 1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/src/option.c b/src/option.c
> > index f4ff7c0..02be995 100644
> > --- a/src/option.c
> > +++ b/src/option.c
> } @@ -4034,7 +4034,7 @@ static int one_opt(int option, char *arg, char
> *errstr, char *gen_err, int comma
> > u64)1<<(128-new_addr->prefixlen))-1) &
> > addrpart) != 0)
> > {
> > dhcp_config_free(new);
> > - ret_err(_("bad IPv6 prefix"));
> > + ret_err_free(_("bad IPv6 prefix"), new_addr);
> > }
> >
> > new_addr->flags |= ADDRLIST_PREFIX;
>
> Looks good to me
>
New version of the patch is planned.
As an attempt to express that proposed patches get human attention.
Groeten
Geert Stappers
--
Silence is hard to parse
___
Dnsmasq-discuss mailing list
Dnsmasq-discuss@lists.thekelleys.org.uk
https://lists.thekelleys.org.uk/cgi-bin/mailman/listinfo/dnsmasq-discuss
Re: [Dnsmasq-discuss] [PATCH] Fix potential memory leak
On Fri, Mar 01, 2024 at 04:43:20PM -0500, Brian Haley wrote:
> When a new IPv6 address is being added to a dhcp_config
> struct, if there is anything invalid regarding the prefix
> it looks like there is a potential memory leak.
> ret_err_free() should be used to free it.
>
> Signed-off-by: Brian Haley
> ---
> src/option.c | 6 +++---
> 1 file changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/src/option.c b/src/option.c
> index f4ff7c0..02be995 100644
> --- a/src/option.c
> +++ b/src/option.c
> @@ -4034,7 +4034,7 @@ static int one_opt(int option, char *arg, char *errstr,
> char *gen_err, int comma
> u64)1<<(128-new_addr->prefixlen))-1) &
> addrpart) != 0)
> {
> dhcp_config_free(new);
> - ret_err(_("bad IPv6 prefix"));
> + ret_err_free(_("bad IPv6 prefix"), new_addr);
> }
>
> new_addr->flags |= ADDRLIST_PREFIX;
Looks good to me
> Also, the new addrlist struct is being linked into
> the existing addr6 list in the dhcp_config before the
> validity check, it is best to defer this insertion
> until later so an invalid entry is not present, since
> the CONFIG_ADDR6 flag might not have been set yet.
That is worth its own commit.
Groeten
Geert Stappers
--
Silence is hard to parse
___
Dnsmasq-discuss mailing list
Dnsmasq-discuss@lists.thekelleys.org.uk
https://lists.thekelleys.org.uk/cgi-bin/mailman/listinfo/dnsmasq-discuss
