patchlwIP - A Lightweight TCP/IP stack - Patches: patch #9166, Fix oldest entry lookup in...

 
 

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

patch #9166: Fix oldest entry lookup in dns_enqueue

Submitter:  Giuseppe Andreello <andreello>
Submitted:  Fri 18 Nov 2016 07:13:42 PM UTC
   
 
Category:  DNS Priority:  5 - Normal
Status:  Duplicate Privacy:  Public
Assigned to:  goldsimon Open/Closed:  Closed
Planned Release:  None

Wed 23 Nov 2016 08:06:32 AM UTC, comment #4: 


> I did not modify the dns_can_recycle_entry() function.


That function does not exist in original lwIP (and never existed). That means you have some 3rd party version, which I don't want to support (and this is why your patch doesn't apply).

As to your bug report: that bug has already been fixed back in 2014 (patch #8480) by casting the result of the subtraction to u8_t.

Simon Goldschmidt <goldsimon>
Group administrator
Wed 23 Nov 2016 12:58:25 AM UTC, comment #3: 

Hi Simon,

I did not modify the dns_can_recycle_entry() function.

My understanding is that the following boolean is always false after a wrap-around:

(dns_seqno - pEntry->seqno) > lseq


As I understand, this happens because:
1. in C, all operands get promoted to int (if the result of the operation can fit; larger types otherwise)
2. the result of the difference is negative
3. lseq is initialized to 0

I'm running this on a 64bit linux system using gcc.

Could you test the following example with your compiler?

#include <stdio.h>
#include <inttypes.h>

int main(const char *argv, int argc)
{  
    printf("Hello world!\n");

    uint8_t a = 5;
    uint8_t b = 7;
    uint8_t c = 0;
    uint8_t u8diff;
    int intdiff;

    if ((a - b) > c)
    {  
        printf("(5-7) > 0\n");
    }
    else
    {  
        printf("(5-7) <= 0\n");
    }

    u8diff = (uint8_t)(a-b);
    intdiff = (int)(a-b);

    printf("u8diff: %"PRIu8"\n", u8diff);
    printf("intdiff: %d\n", intdiff);

    return 0;
}


I get the output below. As you can see, the "if" statement behaves as if the operands are converted to int.


$ gcc example.c && ./a.out
Hello world!
(5-7) <= 0
u8diff: 254
intdiff: -2

I have gcc 4.8.4 on linux x86_64

Giuseppe Andreello <andreello>
Tue 22 Nov 2016 08:25:40 PM UTC, comment #2: 

Plus, I've tested it (without your patch; in win32 debugger) and it works for me:

dns_seqno = 2
DNS_TABLE_SIZE = 4
dns_table[*all*].state = DNS_STATE_DONE
dns_table[0].seqno = 1
dns_table[1].seqno = 5
dns_table[2].seqno = 6
dns_table[3].seqno = 7

-> next overwritten entry is i=1. Seems perfectly right.

Please provide more info why this should be wrong.

Simon Goldschmidt <goldsimon>
Group administrator
Tue 22 Nov 2016 08:07:55 PM UTC, comment #1: 

This is one very strange diff you attached: what's this dns_can_recycle_entry() function?

Simon Goldschmidt <goldsimon>
Group administrator
Fri 18 Nov 2016 07:13:42 PM UTC, original submission:  

If dns_seqno wraps around, dns_enqueue does not compute the age of the entries correctly, and can return ERR_MEM even if there are recyclable entries.

Giuseppe Andreello <andreello>

 

(Note: upload size limit is set to 16384 kB, after insertion of the required escape characters.)

Attached Files
file #39013:  dns_enqueue.diff added by andreello (1KiB - application/octet-stream)

 

Depends on the following items: None found

Items that depend on this one: None found

 

Carbon-Copy List
  • -email is unavailable- added by goldsimon (Posted a comment)
  • -email is unavailable- added by andreello (Submitted the item)
  •  

    There are 0 votes so far. Votes easily highlight which items people would like to see resolved in priority, independently of the priority of the item set by tracker managers.

     

    Follow 5 latest changes.

    Date Changed by Updated Field Previous Value => Replaced by
    2016-11-23 goldsimon StatusWorks For Me Duplicate
        Open/ClosedOpen Closed
    2016-11-22 goldsimon StatusNone Works For Me
        Assigned toNone goldsimon
    2016-11-18 andreello Attached File- Added dns_enqueue.diff, #39013

    Back to the top

    Powered by Savane 3.13-02a9.
    Corresponding source code