Re: [net-next PATCH v4 3/3] net: reserve ports for applications usingfixed port numbers

From: Cong Wang
Date: Tue Feb 16 2010 - 08:03:17 EST


Octavian Purdila wrote:
On Tuesday 16 February 2010 11:37:04 you wrote:
BUILD_BUG_ON(sizeof(struct inet_skb_parm) > sizeof(dummy_skb->cb));

+ sysctl_local_reserved_ports = kzalloc(65536 / 8, GFP_KERNEL);
+ if (!sysctl_local_reserved_ports)
+ goto out;
+
I think we should also consider the ports in ip_local_port_range,
since we can only reserve the ports in that range.


That is subject to changes at runtime, which means we will have to readjust the bitmap at runtime which introduces the need for additional synchronization operations which I would rather avoid.

Why? As long as the bitmap is global, this will not be hard.

Consider that if one user writes a port number which is beyond
the ip_local_port_range into ip_local_reserved_ports, we should
not accept this, because it doesn't make any sense. But with your
patch, we do.



+ {
+ .procname = "ip_local_reserved_ports",
+ .data = NULL, /* initialized in sysctl_ipv4_init */
+ .maxlen = 65536,
+ .mode = 0644,
+ .proc_handler = proc_dobitmap,
+ },
Isn't there an off-by-one here?

In patch 2/3, you use 0 to set the fist bit, then how about 65535 which
writes 65536th bit? This is beyond the range of port number.


This seems fine to me, 65535 is the value used by both the port checking function and the proc read/write function. And it translates indeed to 65536th bit, but that is also bit 65535 if you start counting bits from 0 instead of 1. The usual computing/natural arithmetic confusion for the meaning of first :)


Oh, I see.

Thanks.

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/