[Dnsmasq-discuss] [PATCH] Various fixes detected by static analysis

[Petr Mensik]

Hi Simon and all others,

I have tried running dnsmasq under coverity, static analysis tool. It
found some warnings. I have fixed some things. Most obvious error was
inconsistent handling of buffer length of interface names. Buffer size
is IFNAMSIZ long, that is 16 bytes. But if interface should have
terminating zero, max. useable length is 15. Sometimes, buffer size is
16+1, sometimes only 16. Sometimes name might be sent to the kernel
unterminated. According to [1] it cannot be longer in Linux.

I have created shared simple function that will always terminate string.
And few little improvements around. What do you think?

It complains a lot about returns from one_opt in option.c. I can make
patch that will deallocate memory before returning error. In some cases,
i think option parsing does not have to be fatal. Would you accept fixes
that will free memory before return? I think in some cases option
parsing can be used for files that can be reread multiple times when
running.

1.
https://git.kernel.org/pub/scm/linux/kernel/git/stable/linux.git/diff/?id=f236da93df5be85409c24f03683e3d8c54fac72b

[Simon Kelley]

Post by Petr Mensik
Hi Simon and all others,
I have tried running dnsmasq under coverity, static analysis tool. It
found some warnings. I have fixed some things. Most obvious error was
inconsistent handling of buffer length of interface names. Buffer size
is IFNAMSIZ long, that is 16 bytes. But if interface should have
terminating zero, max. useable length is 15. Sometimes, buffer size is
16+1, sometimes only 16. Sometimes name might be sent to the kernel
unterminated. According to [1] it cannot be longer in Linux.
I have created shared simple function that will always terminate string.
And few little improvements around. What do you think?

Looks good on first look. I'll find time to go through it in more detail
and apply it soon.

Post by Petr Mensik
It complains a lot about returns from one_opt in option.c. I can make
patch that will deallocate memory before returning error. In some cases,
i think option parsing does not have to be fatal. Would you accept fixes
that will free memory before return? I think in some cases option
parsing can be used for files that can be reread multiple times when
running.

The only code where an error is not fatal is for dhcp-option, dhcp-host,
server and rev-server which get called when files containing just those
options are read dynamically. That means parse_dhcp_opt() and code
starting at 3025, 2401 and 2493
I suspect there are therefore memory leaks in those which might be worth
fixing. For the vast majority of errors in one_opt, they are fatal, and
complicating the code massively to avoid warnings from coverity is
probably not a good move.


Cheers,

Simon.

Post by Petr Mensik
1.
https://git.kernel.org/pub/scm/linux/kernel/git/stable/linux.git/diff/?id=f236da93df5be85409c24f03683e3d8c54fac72b
_______________________________________________
Dnsmasq-discuss mailing list
http://lists.thekelleys.org.uk/mailman/listinfo/dnsmasq-discuss