[2/6] slirp: Fix assertion failure on rejected DHCP requests

The guest network stack might DHCPREQUEST an address that the slirp built
in dhcp server can't let it have - for example if the guest has an old
leases file from another network configuration.  In this case the dhcp
server should and does reject the request and prepares to send a DHCPNAK
to the client.

However, in this case the daddr variable in bootp_reply() is set to
0.0.0.0.  Shortly afterwards, it unconditionally attempts to pre-insert the
new client address into the ARP table.  This causes an assertion failure in
arp_address_add() because of the 0.0.0.0 address.

According to RFC2131, DHCPNAK messages for clients on the same subnet
must be sent to the broadcast address (S3.2, subpoint 2).

Cc: Jan Kiszka <jan.kiszka@siemens.com>

Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
---
 slirp/bootp.c |    3 ++-
 1 files changed, 2 insertions(+), 1 deletions(-)

On 2012-02-24 01:23, David Gibson wrote:
> The guest network stack might DHCPREQUEST an address that the slirp built
> in dhcp server can't let it have - for example if the guest has an old
> leases file from another network configuration.  In this case the dhcp
> server should and does reject the request and prepares to send a DHCPNAK
> to the client.
> 
> However, in this case the daddr variable in bootp_reply() is set to
> 0.0.0.0.  Shortly afterwards, it unconditionally attempts to pre-insert the
> new client address into the ARP table.  This causes an assertion failure in
> arp_address_add() because of the 0.0.0.0 address.
> 
> According to RFC2131, DHCPNAK messages for clients on the same subnet
> must be sent to the broadcast address (S3.2, subpoint 2).
> 
> Cc: Jan Kiszka <jan.kiszka@siemens.com>
> 
> Signed-off-by: David Gibson <david@gibson.dropbear.id.au>

Thanks, applied to the slirp queue.

Jan

> ---
>  slirp/bootp.c |    3 ++-
>  1 files changed, 2 insertions(+), 1 deletions(-)
> 
> diff --git a/slirp/bootp.c b/slirp/bootp.c
> index efd1fe7..64eac7d 100644
> --- a/slirp/bootp.c
> +++ b/slirp/bootp.c
> @@ -200,7 +200,8 @@ static void bootp_reply(Slirp *slirp, const struct bootp_t *bp)
>              daddr.sin_addr = preq_addr;
>              memcpy(bc->macaddr, client_ethaddr, ETH_ALEN);
>          } else {
> -            daddr.sin_addr.s_addr = 0;
> +            /* DHCPNAKs should be sent to broadcast */
> +            daddr.sin_addr.s_addr = 0xffffffff;
>          }
>      } else {
>          bc = find_addr(slirp, &daddr.sin_addr, bp->bp_hwaddr);

On Mon, Feb 27, 2012 at 02:58:07PM +0100, Jan Kiszka wrote:
> On 2012-02-24 01:23, David Gibson wrote:
> > The guest network stack might DHCPREQUEST an address that the slirp built
> > in dhcp server can't let it have - for example if the guest has an old
> > leases file from another network configuration.  In this case the dhcp
> > server should and does reject the request and prepares to send a DHCPNAK
> > to the client.
> > 
> > However, in this case the daddr variable in bootp_reply() is set to
> > 0.0.0.0.  Shortly afterwards, it unconditionally attempts to pre-insert the
> > new client address into the ARP table.  This causes an assertion failure in
> > arp_address_add() because of the 0.0.0.0 address.
> > 
> > According to RFC2131, DHCPNAK messages for clients on the same subnet
> > must be sent to the broadcast address (S3.2, subpoint 2).
> > 
> > Cc: Jan Kiszka <jan.kiszka@siemens.com>
> > 
> > Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
> 
> Thanks, applied to the slirp queue.

Thanks.