Re: [PATCH] usb: gadget: dfu: Fix the unchecked length field

[Fabio Estevam]

Hi Sultan,

On Tue, Nov 29, 2022 at 4:49 PM Sultan Khan <sultanqa...@gmail.com> wrote:
>
> While I haven't yet gotten around to trying DFU with this patch applied, my
> guess as to the issue would be the checks of the form "if (ctrl->
> bRequestType == USB_DIR_OUT)" or "if (ctrl->bRequestType == USB_DIR_IN)".
> The bRequestType field contains many flag bits other than the direction
> bit. The checks should just check that the USB_DIR_IN bit (0x80) is set or
> not set, rather than checking if the entire ctrl->bRequestType field equals
> some value.

Is your suggestion as below?

--- a/drivers/usb/gadget/f_dfu.c
+++ b/drivers/usb/gadget/f_dfu.c
@@ -325,7 +325,7 @@ static int state_dfu_idle(struct f_dfu *f_dfu,

        switch (ctrl->bRequest) {
        case USB_REQ_DFU_DNLOAD:
-               if (ctrl->bRequestType == USB_DIR_OUT) {
+               if (ctrl->bRequestType & USB_DIR_OUT) {
                        if (len == 0) {
                                f_dfu->dfu_state = DFU_STATE_dfuERROR;
                                value = RET_STALL;
@@ -337,7 +337,7 @@ static int state_dfu_idle(struct f_dfu *f_dfu,
                }
                break;
        case USB_REQ_DFU_UPLOAD:
-               if (ctrl->bRequestType == USB_DIR_IN) {
+               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);
@@ -436,7 +436,7 @@ static int state_dfu_dnload_idle(struct f_dfu *f_dfu,

        switch (ctrl->bRequest) {
        case USB_REQ_DFU_DNLOAD:
-               if (ctrl->bRequestType == USB_DIR_OUT) {
+               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);
@@ -527,7 +527,7 @@ static int state_dfu_upload_idle(struct f_dfu *f_dfu,

        switch (ctrl->bRequest) {
        case USB_REQ_DFU_UPLOAD:
-               if (ctrl->bRequestType == USB_DIR_IN) {
+               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);