buglwIP - A Lightweight TCP/IP stack - Bugs: bug #34072, UDP Broadcast is received from...

 
 

You are not allowed to post comments on this tracker with your current authentication level.

bug #34072: UDP Broadcast is received from wrong UDP PCB if udp_port matches

Submitted by:  Andreas Becker <abecker>
Submitted on:  Fri 19 Aug 2011 07:25:35 AM UTC  
 
Category: UDPSeverity: 3 - Normal
Item Group: Faulty BehaviourStatus: Fixed
Privacy: PublicAssigned to: Simon Goldschmidt <goldsimon>
Open/Closed: ClosedPlanned Release: None
lwIP version: CVS Head

(Jump to the original submission Jump to the original submission)

Fri 09 Sep 2011 08:31:16 PM UTC, comment #6:

Fixed as suggested in comment #1, since that works unless we want to support overlapping IP networks (which we don't, currently).

Thanks for reporting!

Simon Goldschmidt <goldsimon>
Project AdministratorIn charge of this item.
Tue 30 Aug 2011 12:29:44 PM UTC, comment #5:

I really dislike overlapping IP networks, and would not want lwIP to become less lightweight in order to support them. If we can fix this in a lightweight way that copes with non-overlapping IP networks I think that is sufficient.

Kieran Mansley <kieranm>
Project Administrator
Mon 22 Aug 2011 07:10:07 AM UTC, comment #4:

Hi Ivan,

i don't expect that the netcmp solution would lead to correct results, if we are using overlapping networks on different netifs (-> Comment #2)

I would prefer this solution ( if (broadcast))

(
(ip_addr_isany(&pcb->local_ip) || /* don't care IP*/
(ip_addr_cmp(&(pcb->local_ip), &(inp->ip_addr)) /* IF Local IP == PCB Local IP*/
)

Andreas Becker <abecker>
Fri 19 Aug 2011 04:08:46 PM UTC, comment #3:

I think the netcmp would yield the same result, as this is used to accept a packet on a netif at IP layer. At the moment they both seem like OK solutions.

One thing to keep in mind, the PCB local IP could be IP_ADDR_ANY, so we need to consider both cases (as with unicast packets).

Ivan Delamer <idelamer>
Project Member
Fri 19 Aug 2011 03:56:29 PM UTC, comment #2:

Hi Ivan,

what do you think is the benefit in
- Comparing ip_current_dest_addr with the hopefully correct Net to the PCB?
over
- Match the PCB IP with the IP from the interface, the packet arrives from.

Example with hierarchical routing

First Netif 192.168.0.1 / 16 PCB IP: 192.168.0.1
Second Netif 192.168.1.1 / 24 PCB IP: 192.168.1.1

When something arrives from second Netif, dest IP is 192.168.1.255.
Then you could get the same bad PCB match.

I think wie could expect that the user uses the same IP at the PCB and the local IP.

Andreas Becker <abecker>
Fri 19 Aug 2011 03:25:21 PM UTC, comment #1:

I suggest adding an ip_addr_netcmp for broadcast packets in udp.c

(line 194)

#if IP_SOF_BROADCAST_RECV
(broadcast && (pcb->so_options & SOF_BROADCAST) &&
ip_addr_netcmp(ipX_2_ip(&pcb->local_ip), ip_current_dest_addr(), &inp->netmask)))))) {
#else /* IP_SOF_BROADCAST_RECV */
(broadcast &&
ip_addr_netcmp(ipX_2_ip(&pcb->local_ip), ip_current_dest_addr(), &inp->netmask)))))) {
#endif /* IP_SOF_BROADCAST_RECV */

Ivan Delamer <idelamer>
Project Member
Fri 19 Aug 2011 07:25:35 AM UTC, original submission:

Hi folks,

i have an issue with udp_input() IPV4 at the PCB and IP Matching.

I use two netifs with two UDP PCBs. If I receive a broadcast packet on one Interface, I want to send this to the other Interface, same UDP Ports.
But at the moment I'll receive the packet with the wrong UDP-pcb. Every time it receives arrives with the same UDP PCB.

For example
Netif1: ip_addr: 192.168.0.1/24
PCB 1: local_ip 192.168.0.1 : 1000
Netif2: ip_addr: 192.168.1.1/24
PCB 2: local_ip 192.168.1.1 : 1000

We receive a Packet for 192.168.1.255:

udp_input(.)
{
broadcast = ip_addr_isbroadcast(ip_current_dest_addr(), inp);

> Yes it is Broadcast on this Interface.


for (pcb = udp_pcbs; pcb != NULL; pcb = pcb->next) {
local_match = 0;
...

/* compare PCB local addr+port to UDP destination addr+port */
if ((pcb->local_port == dest) &&
??

> PORT match


... /* IPV 6*/

((
((!broadcast && ipX_addr_isany(0, &pcb->local_ip)) ||
ip_addr_cmp(ipX_2_ip(&pcb->local_ip), ip_current_dest_addr()) || #if LWIP_IGMP
ip_addr_ismulticast(ip_current_dest_addr()) || #endif /* LWIP_IGMP */ #if IP_SOF_BROADCAST_RECV
(broadcast && (pcb->so_options & SOF_BROADCAST)))))) { #else /* IP_SOF_BROADCAST_RECV */
(broadcast))))) {

> YES it is Broadcast but will it be PCB1 or PCB2 ?? --> It could be the wrong PCB, depending hwo ist first on the linked list.


#endif /* IP_SOF_BROADCAST_RECV */
local_match = 1;
...
}

So I suggest, in case of Broadcast, we should match the netif->ip_addr with the pcb->local_ip:

( /* In Broadcast*/
#if IP_SOF_BROADCAST_RECV
(broadcast && (pcb->so_options & SOF_BROADCAST)) #else /* IP_SOF_BROADCAST_RECV */
(broadcast)
#endif /* IP_SOF_BROADCAST_RECV */
&& ( ip_addr_isany(&pcb->local_ip) || /* don't care IP*/
ip_addr_cmp(&(pcb->local_ip), &(inp->ip_addr)) /* IF Local IP == PCB Local IP*/

/* In Broadcast*/)

What do You think?

Andreas Becker <abecker>

 

No files currently attached

 

Depends on the following items: None found

Items that depend on this one: None found

 

Carbon-Copy List
  • -unavailable- added by goldsimon (Posted a comment)
  • -unavailable- added by kieranm (Posted a comment)
  • -unavailable- added by idelamer (Posted a comment)
  • -unavailable- added by abecker (Submitted the item)
  •  

    Do you think this task is very important?
    If so, you can click here to add your encouragement to it.
    This task has 0 encouragements so far.

    Only logged-in users can vote.

     

    Please enter the title of George Orwell's famous dystopian book (it's a date):

     

     

    Follow 4 latest changes.

    Date Changed By Updated Field Previous Value => Replaced By
    Fri 09 Sep 2011 08:31:16 PM UTCgoldsimonStatusNone=>Fixed
      Assigned toNone=>goldsimon
      Open/ClosedOpen=>Closed
      Planned Release=>1.4.1

    Back to the top


    Powered by Savane 3.1-cleanup1