Skip to content
This repository was archived by the owner on Mar 26, 2026. It is now read-only.

Commit 748c256

Browse files
purdeaandreiwhitequark
authored andcommitted
Fix race conditions in dfu examples
I have tested boot-dfu downloading firmware with dfu-util, while executing the following in the background, as root: `while true; do lsusb -v -d 04b4:8613 > /dev/null; done` With old version of boot-dfu this results in corrupted data. With the new version of boot-dfu this works correctly. I had to allocate an additonal scratch2 buffer. We cannot use the normal scratch buffer, because it's used when getting descriptors, and the OS may send GET_DESCRIPTOR requests at any time. I have also tested the new version of boot-dfu-spiflash, but only flashing the eeprom. I have also tested the new version of boot-uf2-dfu in dfu-mode only.
1 parent 6694d9a commit 748c256

4 files changed

Lines changed: 20 additions & 5 deletions

File tree

examples/boot-dfu-spiflash/Makefile

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,5 +2,8 @@ TARGET = boot-dfu-spiflash
22
LIBRARIES = fx2 fx2usb fx2dfu fx2isrs
33
MODEL = medium
44

5+
CODE_SIZE ?= 0x3c00
6+
XRAM_SIZE ?= 0x0400
7+
58
LIBFX2 = ../../firmware/library
69
include $(LIBFX2)/fx2rules.mk

examples/boot-uf2-dfu/Makefile

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,5 +2,8 @@ TARGET = boot-uf2-dfu
22
LIBRARIES = fx2 fx2usb fx2dfu fx2usbmassstor fx2uf2 fx2isrs
33
MODEL = medium
44

5+
CODE_SIZE ?= 0x3c00
6+
XRAM_SIZE ?= 0x0400
7+
58
LIBFX2 = ../../firmware/library
69
include $(LIBFX2)/fx2rules.mk

firmware/boot-dfu/Makefile

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,5 +2,8 @@ TARGET = boot-dfu
22
LIBRARIES = fx2 fx2usb fx2dfu fx2isrs
33
MODEL = small
44

5+
CODE_SIZE ?= 0x3c00
6+
XRAM_SIZE ?= 0x0400
7+
58
LIBFX2 = ../library
69
include $(LIBFX2)/fx2rules.mk

firmware/library/usbdfu.c

Lines changed: 11 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,9 @@
33

44
#pragma save
55
#pragma nooverlay
6+
7+
__xdata uint8_t scratch2[512];
8+
69
bool usb_dfu_setup(usb_dfu_iface_state_t *dfu, __xdata struct usb_req_setup *req) {
710
uint8_t interface = dfu->state > USB_DFU_STATE_appDETACH ? 0 : dfu->interface;
811

@@ -38,7 +41,9 @@ bool usb_dfu_setup(usb_dfu_iface_state_t *dfu, __xdata struct usb_req_setup *req
3841
!dfu->sync) {
3942
// If we're here, then EP0BUF is still in use, but the host already sent GETSTATUS.
4043
// If we do SETUP_EP0_* right now, we'll overwrite EP0BUF, and get corrupted data.
41-
// So, delay responding to this packet until after EP0BUF is copied to scratch space.
44+
// So, delay responding to this packet until after EP0BUF is copied a second scratch
45+
// space. We cannot use the normal scratch space, because it's used when getting
46+
// descriptors, and our downloaded data might get overwritten.
4247
//
4348
// (GETSTATUS in dfuMANIFEST-SYNC does not have this restriction, but these requests
4449
// use identical flows in the DFU spec, and it is simpler to handle them the same way.)
@@ -93,14 +98,14 @@ bool usb_dfu_setup(usb_dfu_iface_state_t *dfu, __xdata struct usb_req_setup *req
9398
dfu->length = req->wLength;
9499
dfu->pending = true;
95100
dfu->sync = false;
96-
SETUP_EP0_BUF(0);
101+
SETUP_EP0_OUT_BUF();
97102
return true;
98103
} else if(dfu->state == USB_DFU_STATE_dfuDNLOAD_IDLE && req->wLength > 0) {
99104
dfu->state = USB_DFU_STATE_dfuDNLOAD_SYNC;
100105
dfu->length = req->wLength;
101106
dfu->pending = true;
102107
dfu->sync = false;
103-
SETUP_EP0_BUF(0);
108+
SETUP_EP0_OUT_BUF();
104109
return true;
105110
} else if(dfu->state == USB_DFU_STATE_dfuDNLOAD_IDLE) {
106111
dfu->state = USB_DFU_STATE_dfuMANIFEST_SYNC;
@@ -149,15 +154,16 @@ void usb_dfu_setup_deferred(usb_dfu_iface_state_t *dfu) {
149154
}
150155
} else if(dfu->state == USB_DFU_STATE_dfuDNLOAD_SYNC) {
151156
while(EP0CS & _BUSY);
152-
xmemcpy(scratch, &EP0BUF[0], dfu->length);
157+
xmemcpy(scratch2, &EP0BUF[0], dfu->length);
158+
ACK_EP0();
153159

154160
// Wait until we get a GETSTATUS request (in case we still haven't got one), and then reply
155161
// to it from here, after we've safely stashed away EP0BUF contents.
156162
while(!dfu->sync);
157163
usb_dfu_setup(dfu, (__xdata struct usb_req_setup *)SETUPDAT);
158164
return;
159165
} else if(dfu->state == USB_DFU_STATE_dfuDNBUSY) {
160-
dfu->status = dfu->firmware_dnload(dfu->offset, scratch, dfu->length);
166+
dfu->status = dfu->firmware_dnload(dfu->offset, scratch2, dfu->length);
161167
if(dfu->status == USB_DFU_STATUS_OK) {
162168
dfu->offset += dfu->length;
163169
dfu->state = USB_DFU_STATE_dfuDNLOAD_IDLE;

0 commit comments

Comments
 (0)