Re: [Dnsmasq-discuss] Use-after-free with DHCP + use-stale-cache

2024-05-13 Thread Erik Karlsson
Hi Dominik,

SIGALRM is in fact what is used for expiring leases so this patch should
affect the relevant code path:

[PATCH] Update DNS records after pruning DHCP leases

As far as I can tell this is also the only place where lease_prune() is not
followed by lease_update_dns()

I found the issue by analyzing a core dump from a rare occasion where it
crashes even when use-stale-cache is not enabled. From the core dump it was
evident that dns_dirty was 1

Best regards,
Erik

Den mån 13 maj 2024 21:29Dominik Derigs  skrev:

> Resending because I received an error message from Gmail about issues with
> incoming mail before
>
> Please let me know if my message reached you
>
> Dominik
>
>
> --
> *Von:*
> Dominik Derigs 
>
> *Gesendet:* 13. Mai 2024 21:12:23 MESZ
> *An:*
> Erik Karlsson , Simon Kelley <
> si...@thekelleys.org.uk>
>
> *CC:*
> dnsmasq-discuss 
>
> *Betreff:*
> Re: [Dnsmasq-discuss] Use-after-free with DHCP + use-stale-cache
>
>
> Hey Erik,
>
> sorry for the late reply.. I wanted to err on the side of caution this
> time. We have been testing with your patch applied on top of latest master
> for almost four days now and - so far - no new use-after-free events
> occurred. Before, it happened at least once a day. Seems I have
> misinterpreted when SIGALRM is used so I thought your patch wouldn't be
> effective in our case. Sorry for this and thanks for challenging my earlier
> statement.
>
> Best,
> Dominik
> On 06.05.24 11:39, Erik Karlsson wrote:
>
> Hi Dominik,
>
> Are you sure the patch I sent does not solve this? I think it should or
> are there more places where a lease_update_dns(0) is missing?
> Alternatively, can there be dangling pointers left even
> after lease_update_dns has been run?
>
> Best regards,
> Erik
>
> Den mån 6 maj 2024 07:14Dominik Derigs via Dnsmasq-discuss <
> dnsmasq-discuss@lists.thekelleys.org.uk> skrev:
>
>> Hey Simon,
>>
>> we found a bug resulting in a use-after-free returning garbage data and
>> possibly crash when using DHCP + stale cache data.
>>
>> The bug is triggered when using DHCP and a lease expires. It's name is
>> then free'd in kill_name() + do_script_run(). When the PTR record is
>> queried thereafter and use-stale-cache is enabled, dnsmasq accesses this
>> dangling pointer and returns random data - often a string containing a few
>> control characters, once dnsmasq even SEGFAULTed.
>>
>> Related dnsmasq.log:
>>
>> May  5 19:00:00 dnsmasq[4395]: query[PTR] 141.2.168.192.in-addr.arpa from 
>> 127.0.0.1May  5 19:00:00 dnsmasq[4395]: DHCP 192.168.2.141 is **> unprintable>**May  5 19:00:00 dnsmasq[4395]: forwarded 
>> 141.2.168.192.in-addr.arpa to 1.0.0.1
>>
>> The final immediate "forwarded" line comes from dnsmasq itself and
>> confirms that this was triggered by use-stale-cache.
>>
>> Best,
>> Dominik
>>
>> P.S.: The patch recently sent by Erik Karlsson doesn't fix this, it
>> touches other code.
>> ___
>> Dnsmasq-discuss mailing list
>> Dnsmasq-discuss@lists.thekelleys.org.uk
>> https://lists.thekelleys.org.uk/cgi-bin/mailman/listinfo/dnsmasq-discuss
>>
>
___
Dnsmasq-discuss mailing list
Dnsmasq-discuss@lists.thekelleys.org.uk
https://lists.thekelleys.org.uk/cgi-bin/mailman/listinfo/dnsmasq-discuss


Re: [Dnsmasq-discuss] Use-after-free with DHCP + use-stale-cache

2024-05-13 Thread Dominik Derigs via Dnsmasq-discuss

Hey Erik,

sorry for the late reply.. I wanted to err on the side of caution this 
time. We have been testing with your patch applied on top of latest 
master for almost four days now and - so far - no new use-after-free 
events occurred. Before, it happened at least once a day. Seems I have 
misinterpreted when SIGALRM is used so I thought your patch wouldn't be 
effective in our case. Sorry for this and thanks for challenging my 
earlier statement.


Best,
Dominik

On 06.05.24 11:39, Erik Karlsson wrote:

Hi Dominik,

Are you sure the patch I sent does not solve this? I think it should 
or are there more places where a lease_update_dns(0) is missing? 
Alternatively, can there be dangling pointers left even 
after lease_update_dns has been run?


Best regards,
Erik

Den mån 6 maj 2024 07:14Dominik Derigs via Dnsmasq-discuss 
 skrev:


Hey Simon,

we found a bug resulting in a use-after-free returning garbage
data and possibly crash when using DHCP + stale cache data.

The bug is triggered when using DHCP and a lease expires. It's
name is then free'd in kill_name() + do_script_run(). When the PTR
record is queried thereafter and use-stale-cache is enabled,
dnsmasq accesses this dangling pointer and returns random data -
often a string containing a few control characters, once dnsmasq
even SEGFAULTed.

Related dnsmasq.log:

|May 5 19:00:00 dnsmasq[4395]: query[PTR]
141.2.168.192.in-addr.arpa from 127.0.0.1 May 5 19:00:00
dnsmasq[4395]: DHCP 192.168.2.141 is  May 5
19:00:00 dnsmasq[4395]: forwarded 141.2.168.192.in-addr.arpa to
1.0.0.1|

The final immediate "forwarded" line comes from dnsmasq itself and
confirms that this was triggered by use-stale-cache.

Best,
Dominik

P.S.: The patch recently sent by Erik Karlsson doesn't fix this,
it touches other code.

___
Dnsmasq-discuss mailing list
Dnsmasq-discuss@lists.thekelleys.org.uk
https://lists.thekelleys.org.uk/cgi-bin/mailman/listinfo/dnsmasq-discuss
___
Dnsmasq-discuss mailing list
Dnsmasq-discuss@lists.thekelleys.org.uk
https://lists.thekelleys.org.uk/cgi-bin/mailman/listinfo/dnsmasq-discuss


Re: [Dnsmasq-discuss] Use-after-free with DHCP + use-stale-cache

2024-05-06 Thread Erik Karlsson
Hi Dominik,

Are you sure the patch I sent does not solve this? I think it should or are
there more places where a lease_update_dns(0) is missing? Alternatively,
can there be dangling pointers left even after lease_update_dns has been
run?

Best regards,
Erik

Den mån 6 maj 2024 07:14Dominik Derigs via Dnsmasq-discuss <
dnsmasq-discuss@lists.thekelleys.org.uk> skrev:

> Hey Simon,
>
> we found a bug resulting in a use-after-free returning garbage data and
> possibly crash when using DHCP + stale cache data.
>
> The bug is triggered when using DHCP and a lease expires. It's name is
> then free'd in kill_name() + do_script_run(). When the PTR record is
> queried thereafter and use-stale-cache is enabled, dnsmasq accesses this
> dangling pointer and returns random data - often a string containing a few
> control characters, once dnsmasq even SEGFAULTed.
>
> Related dnsmasq.log:
>
> May  5 19:00:00 dnsmasq[4395]: query[PTR] 141.2.168.192.in-addr.arpa from 
> 127.0.0.1May  5 19:00:00 dnsmasq[4395]: DHCP 192.168.2.141 is ** unprintable>**May  5 19:00:00 dnsmasq[4395]: forwarded 
> 141.2.168.192.in-addr.arpa to 1.0.0.1
>
> The final immediate "forwarded" line comes from dnsmasq itself and
> confirms that this was triggered by use-stale-cache.
>
> Best,
> Dominik
>
> P.S.: The patch recently sent by Erik Karlsson doesn't fix this, it
> touches other code.
> ___
> Dnsmasq-discuss mailing list
> Dnsmasq-discuss@lists.thekelleys.org.uk
> https://lists.thekelleys.org.uk/cgi-bin/mailman/listinfo/dnsmasq-discuss
>
___
Dnsmasq-discuss mailing list
Dnsmasq-discuss@lists.thekelleys.org.uk
https://lists.thekelleys.org.uk/cgi-bin/mailman/listinfo/dnsmasq-discuss


[Dnsmasq-discuss] Use-after-free with DHCP + use-stale-cache

2024-05-05 Thread Dominik Derigs via Dnsmasq-discuss

Hey Simon,

we found a bug resulting in a use-after-free returning garbage data and 
possibly crash when using DHCP + stale cache data.


The bug is triggered when using DHCP and a lease expires. It's name is 
then free'd in kill_name() + do_script_run(). When the PTR record is 
queried thereafter and use-stale-cache is enabled, dnsmasq accesses this 
dangling pointer and returns random data - often a string containing a 
few control characters, once dnsmasq even SEGFAULTed.


Related dnsmasq.log:

|May 5 19:00:00 dnsmasq[4395]: query[PTR] 141.2.168.192.in-addr.arpa 
from 127.0.0.1 May 5 19:00:00 dnsmasq[4395]: DHCP 192.168.2.141 is 
 May 5 19:00:00 dnsmasq[4395]: forwarded 
141.2.168.192.in-addr.arpa to 1.0.0.1|


The final immediate "forwarded" line comes from dnsmasq itself and 
confirms that this was triggered by use-stale-cache.


Best,
Dominik

P.S.: The patch recently sent by Erik Karlsson doesn't fix this, it 
touches other code.
___
Dnsmasq-discuss mailing list
Dnsmasq-discuss@lists.thekelleys.org.uk
https://lists.thekelleys.org.uk/cgi-bin/mailman/listinfo/dnsmasq-discuss