[Dnsmasq-discuss] [PATCH] Re: 2.86rc1

[Simon Kelley]

On 30/09/2021 23:49, Petr Menšík wrote:
> Thanks!
> 
> I were checking it a bit on test build and found part of file
> 0013-Fix-coverity-issues-detected-in-domain-match.c.patch avoided
> application. domain-match.c:447 still has add_resource_record return
> value unchecked, unlike A record above.
> 
> Btw, return values shadows truncp pointer. If add_resource_record ever
> returns 0, truncp would always be set to 1. It seems trunc =
> !add_resource_record() || trunc; Would give the same result without
> passing it extra time as a pointer. Perhaps it should set { trunc = 1;
> break; } on !add_resource_record() and not return 0 as I proposed first.
> 

The intention of add_resource_record() (and the way it's used everywhere
else) is that you can just keep calling it, and if any record goes over
the packet limit, it will set *trunc, stop adding records, and return 0
on subsequent calls. The only thing you have to do is not bump the
section record counter if it returns zero, and set the trunc flag if
*trunc is 1 by the end.

This code fails to do the first, and therefore breaks.


https://thekelleys.org.uk/gitweb/?p=dnsmasq.git;a=commit;h=d2ad5dc073aaacaf22b117f16106282a73586803

does it right.


Cheers,


Simon.

> Attached alternative fix, which works better and were actually tested by:
> 
> for F in {1..40}; do echo "--address=/test/127.0.0.$F"; done | xargs
> src/dnsmasq -d --port 2053 --conf-file=/dev/null --log-queries
> 
> for F in {1..20}; do echo "--address=/test/::$F"; done | xargs
> src/dnsmasq -d --port 2053 --conf-file=/dev/null --log-queries
> 
> Both create truncated responses to test a/aaaa queries. Current state
> would not reply on A truncated query at all.
> 
> On 9/12/21 00:05, Simon Kelley wrote:
>> Applied in 2.87.
>>
>> Cheers,
>>
>> Simon.
>>
>>
>>
>> On 03/09/2021 22:47, Petr Menšík wrote:
>>> Hi Simon,
>>>
>>> I have prepared a set of patches applied over 2.86rc3 release. They were
>>> made to silent some of reports from Coverity scans we do for our
>>> packages. I did include reported parts in commit messages, so commit
>>> messages are somehow noisy and contain more bytes that the diffs itself.
>>>
>>> It should add few safety checks on multiple places. Fix few error paths
>>> not releasing allocated memory and retries in case of failed syscall. It
>>> is not perfect, but should be good enough.
>>>
>>> Not heavily tested, compiles without issues or warnings and reduced
>>> reported issues. Review would be appreciated.
>>>
>>> What do you think, can they still be merged?
>>>
>>> Cheers,
>>>
>>> Petr
>>>
>>> On 8/25/21 3:46 PM, Simon Kelley wrote:
>>>> I just pushed a few final changes, tagged as dnsmasq-2.86rc1.
>>>>
>>>> I'm fairly confident that this can be released as 2.86 in the near
>>>> future, but if you can, please test it now, to avoid disappointment later.
>>>>
>>>> https://thekelleys.org.uk/dnsmasq/release-candidates/dnsmasq-2.86rc1.tar.gz
>>>>
>>>> Cheers,
>>>>
>>>> Simon.
>>>>
>>>>
>>>> _______________________________________________
>>>> Dnsmasq-discuss mailing list
>>>> Dnsmasq-discuss at lists.thekelleys.org.uk
>>>> https://lists.thekelleys.org.uk/cgi-bin/mailman/listinfo/dnsmasq-discuss
>>>>
>>>>
>>>> _______________________________________________
>>>> Dnsmasq-discuss mailing list
>>>> Dnsmasq-discuss at lists.thekelleys.org.uk
>>>> https://lists.thekelleys.org.uk/cgi-bin/mailman/listinfo/dnsmasq-discuss
>>
>> _______________________________________________
>> Dnsmasq-discuss mailing list
>> Dnsmasq-discuss at lists.thekelleys.org.uk
>> https://lists.thekelleys.org.uk/cgi-bin/mailman/listinfo/dnsmasq-discuss
> 
> 
> _______________________________________________
> Dnsmasq-discuss mailing list
> Dnsmasq-discuss at lists.thekelleys.org.uk
> https://lists.thekelleys.org.uk/cgi-bin/mailman/listinfo/dnsmasq-discuss
>