PATCH: udhcpd "option domain" multiple values
Gabriel L. Somlo
somlo at cmu.edu
Fri Feb 2 01:23:04 UTC 2007
On Thu, Feb 01, 2007 at 11:08:20PM +0100, Denis Vlasenko wrote:
> You can overflow existing->data[OPT_LEN].
Yes, by exactly 1 :)
> /* add it to an existing option */
...
> if (existing->data[OPT_LEN] + length <= 255) {
> existing->data = xrealloc(existing->data,
> existing->data[OPT_LEN] + length + 3);
> if ((option->flags & TYPE_MASK) == OPTION_STRING) {
> if (existing->data[OPT_LEN] + length >= 255)
> return;
You already know that's never going to be more than 255 (see the if
statement at the beginning of the quote :) What you don't know is
whether the extra space I want to add will make it 256 instead of 255.
So, maybe do this:
> if (existing->data[OPT_LEN] + length + 1>= 255)
^^^^^
> return;
The other choice would be to do this:
> if (existing->data[OPT_LEN] + length <= 254) {
^^^^^^^
> existing->data = xrealloc(existing->data,
> existing->data[OPT_LEN] + length + 3);
> if ((option->flags & TYPE_MASK) == OPTION_STRING) {
and drop the second check entirely. I'd almost vote for this, but then
we'd be depriving non-string options of one potential byte. Makes the
code cleaner, though... I guess you're the decider :)
Otherwise, it compiles cleanly, and looks ok to me. Won't be able to
test it until some 15 hours from now, though, since I'll have to
physically hook up a dhcp client to the box which is in my office...
Cheers,
Gabriel
More information about the busybox
mailing list