Skip to contentSkip to conversation

ipconfig accepts any DHCPACK

PATCH In review Peter Buckingham 3 participants 6 messages Sep 7, 2004 – Sep 8, 2004

Subsystems (3)
Networking Network drivers IPv4/IPv6
lore
Share & tools
lore

Peter Buckingham

author
#0 lore raw

PATCH 1/1 ipconfig accepts any DHCPACK

Hi David,

I was playing around with using ipconfig to initialise a bunch of systems and it turns out that the ipconfig code doesn't check to see whether the ACK is for it or another system. I've just added a simple check to compare the hardware address of the device to the one in the packet.

peter

Signed-off-by Peter Buckingham <[email protected]>
net/ipv4/ipconfig.c +5 -0
@@ -966,6 +966,12 @@ static int __init ic_bootp_recv(struct s
 				break;
 
 			case DHCPACK:
+				for (i = 0; (dev->dev_addr[i] == b->hw_addr[i])
+						&& (i < 16); i++);
+				if (i < 16)
+					goto drop_unlock;
+
 				/* Yeah! */
 				break;
#1 lore raw
Quoted 5 lines · On Tue, 07 Sep 2004 14:46:14 -0700 Peter Buckingham <[email protected]> wrote:

I was playing around with using ipconfig to initialise a bunch of systems and it turns out that the ipconfig code doesn't check to see whether the ACK is for it or another system. I've just added a simple check to compare the hardware address of the device to the one in the packet.

Just because there are 16 bytes of hw address in the bootp packet layout doesn't mean the device actually has that many. Please fix it to use dev->addr_len.

author
#2 lore raw
Quoted 4 lines · David S. Miller

Just because there are 16 bytes of hw address in the bootp packet layout doesn't mean the device actually has that many. Please fix it to use dev->addr_len.

is this okay?

peter

Signed-off-by Peter Buckingham <[email protected]>
net/ipv4/ipconfig.c +5 -0
@@ -966,6 +966,11 @@ static int __init ic_bootp_recv(struct s
 				break;
 
 			case DHCPACK:
+				for (i = 0; (dev->dev_addr[i] == b->hw_addr[i])
+						&& (i < dev->addr_len); i++);
+				if (i < dev->addr_len)
+					goto drop_unlock;
+
 				/* Yeah! */
 				break;
#3 lore raw
Quoted 7 lines · On Tue, 07 Sep 2004 15:45:06 -0700 Peter Buckingham <[email protected]> wrote:
Quoted 4 lines · David S. Miller

Just because there are 16 bytes of hw address in the bootp packet layout doesn't mean the device actually has that many. Please fix it to use dev->addr_len.

is this okay?

yep, looks great, I'll apply it

#4 lore raw
In reply to Peter Buckingham

net/ipv4/ipconfig.c static int __init ic_bootp_recv(struct s · line 973

Quoted 6 lines · Peter Buckingham

is this okay?

net/ipv4/ipconfig.c +5 -0
+				for (i = 0; (dev->dev_addr[i] == b->hw_addr[i])
+						&& (i < dev->addr_len); i++);
+				if (i < dev->addr_len)
+					goto drop_unlock;
+

I had to read that twice.

How about something like:

	for (i=0;i<dev->addr_len;i++)
		if (dev->dev_addr[i] != b->hw_addr[i])
			goto drop_unlock;

or

	if (memcmp(dev->dev_addr, b->hw_addr, dev->addr_len)!=0)
		goto drop_unlock;

instead?

author
#5 lore raw
Quoted 3 lines · Joe Perches
	if (memcmp(dev->dev_addr, b->hw_addr, dev->addr_len)!=0)
		goto drop_unlock;

I prefer this one.

Signed-off-by Peter Buckingham <[email protected]>
net/ipv4/ipconfig.c +3 -0
@@ -966,6 +966,9 @@ static int __init ic_bootp_recv(struct s
 				break;
 
 			case DHCPACK:
+				if (memcmp(dev->dev_addr, b->hw_addr, dev->addr_len) != 0)
+					goto drop_unlock;
+
 				/* Yeah! */
 				break;