DFU implementation does not bound the length field in USB
DFU download setup packets, and it does not verify that
the transfer direction. Fixing the length and transfer
direction.
CVE-2022-2347
Signed-off-by: Venkatesh Yadav Abbarapu <venkatesh.abbar...@amd.com>
---
drivers/usb/gadget/f_dfu.c | 58 ++++++++++++++++++++++++--------------
1 file changed, 37 insertions(+), 21 deletions(-)
diff --git a/drivers/usb/gadget/f_dfu.c b/drivers/usb/gadget/f_dfu.c
index e9340ff5cb..33ef62f8ba 100644
--- a/drivers/usb/gadget/f_dfu.c
+++ b/drivers/usb/gadget/f_dfu.c
@@ -321,23 +321,29 @@ static int state_dfu_idle(struct f_dfu *f_dfu,
u16 len = le16_to_cpu(ctrl->wLength);
int value = 0;
+ len = len > DFU_USB_BUFSIZ ? DFU_USB_BUFSIZ : len;
+
switch (ctrl->bRequest) {
case USB_REQ_DFU_DNLOAD:
- if (len == 0) {
- f_dfu->dfu_state = DFU_STATE_dfuERROR;
- value = RET_STALL;
- break;
+ if (ctrl->bRequestType == USB_DIR_OUT) {
+ if (len == 0) {
+ f_dfu->dfu_state = DFU_STATE_dfuERROR;
+ value = RET_STALL;
+ break;
+ }
+ f_dfu->dfu_state = DFU_STATE_dfuDNLOAD_SYNC;
+ f_dfu->blk_seq_num = w_value;
+ value = handle_dnload(gadget, len);
}
- f_dfu->dfu_state = DFU_STATE_dfuDNLOAD_SYNC;
- f_dfu->blk_seq_num = w_value;
- value = handle_dnload(gadget, len);
break;
case USB_REQ_DFU_UPLOAD:
- f_dfu->dfu_state = DFU_STATE_dfuUPLOAD_IDLE;
- f_dfu->blk_seq_num = 0;
- value = handle_upload(req, len);
- if (value >= 0 && value < len)
- f_dfu->dfu_state = DFU_STATE_dfuIDLE;
+ if (ctrl->bRequestType == USB_DIR_IN) {
+ f_dfu->dfu_state = DFU_STATE_dfuUPLOAD_IDLE;
+ f_dfu->blk_seq_num = 0;
+ value = handle_upload(req, len);
+ if (value >= 0 && value < len)
+ f_dfu->dfu_state = DFU_STATE_dfuIDLE;
+ }
break;
case USB_REQ_DFU_ABORT:
/* no zlp? */
@@ -426,11 +432,15 @@ static int state_dfu_dnload_idle(struct f_dfu *f_dfu,
u16 len = le16_to_cpu(ctrl->wLength);
int value = 0;
+ len = len > DFU_USB_BUFSIZ ? DFU_USB_BUFSIZ : len;
+
switch (ctrl->bRequest) {
case USB_REQ_DFU_DNLOAD:
- f_dfu->dfu_state = DFU_STATE_dfuDNLOAD_SYNC;
- f_dfu->blk_seq_num = w_value;
- value = handle_dnload(gadget, len);
+ if (ctrl->bRequestType == USB_DIR_OUT) {
+ f_dfu->dfu_state = DFU_STATE_dfuDNLOAD_SYNC;
+ f_dfu->blk_seq_num = w_value;
+ value = handle_dnload(gadget, len);
+ }
break;
case USB_REQ_DFU_ABORT:
f_dfu->dfu_state = DFU_STATE_dfuIDLE;
@@ -513,13 +523,17 @@ static int state_dfu_upload_idle(struct f_dfu *f_dfu,
u16 len = le16_to_cpu(ctrl->wLength);
int value = 0;
+ len = len > DFU_USB_BUFSIZ ? DFU_USB_BUFSIZ : len;
+
switch (ctrl->bRequest) {
case USB_REQ_DFU_UPLOAD:
- /* state transition if less data then requested */
- f_dfu->blk_seq_num = w_value;
- value = handle_upload(req, len);
- if (value >= 0 && value < len)
- f_dfu->dfu_state = DFU_STATE_dfuIDLE;
+ if (ctrl->bRequestType == USB_DIR_IN) {
+ /* state transition if less data then requested */
+ f_dfu->blk_seq_num = w_value;
+ value = handle_upload(req, len);
+ if (value >= 0 && value < len)
+ f_dfu->dfu_state = DFU_STATE_dfuIDLE;
+ }
break;
case USB_REQ_DFU_ABORT:
f_dfu->dfu_state = DFU_STATE_dfuIDLE;
@@ -595,6 +609,8 @@ dfu_handle(struct usb_function *f, const struct
usb_ctrlrequest *ctrl)
int value = 0;
u8 req_type = ctrl->bRequestType & USB_TYPE_MASK;
+ len = len > DFU_USB_BUFSIZ ? DFU_USB_BUFSIZ : len;
+
debug("w_value: 0x%x len: 0x%x\n", w_value, len);
debug("req_type: 0x%x ctrl->bRequest: 0x%x f_dfu->dfu_state: 0x%x\n",
req_type, ctrl->bRequest, f_dfu->dfu_state);
@@ -614,7 +630,7 @@ dfu_handle(struct usb_function *f, const struct
usb_ctrlrequest *ctrl)
value = dfu_state[f_dfu->dfu_state] (f_dfu, ctrl, gadget, req);
if (value >= 0) {
- req->length = value;
+ req->length = value > DFU_USB_BUFSIZ ? DFU_USB_BUFSIZ : value;
req->zero = value < len;
value = usb_ep_queue(gadget->ep0, req, 0);
if (value < 0) {
--
2.17.1
