patchAVR Downloader/UploaDEr - Patches: patch #9732, usbtiny_paged_load overflows...

 
 

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

patch #9732: usbtiny_paged_load overflows buffer e.g. when reading EEPROM

Submitter:  Joel Ray Holveck <joelh>
Submitted:  Sat 08 Dec 2018 04:10:04 AM UTC
   
 
Category:  None Priority:  5 - Normal
Status:  Done Privacy:  Public
Assigned to:  joerg_wunsch Open/Closed:  Closed

Fri 18 Sep 2020 09:16:53 PM UTC, comment #1: 

Applied in r1444 (also to usbtiny_paged_write), thanks!

Joerg Wunsch <joerg_wunsch>
Group administrator
Sat 08 Dec 2018 04:10:04 AM UTC, original submission:  

usbtiny_paged_load overflows buffer

If the n_bytes given to usbtiny_paged_load is not a multiple of the programmer's chunk size (for instance, if n_bytes is less than the chunk size), usbtiny will overwrite the end of the buffer.

If I try to read the EEPROM of my ATtiny84A over a USBtinyISP, I get a segfault.  If I set the t84 eeprom page_size parameter in avrdude.conf to 0 (force a byte-by-byte read), it succeeds.

The page_size parameter is normally 4, but usbtiny_paged_load still tries to load 128 bytes for each 4-byte page.

The command line I'm using is:
avrdude -C /path/to/avrdude.conf -p t84 -c usbtiny -U eeprom:r:eedump.hex:i

It runs for 14 seconds before segfaulting.  The progress bar runs:
Reading | #############################################      | 89% 14.02sSegmentation fault (core dumped)

If I apply the enclosed patch, or as a workaround set t84's eeprom page_size to 0 in avrdude.conf (line 11183 in the 6.3 sources' avrdude.conf.in), I get a reasonable EEPROM dump in less than one second.

I did a little debugging, and note that we're reading 128 byte chunks even once we're reading near the end of the EEPROM (see usbtiny_paged_load).  The crash happens when we are reading offset 456.  Since the buffer is only 512 bytes, it seems that we may end up trying to write beyond the end of the malloc'd region, scribbling on other data.

Backtrace and the variables I find relevant to that hypothesis:


(gdb) bt
#0  0x00007ffff79ccbe6 in usb_control_msg ()
   from /lib/x86_64-linux-gnu/libusb-0.1.so.4
#1  0x000000000041a794 in usb_in (requestid=requestid@entry=11,
    val=val@entry=0, index=index@entry=456,
    buffer=0x66d618 '\377' <repeats 56 times>, buflen=buflen@entry=128,
    bitclk=320, pgm=0x6d5e90) at usbtiny.c:123
#2  0x000000000041a987 in usbtiny_paged_load (pgm=0x6d5e90, p=<optimized out>,
    m=0x8a0cb0, page_size=<optimized out>, addr=456, n_bytes=4)
    at usbtiny.c:461
#3  0x0000000000407f47 in avr_read (pgm=pgm@entry=0x6d5e90,
    p=p@entry=0x89f7b0, memtype=0x66d200 "eeprom", v=v@entry=0x0) at avr.c:405
#4  0x000000000041b416 in do_op (pgm=0x6d5e90, p=p@entry=0x89f7b0,
    upd=0x66d1d0, flags=flags@entry=UF_NONE) at update.c:236
#5  0x0000000000404328 in main (argc=<optimized out>, argv=<optimized out>)
    at main.c:1227
(gdb) frame 1
#1  0x000000000041a794 in usb_in (requestid=requestid@entry=11,
    val=val@entry=0, index=index@entry=456,
    buffer=0x66d618 '\377' <repeats 56 times>, buflen=buflen@entry=128,
    bitclk=320, pgm=0x6d5e90) at usbtiny.c:123
123         nbytes = usb_control_msg( PDATA(pgm)->usb_handle,
(gdb) p buflen
$5 = 128
(gdb) p buffer
$6 = (unsigned char *) 0x66d618 '\377' <repeats 56 times>
(gdb) frame 3
#3  0x0000000000407f47 in avr_read (pgm=pgm@entry=0x6d5e90,
    p=p@entry=0x89f7b0, memtype=0x66d200 "eeprom", v=v@entry=0x0) at avr.c:405
405             rc = pgm->paged_load(pgm, p, mem, mem->page_size,
(gdb) p *mem
$12 = {desc = "eeprom", '\000' <repeats 57 times>, paged = 0, size = 512,
  page_size = 4, num_pages = 0, offset = 0, min_write_delay = 4000,
  max_write_delay = 4500, pwroff_after_write = 0, readback = "\377\377",
  mode = 65, delay = 6, blocksize = 4, readsize = 256, pollindex = 0,
  buf = 0x66d450 "", tags = 0x66d700 "Ȏ%\367\377\177", op = {0x8a0de0,
    0x8a0f70, 0x0, 0x0, 0x0, 0x0, 0x8a1100, 0x0, 0x0, 0x8a1290, 0x0, 0x0}}


Joel Ray Holveck <joelh>

 

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

Attached Files
file #45595:  usbtiny.diff added by joelh (453B - text/x-patch)

 

Depends on the following items: None found

Items that depend on this one: None found

 

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

    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
    2020-09-18 joerg_wunsch StatusNone Done
        Assigned toNone joerg_wunsch
        Open/ClosedOpen Closed
    2018-12-08 joelh Attached File- Added usbtiny.diff, #45595
        Carbon-Copy- Added -email is unavailable-

    Back to the top

    Powered by Savane 3.13-f8d8.
    Corresponding source code