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.abbarapu@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) {