weird code (bug?) in IP options handling

weird code (bug?) in IP options handling

[Nadav Har'El]

While browsing the ip-options setting code in Linux 2.4-18, I came across
the following peculiar code in net/ipv4/ip_options.c in ip_options_compile.
The code is apparently used when a setsockopt(fd, SOL_IP, IP_OPTIONS,...)
is called and one of the options is IPOPT_END (0):

        for (l = opt->optlen; l > 0; ) {
                switch (*optptr) {
                      case IPOPT_END:
                        for (optptr++, l--; l>0; l--) {
                                if (*optptr != IPOPT_END) {
                                        *optptr = IPOPT_END;
                                        opt->is_changed = 1;
                                }
                        }
                        goto eol;
	...

I am *guessing* that this piece of code was meant to zero (assign as
IPOPT_END) all option bytes after the first IPOPT_END. However, this is
not what actually happens in this code. In this code, only a single
byte after the first IPOPT_END (optptr++) is zeroed, and this same
assignment is done many times in a loop! This can't have been intended :)

I am guessing that this is just a programming error, and the programmer
perhaps intended to make the assignment in the inner for loop as

	*optptr = IPOPT_END;

Who's in charge for this code and can perhaps take a look at it?

By the way, I wonder: why zero the options at all? Why not simply leave
all option bytes after the first IPOPT_END as-is?

-- 
Nadav Har'El                        |    Thursday, Sep 19 2002, 14 Tishri 5763
nyh@math.technion.ac.il             |-----------------------------------------
Phone: +972-53-245868, ICQ 13349191 |If you tell the truth, you don't have to
http://nadav.harel.org.il           |remember anything.

Re: weird code (bug?) in IP options handling

[Nadav Har'El]

On Thu, Sep 19, 2002, Nadav Har'El wrote about "weird code (bug?) in IP options handling":
> I am guessing that this is just a programming error, and the programmer
> perhaps intended to make the assignment in the inner for loop as
> 
> 	*optptr = IPOPT_END;

Oops, I meant something like

 	*optptr++ = IPOPT_END;

-- 
Nadav Har'El                        |    Thursday, Sep 19 2002, 14 Tishri 5763
nyh@math.technion.ac.il             |-----------------------------------------
Phone: +972-53-245868, ICQ 13349191 |"God is dead." - Nietzsche; "Nietzsche is
http://nadav.harel.org.il           |dead" - God