Syslinux - Jul 2015 - [PATCH] Updated udp.c to use real client ip and subnetmask values if on local subnet

from: Jeff Sloan <jeff_sloan at selinc.com>

Based on commit: 9314e330

Setting UseDefaultAddress to TRUE uses invalid StationAddress and 
SubnetMask values. This is in a network with a local TFTP/MTFTP server. If 
the server is local, on the same subnet, UseDefaultAddress is set to false 
and the client ip and subnetmask are loaded, otherwise set 
UseDefaultAddress to TRUE. This is added to core_udp_connect and 
core_udp_sendto.

Signed-off-by: Jeff Sloan <jeff_sloan at selinc.com>
---
Diffstat:
 udp.c |   52 ++++++++++++++++++++++++++++++++++++++++++++++++++--
 1 file changed, 50 insertions(+), 2 deletions(-)

--- udp.c       2015-07-06 07:22:04.000000000 -0700
+++ efi/udp.c   2015-07-08 12:13:48.000000000 -0700
@@ -142,6 +142,7 @@ void core_udp_connect(struct pxe_pvt_ino
     EFI_UDP4_CONFIG_DATA udata;
     EFI_STATUS status;
     EFI_UDP4 *udp;
+    uint32_t tip;
 
     udp = (EFI_UDP4 *)socket->net.efi.binding->this;
 
@@ -150,7 +151,30 @@ void core_udp_connect(struct pxe_pvt_ino
     /* Re-use the existing local port number */
     udata.StationPort = socket->net.efi.localport;
 
-    udata.UseDefaultAddress = TRUE;
+    /**
+     * Determine if this is remote or local subnet to set
+     * UseDefaultAddress. If ip (TFTP/MTFTP server or 
+     * RemoteAddress) is on same subnet as client, 
+     * which is stored in IPInfo.myip, this is local subnet. 
+     */
+    if (0 == IPInfo.myip || 0 == ip) {
+        /* if either address is NULL, enable default addresses */
+        udata.UseDefaultAddress = TRUE;
+    } else {
+        if ((IPInfo.myip & IPInfo.netmask) == (ip & IPInfo.netmask)) {
+            /* On same subnet */
+            udata.UseDefaultAddress = FALSE;
+           /* Set ip and subnet*/
+           tip = IPInfo.myip;
+            memcpy(&udata.StationAddress, &tip, sizeof(tip));
+            tip = IPInfo.netmask;
+            memcpy(&udata.SubnetMask, &tip, sizeof(tip));
+        } else {
+            /*on different subnets*/
+            udata.UseDefaultAddress = TRUE;
+        }
+    }
+
     memcpy(&udata.RemoteAddress, &ip, sizeof(ip));
     udata.RemotePort = port;
     udata.AcceptPromiscuous = TRUE;
@@ -350,6 +374,7 @@ void core_udp_sendto(struct pxe_pvt_inod
     EFI_STATUS status;
     struct efi_binding *b;
     EFI_UDP4 *udp;
+    uint32_t tip;
 
     (void)socket;
 
@@ -372,7 +397,30 @@ void core_udp_sendto(struct pxe_pvt_inod
     /* Re-use the existing local port number */
     udata.StationPort = socket->net.efi.localport;
 
-    udata.UseDefaultAddress = TRUE;
+    /**
+     * Determine if this is remote or local subnet to set
+     * UseDefaultAddress. If ip (TFTP/MTFTP server or 
+     * RemoteAddress) is on same subnet as client, 
+     * which is stored in IPInfo.myip, this is local subnet. 
+     */
+    if (0 == IPInfo.myip || 0 == ip) {
+        /* if either address is NULL, enable default addresses */
+        udata.UseDefaultAddress = TRUE;
+    } else {
+        if ((IPInfo.myip & IPInfo.netmask) == (ip & IPInfo.netmask)) {
+            /* On same subnet */
+            udata.UseDefaultAddress = FALSE;
+           /* Set ip and subnet*/
+           tip = IPInfo.myip;
+            memcpy(&udata.StationAddress, &tip, sizeof(tip));
+            tip = IPInfo.netmask;
+            memcpy(&udata.SubnetMask, &tip, sizeof(tip));
+        } else {
+            /*on different subnets*/
+            udata.UseDefaultAddress = TRUE;
+        }
+    }
+
     memcpy(&udata.RemoteAddress, &ip, sizeof(ip));
     udata.RemotePort = port;
     udata.AcceptPromiscuous = TRUE;

On Wed, Jul 8, 2015 at 3:32 PM,  <jeff_sloan at selinc.com>
wrote:
> from: Jeff Sloan <jeff_sloan at selinc.com>
>
> Based on commit: 9314e330
1) I don't see this commit ID.

2) Did you try commit ID 5186539 without this patch?  If so, what did
you observe?  I believe I asked you to try a more current commit but
never heard a response.

-- 
-Gene

Gene,

I am very sorry. I did try the patch you suggested and had the same issue. 
Then, I grabbed the latest patch on Patrick's git repo. That's the
commit
that I listed. It was posted two days ago.

I just noticed this on Patrick's repo: This branch is 6 commits ahead, 2 
commits behind geneC:master 


Jeff Sloan
Software Engineer - Computing Systems
Schweitzer Engineering Laboratories, Inc.
(509) 336-2098
jeff_sloan at selinc.com



From:   Gene Cumm <gene.cumm at gmail.com>
To:     jeff_sloan at selinc.com, 
Cc:     For discussion of Syslinux and tftp-hpa <syslinux at zytor.com>, 
masotta <masottaus at yahoo.com>
Date:   07/08/2015 03:06 PM
Subject:        Re: [PATCH] Updated udp.c to use real client ip and 
subnetmask values if on local subnet



On Wed, Jul 8, 2015 at 3:32 PM,  <jeff_sloan at selinc.com>
wrote:
> from: Jeff Sloan <jeff_sloan at selinc.com>
>
> Based on commit: 9314e330
1) I don't see this commit ID.

2) Did you try commit ID 5186539 without this patch?  If so, what did
you observe?  I believe I asked you to try a more current commit but
never heard a response.

-- 
-Gene

>>>
 Setting UseDefaultAddress to TRUE uses invalid StationAddress
 and SubnetMask values. This is in a network with a local TFTP/MTFTP server.
 If the server is local, on the same subnet, UseDefaultAddress is set to
 false and the client ip and subnetmask are loaded, otherwise
 set UseDefaultAddress to TRUE. This is added to core_udp_connect and
 core_udp_sendto.

 Signed-off-by: Jeff Sloan
<<<

I'm also interested on this, but I do not quite understand the problem.
Could you please tell us more about this issue (Wireshark)

Best,
Patrick

>>>
 I'm also interested on this, but I do not quite understand the problem.
 Could you please tell us more about this issue (Wireshark)
<<<

Attached is a Wireshark capture using the current commit (43f5efa) from 
Patrick's repo, .Syslinux.efi is downloaded, there is an ARP and then 
nothing hits the wire from the client. The internal failing scenario is 
core_udp_sendto in syslinux calls Udp4Configure BIOS command, (I will grab 
the packet.) BIOS returns an EFI_UNSUPPORTED (EFIERR (3)) error but 
tftp_open still tries to transmit a packet and waits for a reply 
(core_udp_recv). The loop timeout values are long enough that the watchdog 
eventually times out and reboots the client.

In tftp.c (tftp_open) starting at line 240:

sendreq:
    timeout = *timeout_ptr++;
    if (!timeout)
    return;         /* No file available... */
    oldtime = jiffies();

    core_udp_sendto(socket, rrq_packet_buf, rrq_len, url->ip, url->port);

    /* If the WRITE call fails, we let the timeout take care of it... */
wait_pkt:
    for (;;) {
    buf_len = sizeof(reply_packet_buf);

    err = core_udp_recv(socket, reply_packet_buf, &buf_len,
                &src_ip, &src_port);
    if (err) {
        jiffies_t now = jiffies();
        if (now - oldtime >= timeout)
         goto sendreq;
    } else {
        /* Make sure the packet actually came from the server and
           is long enough for a TFTP opcode */
        dprintf("tftp_open: got packet buflen=%d\n", buf_len);
        if ((src_ip == url->ip) && (buf_len >= 2))
        break;
    }


I owe you the exact contents of the failing packets/buffers but I am 
headed out the door to see if a doctor can keep me alive... (I think I 
have bubonic plague.) As soon as I return I will get the info to you but I 
wanted to let you know that I am working on it. I will also set up and 
test a multi-NIC network as well as remote proxy/TFTP.

Current setup: The DHCP server is remote but the proxy and TFTP servers 
are local.

Let me know if there is anything else you need or have any questions.

Thanks!


Jeff Sloan
-------------- next part --------------
A non-text attachment was scrubbed...
Name: UseDefaultAddressTrue.pcapng
Type: application/octet-stream
Size: 227088 bytes
Desc: not available
URL:
<http://www.zytor.com/pipermail/syslinux/attachments/20150709/c6892de4/attachment-0001.obj>