Re: [PATCH] HID: hiddev: fix potential Spectre v1
Hi Breno,
On 10/17/18 9:47 PM, Breno Leitao wrote:
> uref->usage_index can be indirectly controlled by userspace, hence leading
> to a potential exploitation of the Spectre variant 1 vulnerability.
>
> This problem might show up in the cmd = HIDIOCGCOLLECTIONINDEX flow at
> function
> hiddev_ioctl_usage(), where uref->usage_index is compared to field->maxusage
> and then used as an index to dereference field->usage array.
>
> This is a summary of the current flow, which matches the traditional
> Spectre V1 issue:
>
> copy_from_user(uref, user_arg, sizeof(*uref))
> if (uref->usage_index >= field->maxusage)
> goto inval;
> i = field->usage[uref->usage_index].collection_index;
> return i;
>
> This patch fixes this by sanitizing field uref->usage_index before using it to
> index field->usage, thus, avoiding speculation in the first load.
>
> Signed-off-by: Breno Leitao
> ---
> drivers/hid/usbhid/hiddev.c | 3 +++
> 1 file changed, 3 insertions(+)
>
> diff --git a/drivers/hid/usbhid/hiddev.c b/drivers/hid/usbhid/hiddev.c
> index 23872d08308c..8829cbc1f6b1 100644
> --- a/drivers/hid/usbhid/hiddev.c
> +++ b/drivers/hid/usbhid/hiddev.c
> @@ -512,6 +512,9 @@ static noinline int hiddev_ioctl_usage(struct hiddev
> *hiddev, unsigned int cmd,
> if (cmd == HIDIOCGCOLLECTIONINDEX) {
> if (uref->usage_index >= field->maxusage)
> goto inval;
> + uref->usage_index =
> + array_index_nospec(uref->usage_index,
> +field->maxusage);
Good catch.
> } else if (uref->usage_index >= field->report_count)
> goto inval;
Although, notice that this is the same index, and it can be used to index
field->value[]
at lines 526 and 532.
Thanks
--
Gustavo
Re: [PATCH v3] usbip: vhci_sysfs: fix potential Spectre v1
On 05/22/2018 10:56 AM, Shuah Khan wrote:
On 05/18/2018 07:13 PM, Gustavo A. R. Silva wrote:
pdev_nr and rhport can be controlled by user-space, hence leading to
a potential exploitation of the Spectre variant 1 vulnerability.
This issue was detected with the help of Smatch:
drivers/usb/usbip/vhci_sysfs.c:238 detach_store() warn: potential spectre issue
'vhcis'
drivers/usb/usbip/vhci_sysfs.c:328 attach_store() warn: potential spectre issue
'vhcis'
drivers/usb/usbip/vhci_sysfs.c:338 attach_store() warn: potential spectre issue
'vhci->vhci_hcd_ss->vdev'
drivers/usb/usbip/vhci_sysfs.c:340 attach_store() warn: potential spectre issue
'vhci->vhci_hcd_hs->vdev'
Fix this by sanitizing pdev_nr and rhport before using them to index
vhcis and vhci->vhci_hcd_ss->vdev respectively.
Notice that given that speculation windows are large, the policy is
to kill the speculation on the first load and not worry if it can be
completed with a dependent load/store [1].
[1] https://marc.info/?l=linux-kernel=152449131114778=2
Cc: sta...@vger.kernel.org
Signed-off-by: Gustavo A. R. Silva <gust...@embeddedor.com>
---
Changes in v3:
- Pass the addresses of pdev_nr and rhport into valid_port and
valid_args.
Changes in v2:
- Place the barriers into valid_port.
drivers/usb/usbip/vhci_sysfs.c | 24
1 file changed, 16 insertions(+), 8 deletions(-)
diff --git a/drivers/usb/usbip/vhci_sysfs.c b/drivers/usb/usbip/vhci_sysfs.c
index 4880838..be37aec 100644
--- a/drivers/usb/usbip/vhci_sysfs.c
+++ b/drivers/usb/usbip/vhci_sysfs.c
@@ -10,6 +10,9 @@
#include
#include
+/* Hardening for Spectre-v1 */
+#include
+
#include "usbip_common.h"
#include "vhci.h"
@@ -205,16 +208,20 @@ static int vhci_port_disconnect(struct vhci_hcd *vhci_hcd, __u32 rhport)
return 0;
}
-static int valid_port(__u32 pdev_nr, __u32 rhport)
+static int valid_port(__u32 *pdev_nr, __u32 *rhport)
{
- if (pdev_nr >= vhci_num_controllers) {
- pr_err("pdev %u\n", pdev_nr);
+ if (*pdev_nr >= vhci_num_controllers) {
+ pr_err("pdev %u\n", *pdev_nr);
return 0;
}
- if (rhport >= VHCI_HC_PORTS) {
- pr_err("rhport %u\n", rhport);
+ *pdev_nr = array_index_nospec(*pdev_nr, vhci_num_controllers);
+
+ if (*rhport >= VHCI_HC_PORTS) {
+ pr_err("rhport %u\n", *rhport);
return 0;
}
+ *rhport = array_index_nospec(*rhport, VHCI_HC_PORTS);
+
return 1;
}
@@ -232,7 +239,7 @@ static ssize_t detach_store(struct device *dev, struct device_attribute *attr,
pdev_nr = port_to_pdev_nr(port);
rhport = port_to_rhport(port);
- if (!valid_port(pdev_nr, rhport))
+ if (!valid_port(_nr, ))
return -EINVAL;
hcd = platform_get_drvdata(vhcis[pdev_nr].pdev);
@@ -258,7 +265,8 @@ static ssize_t detach_store(struct device *dev, struct
device_attribute *attr,
}
static DEVICE_ATTR_WO(detach);
-static int valid_args(__u32 pdev_nr, __u32 rhport, enum usb_device_speed speed)
+static int valid_args(__u32 *pdev_nr, __u32 *rhport,
+ enum usb_device_speed speed)
{
if (!valid_port(pdev_nr, rhport)) {
return 0;
@@ -322,7 +330,7 @@ static ssize_t attach_store(struct device *dev, struct
device_attribute *attr,
sockfd, devid, speed);
/* check received parameters */
- if (!valid_args(pdev_nr, rhport, speed))
+ if (!valid_args(_nr, , speed))
return -EINVAL;
hcd = platform_get_drvdata(vhcis[pdev_nr].pdev);
Looks good to me. Thanks for taking care of this.
Glad to help. :)
Acked-by: Shuah Khan (Samsung OSG) <sh...@kernel.org>
Thanks
--
Gustavo
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2] usbip: vhci_sysfs: fix potential Spectre v1
On 05/19/2018 02:04 AM, Greg Kroah-Hartman wrote: Greg, I've been talking with Dan Williams (intel) about this kind of issues [1] and it seems my original assumptions are correct. Hence, this patch is not useful and, in order to actually prevent speculation here we would need to pass the address of pdev_nr and rhport into valid_port, otherwise there may be speculation at drivers/usb/usbip/vhci_sysfs.c:235: if (!valid_port(pdev_nr, rhport)) return -EINVAL; hcd = platform_get_drvdata(vhcis[pdev_nr].pdev); Ah, yes, sorry, you do need to pass the address through, my mistake completely. But the location for the checking is still the right place to do it, so I was half-right :) Yep. And that totally make sense. I already sent v3: https://marc.info/?l=linux-kernel=152669243313887=2 Thanks! -- Gustavo -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v3] usbip: vhci_sysfs: fix potential Spectre v1
pdev_nr and rhport can be controlled by user-space, hence leading to
a potential exploitation of the Spectre variant 1 vulnerability.
This issue was detected with the help of Smatch:
drivers/usb/usbip/vhci_sysfs.c:238 detach_store() warn: potential spectre issue
'vhcis'
drivers/usb/usbip/vhci_sysfs.c:328 attach_store() warn: potential spectre issue
'vhcis'
drivers/usb/usbip/vhci_sysfs.c:338 attach_store() warn: potential spectre issue
'vhci->vhci_hcd_ss->vdev'
drivers/usb/usbip/vhci_sysfs.c:340 attach_store() warn: potential spectre issue
'vhci->vhci_hcd_hs->vdev'
Fix this by sanitizing pdev_nr and rhport before using them to index
vhcis and vhci->vhci_hcd_ss->vdev respectively.
Notice that given that speculation windows are large, the policy is
to kill the speculation on the first load and not worry if it can be
completed with a dependent load/store [1].
[1] https://marc.info/?l=linux-kernel=152449131114778=2
Cc: sta...@vger.kernel.org
Signed-off-by: Gustavo A. R. Silva <gust...@embeddedor.com>
---
Changes in v3:
- Pass the addresses of pdev_nr and rhport into valid_port and
valid_args.
Changes in v2:
- Place the barriers into valid_port.
drivers/usb/usbip/vhci_sysfs.c | 24
1 file changed, 16 insertions(+), 8 deletions(-)
diff --git a/drivers/usb/usbip/vhci_sysfs.c b/drivers/usb/usbip/vhci_sysfs.c
index 4880838..be37aec 100644
--- a/drivers/usb/usbip/vhci_sysfs.c
+++ b/drivers/usb/usbip/vhci_sysfs.c
@@ -10,6 +10,9 @@
#include
#include
+/* Hardening for Spectre-v1 */
+#include
+
#include "usbip_common.h"
#include "vhci.h"
@@ -205,16 +208,20 @@ static int vhci_port_disconnect(struct vhci_hcd
*vhci_hcd, __u32 rhport)
return 0;
}
-static int valid_port(__u32 pdev_nr, __u32 rhport)
+static int valid_port(__u32 *pdev_nr, __u32 *rhport)
{
- if (pdev_nr >= vhci_num_controllers) {
- pr_err("pdev %u\n", pdev_nr);
+ if (*pdev_nr >= vhci_num_controllers) {
+ pr_err("pdev %u\n", *pdev_nr);
return 0;
}
- if (rhport >= VHCI_HC_PORTS) {
- pr_err("rhport %u\n", rhport);
+ *pdev_nr = array_index_nospec(*pdev_nr, vhci_num_controllers);
+
+ if (*rhport >= VHCI_HC_PORTS) {
+ pr_err("rhport %u\n", *rhport);
return 0;
}
+ *rhport = array_index_nospec(*rhport, VHCI_HC_PORTS);
+
return 1;
}
@@ -232,7 +239,7 @@ static ssize_t detach_store(struct device *dev, struct
device_attribute *attr,
pdev_nr = port_to_pdev_nr(port);
rhport = port_to_rhport(port);
- if (!valid_port(pdev_nr, rhport))
+ if (!valid_port(_nr, ))
return -EINVAL;
hcd = platform_get_drvdata(vhcis[pdev_nr].pdev);
@@ -258,7 +265,8 @@ static ssize_t detach_store(struct device *dev, struct
device_attribute *attr,
}
static DEVICE_ATTR_WO(detach);
-static int valid_args(__u32 pdev_nr, __u32 rhport, enum usb_device_speed speed)
+static int valid_args(__u32 *pdev_nr, __u32 *rhport,
+ enum usb_device_speed speed)
{
if (!valid_port(pdev_nr, rhport)) {
return 0;
@@ -322,7 +330,7 @@ static ssize_t attach_store(struct device *dev, struct
device_attribute *attr,
sockfd, devid, speed);
/* check received parameters */
- if (!valid_args(pdev_nr, rhport, speed))
+ if (!valid_args(_nr, , speed))
return -EINVAL;
hcd = platform_get_drvdata(vhcis[pdev_nr].pdev);
--
2.7.4
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2] usbip: vhci_sysfs: fix potential Spectre v1
On 05/18/2018 11:06 AM, Shuah Khan wrote: On 05/18/2018 07:47 AM, Greg Kroah-Hartman wrote: On Thu, May 17, 2018 at 03:16:28PM -0500, Gustavo A. R. Silva wrote: pdev_nr and rhport can be controlled by user-space, hence leading to a potential exploitation of the Spectre variant 1 vulnerability. This issue was detected with the help of Smatch: drivers/usb/usbip/vhci_sysfs.c:238 detach_store() warn: potential spectre issue 'vhcis' drivers/usb/usbip/vhci_sysfs.c:328 attach_store() warn: potential spectre issue 'vhcis' drivers/usb/usbip/vhci_sysfs.c:338 attach_store() warn: potential spectre issue 'vhci->vhci_hcd_ss->vdev' drivers/usb/usbip/vhci_sysfs.c:340 attach_store() warn: potential spectre issue 'vhci->vhci_hcd_hs->vdev' Fix this by sanitizing pdev_nr and rhport before using them to index vhcis and vhci->vhci_hcd_ss->vdev respectively. Notice that given that speculation windows are large, the policy is to kill the speculation on the first load and not worry if it can be completed with a dependent load/store [1]. [1] https://marc.info/?l=linux-kernel=152449131114778=2 Cc: sta...@vger.kernel.org Signed-off-by: Gustavo A. R. Silva <gust...@embeddedor.com> --- Changes in v2: - Place the barriers into valid_port. attach_store() doesn't call valid_port() - can you make the change to have attach_store() call valid_port() to protect that code path. Thanks for the change. I'll wait for Shuah's ack/review before queueing this up just as she knows that codebase much better than anyone else. Greg, I've been talking with Dan Williams (intel) about this kind of issues [1] and it seems my original assumptions are correct. Hence, this patch is not useful and, in order to actually prevent speculation here we would need to pass the address of pdev_nr and rhport into valid_port, otherwise there may be speculation at drivers/usb/usbip/vhci_sysfs.c:235: if (!valid_port(pdev_nr, rhport)) return -EINVAL; hcd = platform_get_drvdata(vhcis[pdev_nr].pdev); [1] https://marc.info/?l=linux-kernel=152668152509103=2 Thanks -- Gustavo -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v2] usbip: vhci_sysfs: fix potential Spectre v1
pdev_nr and rhport can be controlled by user-space, hence leading to
a potential exploitation of the Spectre variant 1 vulnerability.
This issue was detected with the help of Smatch:
drivers/usb/usbip/vhci_sysfs.c:238 detach_store() warn: potential spectre issue
'vhcis'
drivers/usb/usbip/vhci_sysfs.c:328 attach_store() warn: potential spectre issue
'vhcis'
drivers/usb/usbip/vhci_sysfs.c:338 attach_store() warn: potential spectre issue
'vhci->vhci_hcd_ss->vdev'
drivers/usb/usbip/vhci_sysfs.c:340 attach_store() warn: potential spectre issue
'vhci->vhci_hcd_hs->vdev'
Fix this by sanitizing pdev_nr and rhport before using them to index
vhcis and vhci->vhci_hcd_ss->vdev respectively.
Notice that given that speculation windows are large, the policy is
to kill the speculation on the first load and not worry if it can be
completed with a dependent load/store [1].
[1] https://marc.info/?l=linux-kernel=152449131114778=2
Cc: sta...@vger.kernel.org
Signed-off-by: Gustavo A. R. Silva <gust...@embeddedor.com>
---
Changes in v2:
- Place the barriers into valid_port.
drivers/usb/usbip/vhci_sysfs.c | 6 ++
1 file changed, 6 insertions(+)
diff --git a/drivers/usb/usbip/vhci_sysfs.c b/drivers/usb/usbip/vhci_sysfs.c
index 4880838..69db0c9 100644
--- a/drivers/usb/usbip/vhci_sysfs.c
+++ b/drivers/usb/usbip/vhci_sysfs.c
@@ -10,6 +10,8 @@
#include
#include
+#include
+
#include "usbip_common.h"
#include "vhci.h"
@@ -211,10 +213,14 @@ static int valid_port(__u32 pdev_nr, __u32 rhport)
pr_err("pdev %u\n", pdev_nr);
return 0;
}
+ pdev_nr = array_index_nospec(pdev_nr, vhci_num_controllers);
+
if (rhport >= VHCI_HC_PORTS) {
pr_err("rhport %u\n", rhport);
return 0;
}
+ rhport = array_index_nospec(rhport, VHCI_HC_PORTS);
+
return 1;
}
--
2.7.4
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] usbip: vhci_sysfs: fix potential Spectre v1
On 05/17/2018 02:15 PM, Greg Kroah-Hartman wrote: Shouldn't we just do this in one place, in the valid_port() function? That way it keeps the range checking logic in one place (now it is in 3 places in the function), which should make maintenance much simpler. Yep, I thought about that, the thing is: what happens if the hardware is "trained" to predict that valid_port always evaluates to false, and then malicious values are stored in pdev_nr and nhport? It seems to me that under this scenario we need to serialize instructions in this place. What do you think? I don't understand, it should not matter where you put the barrier. Be it a function call back or right after it, it does the same thing, it stops speculation from crossing that barrier. Yeah. It makes sense. So it _should_ work either way, if I understand the issue correctly. If not, what am I missing? No. It seems I'm the one who was missing something. I'll place the barrier into valid_port and send v2 shortly. Thanks! -- Gustavo -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] usbip: vhci_sysfs: fix potential Spectre v1
Hi Greg, On 05/17/2018 01:51 AM, Greg Kroah-Hartman wrote: On Wed, May 16, 2018 at 05:22:00PM -0500, Gustavo A. R. Silva wrote: pdev_nr and rhport can be controlled by user-space, hence leading to a potential exploitation of the Spectre variant 1 vulnerability. This issue was detected with the help of Smatch: drivers/usb/usbip/vhci_sysfs.c:238 detach_store() warn: potential spectre issue 'vhcis' drivers/usb/usbip/vhci_sysfs.c:328 attach_store() warn: potential spectre issue 'vhcis' drivers/usb/usbip/vhci_sysfs.c:338 attach_store() warn: potential spectre issue 'vhci->vhci_hcd_ss->vdev' drivers/usb/usbip/vhci_sysfs.c:340 attach_store() warn: potential spectre issue 'vhci->vhci_hcd_hs->vdev' Nit, no need to line-wrap long error messages from tools :) Got it. Fix this by sanitizing pdev_nr and rhport before using them to index vhcis and vhci->vhci_hcd_ss->vdev respectively. Notice that given that speculation windows are large, the policy is to kill the speculation on the first load and not worry if it can be completed with a dependent load/store [1]. [1] https://marc.info/?l=linux-kernel=152449131114778=2 Cc: sta...@vger.kernel.org Signed-off-by: Gustavo A. R. Silva <gust...@embeddedor.com> --- drivers/usb/usbip/vhci_sysfs.c | 6 ++ 1 file changed, 6 insertions(+) diff --git a/drivers/usb/usbip/vhci_sysfs.c b/drivers/usb/usbip/vhci_sysfs.c index 4880838..9045888 100644 --- a/drivers/usb/usbip/vhci_sysfs.c +++ b/drivers/usb/usbip/vhci_sysfs.c @@ -10,6 +10,8 @@ #include #include +#include + #include "usbip_common.h" #include "vhci.h" @@ -235,6 +237,8 @@ static ssize_t detach_store(struct device *dev, struct device_attribute *attr, if (!valid_port(pdev_nr, rhport)) return -EINVAL; + pdev_nr = array_index_nospec(pdev_nr, vhci_num_controllers); + rhport = array_index_nospec(rhport, VHCI_HC_PORTS); Shouldn't we just do this in one place, in the valid_port() function? That way it keeps the range checking logic in one place (now it is in 3 places in the function), which should make maintenance much simpler. Yep, I thought about that, the thing is: what happens if the hardware is "trained" to predict that valid_port always evaluates to false, and then malicious values are stored in pdev_nr and nhport? It seems to me that under this scenario we need to serialize instructions in this place. What do you think? Thanks -- Gustavo -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] usbip: vhci_sysfs: fix potential Spectre v1
pdev_nr and rhport can be controlled by user-space, hence leading to
a potential exploitation of the Spectre variant 1 vulnerability.
This issue was detected with the help of Smatch:
drivers/usb/usbip/vhci_sysfs.c:238 detach_store() warn: potential
spectre issue 'vhcis'
drivers/usb/usbip/vhci_sysfs.c:328 attach_store() warn: potential
spectre issue 'vhcis'
drivers/usb/usbip/vhci_sysfs.c:338 attach_store() warn: potential
spectre issue 'vhci->vhci_hcd_ss->vdev'
drivers/usb/usbip/vhci_sysfs.c:340 attach_store() warn: potential
spectre issue 'vhci->vhci_hcd_hs->vdev'
Fix this by sanitizing pdev_nr and rhport before using them to index
vhcis and vhci->vhci_hcd_ss->vdev respectively.
Notice that given that speculation windows are large, the policy is
to kill the speculation on the first load and not worry if it can be
completed with a dependent load/store [1].
[1] https://marc.info/?l=linux-kernel=152449131114778=2
Cc: sta...@vger.kernel.org
Signed-off-by: Gustavo A. R. Silva <gust...@embeddedor.com>
---
drivers/usb/usbip/vhci_sysfs.c | 6 ++
1 file changed, 6 insertions(+)
diff --git a/drivers/usb/usbip/vhci_sysfs.c b/drivers/usb/usbip/vhci_sysfs.c
index 4880838..9045888 100644
--- a/drivers/usb/usbip/vhci_sysfs.c
+++ b/drivers/usb/usbip/vhci_sysfs.c
@@ -10,6 +10,8 @@
#include
#include
+#include
+
#include "usbip_common.h"
#include "vhci.h"
@@ -235,6 +237,8 @@ static ssize_t detach_store(struct device *dev, struct
device_attribute *attr,
if (!valid_port(pdev_nr, rhport))
return -EINVAL;
+ pdev_nr = array_index_nospec(pdev_nr, vhci_num_controllers);
+ rhport = array_index_nospec(rhport, VHCI_HC_PORTS);
hcd = platform_get_drvdata(vhcis[pdev_nr].pdev);
if (hcd == NULL) {
dev_err(dev, "port is not ready %u\n", port);
@@ -325,6 +329,8 @@ static ssize_t attach_store(struct device *dev, struct
device_attribute *attr,
if (!valid_args(pdev_nr, rhport, speed))
return -EINVAL;
+ pdev_nr = array_index_nospec(pdev_nr, vhci_num_controllers);
+ rhport = array_index_nospec(rhport, VHCI_HC_PORTS);
hcd = platform_get_drvdata(vhcis[pdev_nr].pdev);
if (hcd == NULL) {
dev_err(dev, "port %d is not ready\n", port);
--
2.7.4
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] typec: tcpm: Fix incorrect 'and' operator
Currently, logical and is being used instead of *bitwise* and.
Fix this by using a proper bitwise and operator.
Addresses-Coverity-ID: 1468455 ("Logical vs. bitwise operator")
Fixes: 64f7c494a3c0 ("typec: tcpm: Add support for sink PPS related
messages")
Signed-off-by: Gustavo A. R. Silva <gust...@embeddedor.com>
---
drivers/usb/typec/tcpm.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/usb/typec/tcpm.c b/drivers/usb/typec/tcpm.c
index 1ee259b..7ee417a 100644
--- a/drivers/usb/typec/tcpm.c
+++ b/drivers/usb/typec/tcpm.c
@@ -1772,7 +1772,7 @@ static void tcpm_pd_ext_msg_request(struct tcpm_port
*port,
enum pd_ext_msg_type type = pd_header_type_le(msg->header);
unsigned int data_size =
pd_ext_header_data_size_le(msg->ext_msg.header);
- if (!(msg->ext_msg.header && PD_EXT_HDR_CHUNKED)) {
+ if (!(msg->ext_msg.header & PD_EXT_HDR_CHUNKED)) {
tcpm_log(port, "Unchunked extended messages unsupported");
return;
}
--
2.7.4
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] usb: core: hcd: mark expected switch fall-through
In preparation to enabling -Wimplicit-fallthrough, mark switch cases
where we are expecting to fall through.
Addresses-Coverity-ID: 1468266 ("Missing break in switch")
Signed-off-by: Gustavo A. R. Silva <gust...@embeddedor.com>
---
drivers/usb/core/hcd.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/drivers/usb/core/hcd.c b/drivers/usb/core/hcd.c
index 66cd3f9..1c21955 100644
--- a/drivers/usb/core/hcd.c
+++ b/drivers/usb/core/hcd.c
@@ -2819,6 +2819,7 @@ int usb_add_hcd(struct usb_hcd *hcd,
case HCD_USB32:
rhdev->rx_lanes = 2;
rhdev->tx_lanes = 2;
+ /* fall through */
case HCD_USB31:
rhdev->speed = USB_SPEED_SUPER_PLUS;
break;
--
2.7.4
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v2] USB: wusbcore: crypto: Remove VLA usage
In preparation to enabling -Wvla, remove VLA and replace it with dynamic memory allocation instead. The use of stack Variable Length Arrays needs to be avoided, as they can be a vector for stack exhaustion, which can be both a runtime bug or a security flaw. Also, in general, as code evolves it is easy to lose track of how big a VLA can get. Thus, we can end up having runtime failures that are hard to debug. Also, fixed as part of the directive to remove all VLAs from the kernel: https://lkml.org/lkml/2018/3/7/621 Notice that in this particular case, an alternative to kzalloc is kcalloc, in which case the code would look as follows instead: iv = kcalloc(crypto_skcipher_ivsize(tfm_cbc), sizeof(*iv), GFP_KERNEL); but if the data type of _iv_ never changes, or the type size is always one byte, kzalloc is good enough. Signed-off-by: Gustavo A. R. Silva <gust...@embeddedor.com> --- Changes in v2: - Fix a memory leak in previous patch. drivers/usb/wusbcore/crypto.c | 8 ++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/drivers/usb/wusbcore/crypto.c b/drivers/usb/wusbcore/crypto.c index 4c00be2d..aff50eb 100644 --- a/drivers/usb/wusbcore/crypto.c +++ b/drivers/usb/wusbcore/crypto.c @@ -202,7 +202,7 @@ static int wusb_ccm_mac(struct crypto_skcipher *tfm_cbc, struct scatterlist sg[4], sg_dst; void *dst_buf; size_t dst_size; - u8 iv[crypto_skcipher_ivsize(tfm_cbc)]; + u8 *iv; size_t zero_padding; /* @@ -224,7 +224,9 @@ static int wusb_ccm_mac(struct crypto_skcipher *tfm_cbc, if (!dst_buf) goto error_dst_buf; - memset(iv, 0, sizeof(iv)); + iv = kzalloc(crypto_skcipher_ivsize(tfm_cbc), GFP_KERNEL); + if (!iv) + goto error_iv; /* Setup B0 */ scratch->b0.flags = 0x59; /* Format B0 */ @@ -276,6 +278,8 @@ static int wusb_ccm_mac(struct crypto_skcipher *tfm_cbc, bytewise_xor(mic, >ax, iv, 8); result = 8; error_cbc_crypt: + kfree(iv); +error_iv: kfree(dst_buf); error_dst_buf: return result; -- 2.7.4 -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] USB: wusbcore: crypto: Remove VLA usage
I just discovered an issue with this patch. Please, drop it. I'll send v2 shortly. Thanks -- Gustavo On 03/16/2018 08:01 AM, Gustavo A. R. Silva wrote: In preparation to enabling -Wvla, remove VLA and replace it with dynamic memory allocation instead. The use of stack Variable Length Arrays needs to be avoided, as they can be a vector for stack exhaustion, which can be both a runtime bug or a security flaw. Also, in general, as code evolves it is easy to lose track of how big a VLA can get. Thus, we can end up having runtime failures that are hard to debug. Also, fixed as part of the directive to remove all VLAs from the kernel: https://lkml.org/lkml/2018/3/7/621 Notice that in this particular case, an alternative to kzalloc is kcalloc, in which case the code would look as follows instead: iv = kcalloc(crypto_skcipher_ivsize(tfm_cbc), sizeof(*iv), GFP_KERNEL); but if the data type of _iv_ never changes, or the type size is always one byte, kzalloc is good enough. Signed-off-by: Gustavo A. R. Silva <gust...@embeddedor.com> --- drivers/usb/wusbcore/crypto.c | 11 +++ 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/drivers/usb/wusbcore/crypto.c b/drivers/usb/wusbcore/crypto.c index 4c00be2d..3511473 100644 --- a/drivers/usb/wusbcore/crypto.c +++ b/drivers/usb/wusbcore/crypto.c @@ -202,7 +202,7 @@ static int wusb_ccm_mac(struct crypto_skcipher *tfm_cbc, struct scatterlist sg[4], sg_dst; void *dst_buf; size_t dst_size; - u8 iv[crypto_skcipher_ivsize(tfm_cbc)]; + u8 *iv; size_t zero_padding; /* @@ -222,9 +222,11 @@ static int wusb_ccm_mac(struct crypto_skcipher *tfm_cbc, zero_padding; dst_buf = kzalloc(dst_size, GFP_KERNEL); if (!dst_buf) - goto error_dst_buf; + goto error_alloc; - memset(iv, 0, sizeof(iv)); + iv = kzalloc(crypto_skcipher_ivsize(tfm_cbc), GFP_KERNEL); + if (!iv) + goto error_alloc; /* Setup B0 */ scratch->b0.flags = 0x59;/* Format B0 */ @@ -276,8 +278,9 @@ static int wusb_ccm_mac(struct crypto_skcipher *tfm_cbc, bytewise_xor(mic, >ax, iv, 8); result = 8; error_cbc_crypt: + kfree(iv); kfree(dst_buf); -error_dst_buf: +error_alloc: return result; } -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] USB: wusbcore: crypto: Remove VLA usage
In preparation to enabling -Wvla, remove VLA and replace it with dynamic memory allocation instead. The use of stack Variable Length Arrays needs to be avoided, as they can be a vector for stack exhaustion, which can be both a runtime bug or a security flaw. Also, in general, as code evolves it is easy to lose track of how big a VLA can get. Thus, we can end up having runtime failures that are hard to debug. Also, fixed as part of the directive to remove all VLAs from the kernel: https://lkml.org/lkml/2018/3/7/621 Notice that in this particular case, an alternative to kzalloc is kcalloc, in which case the code would look as follows instead: iv = kcalloc(crypto_skcipher_ivsize(tfm_cbc), sizeof(*iv), GFP_KERNEL); but if the data type of _iv_ never changes, or the type size is always one byte, kzalloc is good enough. Signed-off-by: Gustavo A. R. Silva <gust...@embeddedor.com> --- drivers/usb/wusbcore/crypto.c | 11 +++ 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/drivers/usb/wusbcore/crypto.c b/drivers/usb/wusbcore/crypto.c index 4c00be2d..3511473 100644 --- a/drivers/usb/wusbcore/crypto.c +++ b/drivers/usb/wusbcore/crypto.c @@ -202,7 +202,7 @@ static int wusb_ccm_mac(struct crypto_skcipher *tfm_cbc, struct scatterlist sg[4], sg_dst; void *dst_buf; size_t dst_size; - u8 iv[crypto_skcipher_ivsize(tfm_cbc)]; + u8 *iv; size_t zero_padding; /* @@ -222,9 +222,11 @@ static int wusb_ccm_mac(struct crypto_skcipher *tfm_cbc, zero_padding; dst_buf = kzalloc(dst_size, GFP_KERNEL); if (!dst_buf) - goto error_dst_buf; + goto error_alloc; - memset(iv, 0, sizeof(iv)); + iv = kzalloc(crypto_skcipher_ivsize(tfm_cbc), GFP_KERNEL); + if (!iv) + goto error_alloc; /* Setup B0 */ scratch->b0.flags = 0x59; /* Format B0 */ @@ -276,8 +278,9 @@ static int wusb_ccm_mac(struct crypto_skcipher *tfm_cbc, bytewise_xor(mic, >ax, iv, 8); result = 8; error_cbc_crypt: + kfree(iv); kfree(dst_buf); -error_dst_buf: +error_alloc: return result; } -- 2.7.4 -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] usb: dwc2: gadget: Use true and false for boolean values
Assign true or false to boolean variables instead of an integer value.
This issue was detected with the help of Coccinelle.
Signed-off-by: Gustavo A. R. Silva <gust...@embeddedor.com>
---
drivers/usb/dwc2/gadget.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/usb/dwc2/gadget.c b/drivers/usb/dwc2/gadget.c
index e4c3ce0..1f684dd 100644
--- a/drivers/usb/dwc2/gadget.c
+++ b/drivers/usb/dwc2/gadget.c
@@ -116,10 +116,10 @@ static inline void dwc2_gadget_incr_frame_num(struct
dwc2_hsotg_ep *hs_ep)
{
hs_ep->target_frame += hs_ep->interval;
if (hs_ep->target_frame > DSTS_SOFFN_LIMIT) {
- hs_ep->frame_overrun = 1;
+ hs_ep->frame_overrun = true;
hs_ep->target_frame &= DSTS_SOFFN_LIMIT;
} else {
- hs_ep->frame_overrun = 0;
+ hs_ep->frame_overrun = false;
}
}
--
2.7.4
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] USB: misc: chaoskey: Use true and false for boolean values
Assign true or false to boolean variables instead of an integer value.
This issue was detected with the help of Coccinelle.
Signed-off-by: Gustavo A. R. Silva <gust...@embeddedor.com>
---
drivers/usb/misc/chaoskey.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/drivers/usb/misc/chaoskey.c b/drivers/usb/misc/chaoskey.c
index b6a1b93..716cb51 100644
--- a/drivers/usb/misc/chaoskey.c
+++ b/drivers/usb/misc/chaoskey.c
@@ -183,10 +183,10 @@ static int chaoskey_probe(struct usb_interface *interface,
dev->in_ep = in_ep;
if (le16_to_cpu(udev->descriptor.idVendor) != ALEA_VENDOR_ID)
- dev->reads_started = 1;
+ dev->reads_started = true;
dev->size = size;
- dev->present = 1;
+ dev->present = true;
init_waitqueue_head(>wait_q);
@@ -239,7 +239,7 @@ static void chaoskey_disconnect(struct usb_interface
*interface)
usb_set_intfdata(interface, NULL);
mutex_lock(>lock);
- dev->present = 0;
+ dev->present = false;
usb_poison_urb(dev->urb);
if (!dev->open) {
--
2.7.4
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] usb: gadget: compress return logic into one line
Simplify return logic and avoid unnecessary variable assignment.
Signed-off-by: Gustavo A. R. Silva <garsi...@embeddedor.com>
---
drivers/usb/gadget/legacy/ncm.c | 6 ++
1 file changed, 2 insertions(+), 4 deletions(-)
diff --git a/drivers/usb/gadget/legacy/ncm.c b/drivers/usb/gadget/legacy/ncm.c
index fcee1ee0..8465f08 100644
--- a/drivers/usb/gadget/legacy/ncm.c
+++ b/drivers/usb/gadget/legacy/ncm.c
@@ -102,10 +102,8 @@ static int ncm_do_config(struct usb_configuration *c)
}
f_ncm = usb_get_function(f_ncm_inst);
- if (IS_ERR(f_ncm)) {
- status = PTR_ERR(f_ncm);
- return status;
- }
+ if (IS_ERR(f_ncm))
+ return PTR_ERR(f_ncm);
status = usb_add_function(c, f_ncm);
if (status < 0) {
--
2.7.4
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] USB: usbip: remove useless call in usbip_recv
Calling msg_data_left() is only useful for its return value,
which in this particular case is ignored.
Fix this by removing such call.
Addresses-Coverity-ID: 1427080
Fixes: 90120d15f4c3 ("usbip: prevent leaking socket pointer address in
messages")
Signed-off-by: Gustavo A. R. Silva <garsi...@embeddedor.com>
---
drivers/usb/usbip/usbip_common.c | 1 -
1 file changed, 1 deletion(-)
diff --git a/drivers/usb/usbip/usbip_common.c b/drivers/usb/usbip/usbip_common.c
index 7b219d9..b5af6fc 100644
--- a/drivers/usb/usbip/usbip_common.c
+++ b/drivers/usb/usbip/usbip_common.c
@@ -325,7 +325,6 @@ int usbip_recv(struct socket *sock, void *buf, int size)
usbip_dbg_xmit("enter\n");
do {
- msg_data_left();
sock->sk->sk_allocation = GFP_NOIO;
result = sock_recvmsg(sock, , MSG_WAITALL);
--
2.7.4
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] usbnet: ipheth: fix potential null pointer dereference in ipheth_carrier_set
_dev_ is being dereferenced before it is null checked, hence there
is a potential null pointer dereference.
Fix this by moving the pointer dereference after _dev_ has been null
checked.
Addresses-Coverity-ID: 1462020
Fixes: bb1b40c7cb86 ("usbnet: ipheth: prevent TX queue timeouts when device not
ready")
Signed-off-by: Gustavo A. R. Silva <garsi...@embeddedor.com>
---
drivers/net/usb/ipheth.c | 5 -
1 file changed, 4 insertions(+), 1 deletion(-)
diff --git a/drivers/net/usb/ipheth.c b/drivers/net/usb/ipheth.c
index ca71f6c..7275761 100644
--- a/drivers/net/usb/ipheth.c
+++ b/drivers/net/usb/ipheth.c
@@ -291,12 +291,15 @@ static void ipheth_sndbulk_callback(struct urb *urb)
static int ipheth_carrier_set(struct ipheth_device *dev)
{
- struct usb_device *udev = dev->udev;
+ struct usb_device *udev;
int retval;
+
if (!dev)
return 0;
if (!dev->confirmed_pairing)
return 0;
+
+ udev = dev->udev;
retval = usb_control_msg(udev,
usb_rcvctrlpipe(udev, IPHETH_CTRL_ENDP),
IPHETH_CMD_CARRIER_CHECK, /* request */
--
2.7.4
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 6/9] usb: host: isp1362-hcd: mark expected switch fall-through
Quoting Greg Kroah-Hartman <gre...@linuxfoundation.org>: On Wed, Oct 25, 2017 at 02:05:05PM -0500, Gustavo A. R. Silva wrote: Greg, Quoting "Gustavo A. R. Silva" <garsi...@embeddedor.com>: > In preparation to enabling -Wimplicit-fallthrough, mark switch cases > where we are expecting to fall through. > > Signed-off-by: Gustavo A. R. Silva <garsi...@embeddedor.com> > --- > drivers/usb/host/isp1362-hcd.c | 1 + > 1 file changed, 1 insertion(+) > > diff --git a/drivers/usb/host/isp1362-hcd.c b/drivers/usb/host/isp1362-hcd.c > index 9b7e307..753d576 100644 > --- a/drivers/usb/host/isp1362-hcd.c > +++ b/drivers/usb/host/isp1362-hcd.c > @@ -1578,6 +1578,7 @@ static int isp1362_hub_control(struct usb_hcd > *hcd, u16 typeReq, u16 wValue, >spin_lock_irqsave(_hcd->lock, flags); >isp1362_write_reg32(isp1362_hcd, HCRHSTATUS, RH_HS_OCIC); >spin_unlock_irqrestore(_hcd->lock, flags); > + /* fall through */ I'm suspicious this should be a 'break' instead. What do you think? Yeah, this should be a 'break', care to make that patch up instead? Sure thing. Just some questions about the process to follow: Should I send a v2 replying to this particular thread only? like [PATCH v2 6/9] or should I send just a new patch separated from this patch series? I guess this is the case. Some maintainers have told me that in cases where a particular patch in the series needs an update, the complete patchset should be sent again. But I think that depends on the functional impact the patch has over the whole patchset. Thanks -- Gustavo A. R. Silva -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] usb: host: isp1362-hcd: fix missing break in switch
Add missing break statement to prevent the code for case C_HUB_OVER_CURRENT from falling through to case C_HUB_LOCAL_POWER. Signed-off-by: Gustavo A. R. Silva <garsi...@embeddedor.com> --- drivers/usb/host/isp1362-hcd.c | 1 + 1 file changed, 1 insertion(+) diff --git a/drivers/usb/host/isp1362-hcd.c b/drivers/usb/host/isp1362-hcd.c index 9b7e307..a822de4 100644 --- a/drivers/usb/host/isp1362-hcd.c +++ b/drivers/usb/host/isp1362-hcd.c @@ -1578,6 +1578,7 @@ static int isp1362_hub_control(struct usb_hcd *hcd, u16 typeReq, u16 wValue, spin_lock_irqsave(_hcd->lock, flags); isp1362_write_reg32(isp1362_hcd, HCRHSTATUS, RH_HS_OCIC); spin_unlock_irqrestore(_hcd->lock, flags); + break; case C_HUB_LOCAL_POWER: DBG(0, "C_HUB_LOCAL_POWER\n"); break; -- 2.7.4 -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 6/9] usb: host: isp1362-hcd: mark expected switch fall-through
Quoting Greg Kroah-Hartman <gre...@linuxfoundation.org>: [..] Sure thing. Just some questions about the process to follow: Should I send a v2 replying to this particular thread only? like [PATCH v2 6/9] or should I send just a new patch separated from this patch series? I guess this is the case. Brand new patch is fine, this is gone from my patch queue. Some maintainers have told me that in cases where a particular patch in the series needs an update, the complete patchset should be sent again. But I think that depends on the functional impact the patch has over the whole patchset. Yes, it all depends, the rest of these patches are already in my tree. OK. I'll send a new patch shortly. Thanks! -- Gustavo A. R. Silva -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] USB: serial: io_edgeport: mark expected switch fall-throughs
Hi David, Johan,
Quoting Johan Hovold <jo...@kernel.org>:
On Mon, Oct 30, 2017 at 11:54:33AM +, David Laight wrote:
From: Bjørn Mork
> Sent: 28 October 2017 11:57
> > In preparation to enabling -Wimplicit-fallthrough, mark switch cases
> > where we are expecting to fall through.
> >
> > Notice that in this particular case I replaced "...drop on through"
> > comments with a proper "fall through" comment on its own line, which
> > is what GCC is expecting to find.
>
> Sounds to me like GCC is the wrong tool for this. But I would of course
> not mind if it was *just* the text. However, as your patch cleary
> shows, the simplified logic leads to real problems:
>
> > @@ -1819,8 +1819,8 @@ static void process_rcvd_data(struct
edgeport_serial *edge_serial,
> > edge_serial->rxState = EXPECT_DATA;
> > break;
> > }
> > - /* Else, drop through */
> > }
> > + /* fall through */
> > case EXPECT_DATA: /* Expect data */
> > if (bufferLength < edge_serial->rxBytesRemaining) {
> > rxLen = bufferLength;
>
>
> The original comment clearly marked a *conditional* fall through at the
> correct place. Your patch makes it appear as if there is an
> unconditional fall through here. There is not. The fallthrough only
> applies to one of a number of nested if blocks. There are no less than
> 3 break statements in the same case block.
>
> Not a big deal maybe, just as the lack of any "fall through" comment
> isn't a big deal in the first place. But this change is clearly making
> this code harder to read, and the change is therefore harmful IMHO.
>
> If you can't make -Wimplicit-fallthrough work without removing such
> precise fallthrough markings, then my proposal is to drop it and use
> some other tool.
Just remove the 'else' after the 'break' further up.
Yeah, that might be a good way to resolve this. I was gonna suggest
adding the "fall though" comment before the case while keeping the
"Else, drop through" comment in the branch, but removing the else might
be better.
Thanks for the suggestion.
This code is pretty hard to read as is and could really use some clean
up...
I agree. I'll send a V2 of this patch and then let's see if I can help
with some code refactoring.
Thank you
--
Gustavo A. R. Silva
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] USB: serial: io_edgeport: mark expected switch fall-throughs
Quoting Bjørn Mork <bj...@mork.no>:
"Gustavo A. R. Silva" <garsi...@embeddedor.com> writes:
In preparation to enabling -Wimplicit-fallthrough, mark switch cases
where we are expecting to fall through.
Notice that in this particular case I replaced "...drop on through"
comments with a proper "fall through" comment on its own line, which
is what GCC is expecting to find.
Sounds to me like GCC is the wrong tool for this. But I would of course
not mind if it was *just* the text. However, as your patch cleary
shows, the simplified logic leads to real problems:
@@ -1819,8 +1819,8 @@ static void process_rcvd_data(struct
edgeport_serial *edge_serial,
edge_serial->rxState = EXPECT_DATA;
break;
}
- /* Else, drop through */
}
+ /* fall through */
case EXPECT_DATA: /* Expect data */
if (bufferLength < edge_serial->rxBytesRemaining) {
rxLen = bufferLength;
The original comment clearly marked a *conditional* fall through at the
correct place. Your patch makes it appear as if there is an
unconditional fall through here. There is not. The fallthrough only
applies to one of a number of nested if blocks. There are no less than
3 break statements in the same case block.
I see.
You are right.
Not a big deal maybe, just as the lack of any "fall through" comment
isn't a big deal in the first place. But this change is clearly making
this code harder to read, and the change is therefore harmful IMHO.
If you can't make -Wimplicit-fallthrough work without removing such
precise fallthrough markings, then my proposal is to drop it and use
some other tool.
I will talk with the hardening guys to see what we can do about this.
I appreciate for your comments.
Thanks
--
Gustavo A. R. Silva
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] usb: typec: tps6598x: mark expected switch fall-through
In preparation to enabling -Wimplicit-fallthrough, mark switch cases where we are expecting to fall through. Signed-off-by: Gustavo A. R. Silva <garsi...@embeddedor.com> --- drivers/usb/typec/tps6598x.c | 1 + 1 file changed, 1 insertion(+) diff --git a/drivers/usb/typec/tps6598x.c b/drivers/usb/typec/tps6598x.c index 8893f79..b728d9e 100644 --- a/drivers/usb/typec/tps6598x.c +++ b/drivers/usb/typec/tps6598x.c @@ -403,6 +403,7 @@ static int tps6598x_probe(struct i2c_client *client) case TPS_PORTINFO_DRP_UFP_DRD: case TPS_PORTINFO_DRP_DFP_DRD: tps->typec_cap.dr_set = tps6598x_dr_set; + /* fall through */ case TPS_PORTINFO_DRP_UFP: case TPS_PORTINFO_DRP_DFP: tps->typec_cap.pr_set = tps6598x_pr_set; -- 2.7.4 -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] usb: storage: sddr55: mark expected switch fall-through
In preparation to enabling -Wimplicit-fallthrough, mark switch cases
where we are expecting to fall through.
Signed-off-by: Gustavo A. R. Silva <garsi...@embeddedor.com>
---
drivers/usb/storage/sddr55.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/drivers/usb/storage/sddr55.c b/drivers/usb/storage/sddr55.c
index 147c50b..b973da4 100644
--- a/drivers/usb/storage/sddr55.c
+++ b/drivers/usb/storage/sddr55.c
@@ -604,6 +604,7 @@ static unsigned long sddr55_get_capacity(struct us_data
*us) {
case 0x64:
info->pageshift = 8;
info->smallpageshift = 1;
+ /* fall through */
case 0x5d: // 5d is a ROM card with pagesize 512.
return 0x0020;
--
2.7.4
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] usb: gadget: composite: mark expected switch fall-throughs
Hi Felipe, Quoting Felipe Balbi <ba...@kernel.org>: "Gustavo A. R. Silva" <garsi...@embeddedor.com> writes: In preparation to enabling -Wimplicit-fallthrough, mark switch cases where we are expecting to fall through. Signed-off-by: Gustavo A. R. Silva <garsi...@embeddedor.com> Acked-by: Felipe Balbi <felipe.ba...@linux.intel.com> Thank you for the ACKs. -- Gustavo A. R. Silva -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] usb: wusbcore: wa-xfer: mark expected switch fall-through
In preparation to enabling -Wimplicit-fallthrough, mark switch cases where we are expecting to fall through. Signed-off-by: Gustavo A. R. Silva <garsi...@embeddedor.com> --- drivers/usb/wusbcore/wa-xfer.c | 1 + 1 file changed, 1 insertion(+) diff --git a/drivers/usb/wusbcore/wa-xfer.c b/drivers/usb/wusbcore/wa-xfer.c index e70322b..bee2404 100644 --- a/drivers/usb/wusbcore/wa-xfer.c +++ b/drivers/usb/wusbcore/wa-xfer.c @@ -2156,6 +2156,7 @@ static void wa_complete_remaining_xfer_segs(struct wa_xfer *xfer, * do not increment RPIPE avail for the WA_SEG_DELAYED case * since it has not been submitted to the RPIPE. */ + /* fall through */ case WA_SEG_DELAYED: xfer->segs_done++; current_seg->status = status; -- 2.7.4 -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 5/9] usb: host: ehci-hcd: mark expected switch fall-through
In preparation to enabling -Wimplicit-fallthrough, mark switch cases where we are expecting to fall through. Signed-off-by: Gustavo A. R. Silva <garsi...@embeddedor.com> --- drivers/usb/host/ehci-hcd.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/usb/host/ehci-hcd.c b/drivers/usb/host/ehci-hcd.c index 6e834b83..c560a01 100644 --- a/drivers/usb/host/ehci-hcd.c +++ b/drivers/usb/host/ehci-hcd.c @@ -1012,7 +1012,7 @@ ehci_endpoint_disable (struct usb_hcd *hcd, struct usb_host_endpoint *ep) qh_destroy(ehci, qh); break; } - /* else FALL THROUGH */ + /* fall through */ default: /* caller was supposed to have unlinked any requests; * that's not our job. just leak this memory. -- 2.7.4 -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 1/9] usb: host: fotg210-hcd: mark expected switch fall-through
In preparation to enabling -Wimplicit-fallthrough, mark switch cases where we are expecting to fall through. Signed-off-by: Gustavo A. R. Silva <garsi...@embeddedor.com> --- drivers/usb/host/fotg210-hcd.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/usb/host/fotg210-hcd.c b/drivers/usb/host/fotg210-hcd.c index 457cc65..33a4f7e 100644 --- a/drivers/usb/host/fotg210-hcd.c +++ b/drivers/usb/host/fotg210-hcd.c @@ -5449,7 +5449,7 @@ static void fotg210_endpoint_disable(struct usb_hcd *hcd, qh_destroy(fotg210, qh); break; } - /* else FALL THROUGH */ + /* fall through */ default: /* caller was supposed to have unlinked any requests; * that's not our job. just leak this memory. -- 2.7.4 -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 0/9] mark expected switch fall-throughs
In preparation to enabling -Wimplicit-fallthrough, this patchset aims to mark switch cases where we are expecting to fall through. In Kees Cook words: "This is an unfortunate omission in the C language, and thankfully both gcc and clang have stepped up to solve this the same way static analyzers have solved it. It does both document the intention for humans and provide a way for analyzers to report issues. Having the compiler help us not make mistakes is quite handy." In some cases there were "else FALL THROUGH" or "then fallthrough..." comments already in place. So I replaced them with proper "fall through" comments, which is what GCC is expecting to find. Thanks! Gustavo A. R. Silva (9): usb: host: fotg210-hcd: mark expected switch fall-through usb: host: xhci: mark expected switch fall-through usb: host: xhci-mem: mark expected switch fall-through usb: host: ohci-hcd: mark expected switch fall-through usb: host: ehci-hcd: mark expected switch fall-through usb: host: isp1362-hcd: mark expected switch fall-through usb: host: oxu210hp-hcd: mark expected switch fall-through usb: host: xhci-hub: mark expected switch fall-through usb: host: pci-quirks: mark expected switch fall-through drivers/usb/host/ehci-hcd.c | 2 +- drivers/usb/host/fotg210-hcd.c | 2 +- drivers/usb/host/isp1362-hcd.c | 1 + drivers/usb/host/ohci-hcd.c | 2 +- drivers/usb/host/oxu210hp-hcd.c | 2 +- drivers/usb/host/pci-quirks.c | 2 +- drivers/usb/host/xhci-hub.c | 1 + drivers/usb/host/xhci-mem.c | 1 + drivers/usb/host/xhci.c | 1 + 9 files changed, 9 insertions(+), 5 deletions(-) -- 2.7.4 -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 4/9] usb: host: ohci-hcd: mark expected switch fall-through
In preparation to enabling -Wimplicit-fallthrough, mark switch cases where we are expecting to fall through. Signed-off-by: Gustavo A. R. Silva <garsi...@embeddedor.com> --- drivers/usb/host/ohci-hcd.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/usb/host/ohci-hcd.c b/drivers/usb/host/ohci-hcd.c index 4492482..15ec8f9 100644 --- a/drivers/usb/host/ohci-hcd.c +++ b/drivers/usb/host/ohci-hcd.c @@ -382,7 +382,7 @@ ohci_endpoint_disable (struct usb_hcd *hcd, struct usb_host_endpoint *ep) ed_free (ohci, ed); break; } - /* else FALL THROUGH */ + /* fall through */ default: /* caller was supposed to have unlinked any requests; * that's not our job. can't recover; must leak ed. -- 2.7.4 -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 2/9] usb: host: xhci: mark expected switch fall-through
In preparation to enabling -Wimplicit-fallthrough, mark switch cases where we are expecting to fall through. Signed-off-by: Gustavo A. R. Silva <garsi...@embeddedor.com> --- drivers/usb/host/xhci.c | 1 + 1 file changed, 1 insertion(+) diff --git a/drivers/usb/host/xhci.c b/drivers/usb/host/xhci.c index ee077a2..05db6e97 100644 --- a/drivers/usb/host/xhci.c +++ b/drivers/usb/host/xhci.c @@ -4294,6 +4294,7 @@ static unsigned long long xhci_calculate_intel_u1_timeout( break; } /* Otherwise the calculation is the same as isoc eps */ + /* fall through */ case USB_ENDPOINT_XFER_ISOC: timeout_ns = xhci_service_interval_to_ns(desc); timeout_ns = DIV_ROUND_UP_ULL(timeout_ns * 105, 100); -- 2.7.4 -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 6/9] usb: host: isp1362-hcd: mark expected switch fall-through
Greg,
Quoting "Gustavo A. R. Silva" <garsi...@embeddedor.com>:
In preparation to enabling -Wimplicit-fallthrough, mark switch cases
where we are expecting to fall through.
Signed-off-by: Gustavo A. R. Silva <garsi...@embeddedor.com>
---
drivers/usb/host/isp1362-hcd.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/drivers/usb/host/isp1362-hcd.c b/drivers/usb/host/isp1362-hcd.c
index 9b7e307..753d576 100644
--- a/drivers/usb/host/isp1362-hcd.c
+++ b/drivers/usb/host/isp1362-hcd.c
@@ -1578,6 +1578,7 @@ static int isp1362_hub_control(struct usb_hcd
*hcd, u16 typeReq, u16 wValue,
spin_lock_irqsave(_hcd->lock, flags);
isp1362_write_reg32(isp1362_hcd, HCRHSTATUS,
RH_HS_OCIC);
spin_unlock_irqrestore(_hcd->lock, flags);
+ /* fall through */
I'm suspicious this should be a 'break' instead.
What do you think?
case C_HUB_LOCAL_POWER:
DBG(0, "C_HUB_LOCAL_POWER\n");
break;
--
2.7.4
Thanks
--
Gustavo A. R. Silva
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 8/9] usb: host: xhci-hub: mark expected switch fall-through
In preparation to enabling -Wimplicit-fallthrough, mark switch cases where we are expecting to fall through. Signed-off-by: Gustavo A. R. Silva <garsi...@embeddedor.com> --- drivers/usb/host/xhci-hub.c | 1 + 1 file changed, 1 insertion(+) diff --git a/drivers/usb/host/xhci-hub.c b/drivers/usb/host/xhci-hub.c index 9762333..3693b1f 100644 --- a/drivers/usb/host/xhci-hub.c +++ b/drivers/usb/host/xhci-hub.c @@ -1377,6 +1377,7 @@ int xhci_hub_control(struct usb_hcd *hcd, u16 typeReq, u16 wValue, break; case USB_PORT_FEAT_C_SUSPEND: bus_state->port_c_suspend &= ~(1 << wIndex); + /* fall through */ case USB_PORT_FEAT_C_RESET: case USB_PORT_FEAT_C_BH_PORT_RESET: case USB_PORT_FEAT_C_CONNECTION: -- 2.7.4 -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 9/9] usb: host: pci-quirks: mark expected switch fall-through
In preparation to enabling -Wimplicit-fallthrough, mark switch cases where we are expecting to fall through. Signed-off-by: Gustavo A. R. Silva <garsi...@embeddedor.com> --- drivers/usb/host/pci-quirks.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/usb/host/pci-quirks.c b/drivers/usb/host/pci-quirks.c index 6dda362..6731f8d8d 100644 --- a/drivers/usb/host/pci-quirks.c +++ b/drivers/usb/host/pci-quirks.c @@ -841,7 +841,7 @@ static void quirk_usb_disable_ehci(struct pci_dev *pdev) ehci_bios_handoff(pdev, op_reg_base, cap, offset); break; case 0: /* Illegal reserved cap, set cap=0 so we exit */ - cap = 0; /* then fallthrough... */ + cap = 0; /* fall through */ default: dev_warn(>dev, "EHCI: unrecognized capability %02x\n", -- 2.7.4 -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 6/9] usb: host: isp1362-hcd: mark expected switch fall-through
In preparation to enabling -Wimplicit-fallthrough, mark switch cases where we are expecting to fall through. Signed-off-by: Gustavo A. R. Silva <garsi...@embeddedor.com> --- drivers/usb/host/isp1362-hcd.c | 1 + 1 file changed, 1 insertion(+) diff --git a/drivers/usb/host/isp1362-hcd.c b/drivers/usb/host/isp1362-hcd.c index 9b7e307..753d576 100644 --- a/drivers/usb/host/isp1362-hcd.c +++ b/drivers/usb/host/isp1362-hcd.c @@ -1578,6 +1578,7 @@ static int isp1362_hub_control(struct usb_hcd *hcd, u16 typeReq, u16 wValue, spin_lock_irqsave(_hcd->lock, flags); isp1362_write_reg32(isp1362_hcd, HCRHSTATUS, RH_HS_OCIC); spin_unlock_irqrestore(_hcd->lock, flags); + /* fall through */ case C_HUB_LOCAL_POWER: DBG(0, "C_HUB_LOCAL_POWER\n"); break; -- 2.7.4 -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 7/9] usb: host: oxu210hp-hcd: mark expected switch fall-through
In preparation to enabling -Wimplicit-fallthrough, mark switch cases where we are expecting to fall through. Signed-off-by: Gustavo A. R. Silva <garsi...@embeddedor.com> --- drivers/usb/host/oxu210hp-hcd.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/usb/host/oxu210hp-hcd.c b/drivers/usb/host/oxu210hp-hcd.c index ed20fb3..9f8c61e 100644 --- a/drivers/usb/host/oxu210hp-hcd.c +++ b/drivers/usb/host/oxu210hp-hcd.c @@ -3040,7 +3040,7 @@ static void oxu_endpoint_disable(struct usb_hcd *hcd, qh_put(qh); break; } - /* else FALL THROUGH */ + /* fall through */ default: nogood: /* caller was supposed to have unlinked any requests; -- 2.7.4 -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 3/9] usb: host: xhci-mem: mark expected switch fall-through
In preparation to enabling -Wimplicit-fallthrough, mark switch cases where we are expecting to fall through. Signed-off-by: Gustavo A. R. Silva <garsi...@embeddedor.com> --- drivers/usb/host/xhci-mem.c | 1 + 1 file changed, 1 insertion(+) diff --git a/drivers/usb/host/xhci-mem.c b/drivers/usb/host/xhci-mem.c index 795219a..57be885 100644 --- a/drivers/usb/host/xhci-mem.c +++ b/drivers/usb/host/xhci-mem.c @@ -1311,6 +1311,7 @@ static unsigned int xhci_get_endpoint_interval(struct usb_device *udev, * since it uses the same rules as low speed interrupt * endpoints. */ + /* fall through */ case USB_SPEED_LOW: if (usb_endpoint_xfer_int(>desc) || -- 2.7.4 -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] usb: gadget: udc: dummy_hcd: mark expected switch fall-throughs
In preparation to enabling -Wimplicit-fallthrough, mark switch cases where we are expecting to fall through. Signed-off-by: Gustavo A. R. Silva <garsi...@embeddedor.com> --- drivers/usb/gadget/udc/dummy_hcd.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/drivers/usb/gadget/udc/dummy_hcd.c b/drivers/usb/gadget/udc/dummy_hcd.c index 664b64e..9dd26ff 100644 --- a/drivers/usb/gadget/udc/dummy_hcd.c +++ b/drivers/usb/gadget/udc/dummy_hcd.c @@ -569,10 +569,12 @@ static int dummy_enable(struct usb_ep *_ep, if (max <= 1024) break; /* save a return statement */ + /* fall through */ case USB_SPEED_FULL: if (max <= 64) break; /* save a return statement */ + /* fall through */ default: if (max <= 8) break; @@ -590,6 +592,7 @@ static int dummy_enable(struct usb_ep *_ep, if (max <= 1024) break; /* save a return statement */ + /* fall through */ case USB_SPEED_FULL: if (max <= 1023) break; -- 2.7.4 -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] usb: gadget: composite: mark expected switch fall-throughs
In preparation to enabling -Wimplicit-fallthrough, mark switch cases
where we are expecting to fall through.
Signed-off-by: Gustavo A. R. Silva <garsi...@embeddedor.com>
---
drivers/usb/gadget/composite.c | 7 ---
1 file changed, 4 insertions(+), 3 deletions(-)
diff --git a/drivers/usb/gadget/composite.c b/drivers/usb/gadget/composite.c
index 5d061b3..1f1ab6f 100644
--- a/drivers/usb/gadget/composite.c
+++ b/drivers/usb/gadget/composite.c
@@ -170,20 +170,20 @@ int config_ep_by_speed(struct usb_gadget *g,
want_comp_desc = 1;
break;
}
- /* else: Fall trough */
+ /* fall through */
case USB_SPEED_SUPER:
if (gadget_is_superspeed(g)) {
speed_desc = f->ss_descriptors;
want_comp_desc = 1;
break;
}
- /* else: Fall trough */
+ /* fall through */
case USB_SPEED_HIGH:
if (gadget_is_dualspeed(g)) {
speed_desc = f->hs_descriptors;
break;
}
- /* else: fall through */
+ /* fall through */
default:
speed_desc = f->fs_descriptors;
}
@@ -224,6 +224,7 @@ int config_ep_by_speed(struct usb_gadget *g,
case USB_ENDPOINT_XFER_ISOC:
/* mult: bits 1:0 of bmAttributes */
_ep->mult = (comp_desc->bmAttributes & 0x3) + 1;
+ /* fall through */
case USB_ENDPOINT_XFER_BULK:
case USB_ENDPOINT_XFER_INT:
_ep->maxburst = comp_desc->bMaxBurst + 1;
--
2.7.4
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] usb: class: usbtmc: mark expected switch fall-through
In preparation to enabling -Wimplicit-fallthrough, mark switch cases where we are expecting to fall through. Signed-off-by: Gustavo A. R. Silva <garsi...@embeddedor.com> --- drivers/usb/class/usbtmc.c | 1 + 1 file changed, 1 insertion(+) diff --git a/drivers/usb/class/usbtmc.c b/drivers/usb/class/usbtmc.c index 6ebfabf..6adceac 100644 --- a/drivers/usb/class/usbtmc.c +++ b/drivers/usb/class/usbtmc.c @@ -1343,6 +1343,7 @@ static void usbtmc_interrupt(struct urb *urb) case -EOVERFLOW: dev_err(dev, "overflow with length %d, actual length is %d\n", data->iin_wMaxPacketSize, urb->actual_length); + /* fall through */ case -ECONNRESET: case -ENOENT: case -ESHUTDOWN: -- 2.7.4 -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] usb: atm: cxacru: mark expected switch fall-throughs
In preparation to enabling -Wimplicit-fallthrough, mark switch cases where we are expecting to fall through. Signed-off-by: Gustavo A. R. Silva <garsi...@embeddedor.com> --- drivers/usb/atm/cxacru.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/drivers/usb/atm/cxacru.c b/drivers/usb/atm/cxacru.c index 600a670..e4f177d 100644 --- a/drivers/usb/atm/cxacru.c +++ b/drivers/usb/atm/cxacru.c @@ -424,6 +424,7 @@ static ssize_t cxacru_sysfs_store_adsl_state(struct device *dev, case CXPOLL_STOPPING: /* abort stop request */ instance->poll_state = CXPOLL_POLLING; + /* fall through */ case CXPOLL_POLLING: case CXPOLL_SHUTDOWN: /* don't start polling */ @@ -795,6 +796,7 @@ static int cxacru_atm_start(struct usbatm_data *usbatm_instance, case CXPOLL_STOPPING: /* abort stop request */ instance->poll_state = CXPOLL_POLLING; + /* fall through */ case CXPOLL_POLLING: case CXPOLL_SHUTDOWN: /* don't start polling */ -- 2.7.4 -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] usb: host: isp116x-hcd: mark expected switch fall-throughs
In preparation to enabling -Wimplicit-fallthrough, mark switch cases
where we are expecting to fall through.
Addresses-Coverity-ID: 115006
Addresses-Coverity-ID: 115007
Signed-off-by: Gustavo A. R. Silva <garsi...@embeddedor.com>
---
drivers/usb/host/isp116x-hcd.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/drivers/usb/host/isp116x-hcd.c b/drivers/usb/host/isp116x-hcd.c
index 73fec38..ebf2ff2 100644
--- a/drivers/usb/host/isp116x-hcd.c
+++ b/drivers/usb/host/isp116x-hcd.c
@@ -1018,6 +1018,7 @@ static int isp116x_hub_control(struct usb_hcd *hcd,
spin_lock_irqsave(>lock, flags);
isp116x_write_reg32(isp116x, HCRHSTATUS, RH_HS_OCIC);
spin_unlock_irqrestore(>lock, flags);
+ /* fall through */
case C_HUB_LOCAL_POWER:
DBG("C_HUB_LOCAL_POWER\n");
break;
@@ -1433,8 +1434,10 @@ static int isp116x_bus_suspend(struct usb_hcd *hcd)
isp116x_write_reg32(isp116x, HCCONTROL,
(val & ~HCCONTROL_HCFS) |
HCCONTROL_USB_RESET);
+ /* fall through */
case HCCONTROL_USB_RESET:
ret = -EBUSY;
+ /* fall through */
default:/* HCCONTROL_USB_SUSPEND */
spin_unlock_irqrestore(>lock, flags);
break;
--
2.7.4
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] usb: gadget: f_phonet: mark expected switch fall-throughs
In preparation to enabling -Wimplicit-fallthrough, mark switch cases where we are expecting to fall through. Addresses-Coverity-ID: 115004 Addresses-Coverity-ID: 115005 Signed-off-by: Gustavo A. R. Silva <garsi...@embeddedor.com> --- drivers/usb/gadget/function/f_phonet.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/drivers/usb/gadget/function/f_phonet.c b/drivers/usb/gadget/function/f_phonet.c index 9c4c58e..710b688 100644 --- a/drivers/usb/gadget/function/f_phonet.c +++ b/drivers/usb/gadget/function/f_phonet.c @@ -215,6 +215,7 @@ static void pn_tx_complete(struct usb_ep *ep, struct usb_request *req) case -ESHUTDOWN: /* disconnected */ case -ECONNRESET: /* disabled */ dev->stats.tx_aborted_errors++; + /* fall through */ default: dev->stats.tx_errors++; } @@ -362,6 +363,7 @@ static void pn_rx_complete(struct usb_ep *ep, struct usb_request *req) /* Do resubmit in these cases: */ case -EOVERFLOW: /* request buffer overflow */ dev->stats.rx_over_errors++; + /* fall through */ default: dev->stats.rx_errors++; break; -- 2.7.4 -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] usb: serial: kobil_sct: mark expected switch fall-through
In preparation to enabling -Wimplicit-fallthrough, mark switch cases where we are expecting to fall through. Addresses-Coverity-ID: 115014 Signed-off-by: Gustavo A. R. Silva <garsi...@embeddedor.com> --- drivers/usb/serial/kobil_sct.c | 1 + 1 file changed, 1 insertion(+) diff --git a/drivers/usb/serial/kobil_sct.c b/drivers/usb/serial/kobil_sct.c index 3024b9b..2f8fa35 100644 --- a/drivers/usb/serial/kobil_sct.c +++ b/drivers/usb/serial/kobil_sct.c @@ -491,6 +491,7 @@ static void kobil_set_termios(struct tty_struct *tty, break; default: speed = 9600; + /* fall through */ case 9600: urb_val = SUSBCR_SBR_9600; break; -- 2.7.4 -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] usb: storage: uas: mark expected switch fall-through
In preparation to enabling -Wimplicit-fallthrough, mark switch cases where we are expecting to fall through. Addresses-Coverity-ID: 115016 Signed-off-by: Gustavo A. R. Silva <garsi...@embeddedor.com> --- drivers/usb/storage/uas.c | 1 + 1 file changed, 1 insertion(+) diff --git a/drivers/usb/storage/uas.c b/drivers/usb/storage/uas.c index 63cf981..bd4671d 100644 --- a/drivers/usb/storage/uas.c +++ b/drivers/usb/storage/uas.c @@ -668,6 +668,7 @@ static int uas_queuecommand_lck(struct scsi_cmnd *cmnd, break; case DMA_BIDIRECTIONAL: cmdinfo->state |= ALLOC_DATA_IN_URB | SUBMIT_DATA_IN_URB; + /* fall through */ case DMA_TO_DEVICE: cmdinfo->state |= ALLOC_DATA_OUT_URB | SUBMIT_DATA_OUT_URB; case DMA_NONE: -- 2.7.4 -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] usb: gadget: goku_udc: mark expected switch fall-throughs
In preparation to enabling -Wimplicit-fallthrough, mark switch cases
where we are expecting to fall through.
Addresses-Coverity-ID: 145713
Signed-off-by: Gustavo A. R. Silva <garsi...@embeddedor.com>
---
drivers/usb/gadget/udc/goku_udc.c | 14 +-
1 file changed, 9 insertions(+), 5 deletions(-)
diff --git a/drivers/usb/gadget/udc/goku_udc.c
b/drivers/usb/gadget/udc/goku_udc.c
index 8433c22..a85407e 100644
--- a/drivers/usb/gadget/udc/goku_udc.c
+++ b/drivers/usb/gadget/udc/goku_udc.c
@@ -127,11 +127,15 @@ goku_ep_enable(struct usb_ep *_ep, const struct
usb_endpoint_descriptor *desc)
mode = 0;
max = get_unaligned_le16(>wMaxPacketSize);
switch (max) {
- case 64:mode++;
- case 32:mode++;
- case 16:mode++;
- case 8: mode <<= 3;
- break;
+ case 64:
+ mode++; /* fall through */
+ case 32:
+ mode++; /* fall through */
+ case 16:
+ mode++; /* fall through */
+ case 8:
+ mode <<= 3;
+ break;
default:
return -EINVAL;
}
--
2.7.4
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] usb: gadget: serial: mark expected switch fall-through
In preparation to enabling -Wimplicit-fallthrough, mark switch cases
where we are expecting to fall through.
Addresses-Coverity-ID: 1350962
Signed-off-by: Gustavo A. R. Silva <garsi...@embeddedor.com>
---
drivers/usb/gadget/function/u_serial.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/drivers/usb/gadget/function/u_serial.c
b/drivers/usb/gadget/function/u_serial.c
index 4176216..961457e 100644
--- a/drivers/usb/gadget/function/u_serial.c
+++ b/drivers/usb/gadget/function/u_serial.c
@@ -1078,6 +1078,7 @@ static void gs_complete_out(struct usb_ep *ep, struct
usb_request *req)
default:
pr_warn("%s: unexpected %s status %d\n",
__func__, ep->name, req->status);
+ /* fall through */
case 0:
/* normal completion */
spin_lock(>con_lock);
--
2.7.4
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] usb: musb_core: mark expected switch fall-through
In preparation to enabling -Wimplicit-fallthrough, mark switch cases where we are expecting to fall through. Addresses-Coverity-ID: 1397608 Signed-off-by: Gustavo A. R. Silva <garsi...@embeddedor.com> --- drivers/usb/musb/musb_core.c | 1 + 1 file changed, 1 insertion(+) diff --git a/drivers/usb/musb/musb_core.c b/drivers/usb/musb/musb_core.c index ff5a1a8..889ca9b 100644 --- a/drivers/usb/musb/musb_core.c +++ b/drivers/usb/musb/musb_core.c @@ -767,6 +767,7 @@ static irqreturn_t musb_stage0_irq(struct musb *musb, u8 int_usb, case OTG_STATE_B_IDLE: if (!musb->is_active) break; + /* fall through */ case OTG_STATE_B_PERIPHERAL: musb_g_suspend(musb); musb->is_active = musb->g.b_hnp_enable; -- 2.7.4 -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] usb: gadget: f_tcm: mark expected switch fall-through
In preparation to enabling -Wimplicit-fallthrough, mark switch cases
where we are expecting to fall through.
Addresses-Coverity-ID: 703128
Signed-off-by: Gustavo A. R. Silva <garsi...@embeddedor.com>
---
drivers/usb/gadget/function/f_tcm.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/drivers/usb/gadget/function/f_tcm.c
b/drivers/usb/gadget/function/f_tcm.c
index a82e2bd..c9d741d 100644
--- a/drivers/usb/gadget/function/f_tcm.c
+++ b/drivers/usb/gadget/function/f_tcm.c
@@ -1145,6 +1145,7 @@ static int usbg_submit_command(struct f_uas *fu,
default:
pr_debug_once("Unsupported prio_attr: %02x.\n",
cmd_iu->prio_attr);
+ /* fall through */
case UAS_SIMPLE_TAG:
cmd->prio_attr = TCM_SIMPLE_TAG;
break;
--
2.7.4
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] usb: core: urb: mark expected switch fall-through
In preparation to enabling -Wimplicit-fallthrough, mark switch cases where we are expecting to fall through. Addresses-Coverity-ID: 1162594 Signed-off-by: Gustavo A. R. Silva <garsi...@embeddedor.com> --- drivers/usb/core/urb.c | 1 + 1 file changed, 1 insertion(+) diff --git a/drivers/usb/core/urb.c b/drivers/usb/core/urb.c index 8b800e3..06e0151 100644 --- a/drivers/usb/core/urb.c +++ b/drivers/usb/core/urb.c @@ -514,6 +514,7 @@ int usb_submit_urb(struct urb *urb, gfp_t mem_flags) if ((urb->interval < 6) && (xfertype == USB_ENDPOINT_XFER_INT)) return -EINVAL; + /* fall through */ default: if (urb->interval <= 0) return -EINVAL; -- 2.7.4 -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] usb: image: mdc800: mark expected switch fall-through
In preparation to enabling -Wimplicit-fallthrough, mark switch cases where we are expecting to fall through. Signed-off-by: Gustavo A. R. Silva <garsi...@embeddedor.com> --- drivers/usb/image/mdc800.c | 1 + 1 file changed, 1 insertion(+) diff --git a/drivers/usb/image/mdc800.c b/drivers/usb/image/mdc800.c index e92540a..185c4e2 100644 --- a/drivers/usb/image/mdc800.c +++ b/drivers/usb/image/mdc800.c @@ -893,6 +893,7 @@ static ssize_t mdc800_device_write (struct file *file, const char __user *buf, s return -EIO; } mdc800->pic_len=-1; + /* fall through */ case 0x09: /* Download Thumbnail */ mdc800->download_left=answersize+64; -- 2.7.4 -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] usb: phy: phy-msm-usb: mark expected switch fall-through
In preparation to enabling -Wimplicit-fallthrough, mark switch cases where we are expecting to fall through. Addresses-Coverity-ID: 1222118 Signed-off-by: Gustavo A. R. Silva <garsi...@embeddedor.com> --- drivers/usb/phy/phy-msm-usb.c | 1 + 1 file changed, 1 insertion(+) diff --git a/drivers/usb/phy/phy-msm-usb.c b/drivers/usb/phy/phy-msm-usb.c index 3d0dd2f..8bc3403 100644 --- a/drivers/usb/phy/phy-msm-usb.c +++ b/drivers/usb/phy/phy-msm-usb.c @@ -1261,6 +1261,7 @@ static void msm_chg_detect_work(struct work_struct *w) /* fall through */ case USB_CHG_STATE_SECONDARY_DONE: motg->chg_state = USB_CHG_STATE_DETECTED; + /* fall through */ case USB_CHG_STATE_DETECTED: msm_chg_block_off(motg); dev_dbg(phy->dev, "charger = %d\n", motg->chg_type); -- 2.7.4 -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] usb: misc: ftdi-elan: fix duplicated code for different branches
Refactor code in order to avoid identical code for different branches.
This issue was detected with the help of Coccinelle.
Signed-off-by: Gustavo A. R. Silva <gust...@embeddedor.com>
---
drivers/usb/misc/ftdi-elan.c | 27 +--
1 file changed, 1 insertion(+), 26 deletions(-)
diff --git a/drivers/usb/misc/ftdi-elan.c b/drivers/usb/misc/ftdi-elan.c
index c45904f..424ff12 100644
--- a/drivers/usb/misc/ftdi-elan.c
+++ b/drivers/usb/misc/ftdi-elan.c
@@ -857,7 +857,7 @@ static char *have_ed_set_response(struct usb_ftdi *ftdi,
target->actual = 0;
target->non_null = (ed_length >> 15) & 0x0001;
target->repeat_number = (ed_length >> 11) & 0x000F;
- if (ed_type == 0x02) {
+ if (ed_type == 0x02 || ed_type == 0x03) {
if (payload == 0 || target->abandoning > 0) {
target->abandoning = 0;
mutex_unlock(>u132_lock);
@@ -873,31 +873,6 @@ static char *have_ed_set_response(struct usb_ftdi *ftdi,
mutex_unlock(>u132_lock);
return b;
}
- } else if (ed_type == 0x03) {
- if (payload == 0 || target->abandoning > 0) {
- target->abandoning = 0;
- mutex_unlock(>u132_lock);
- ftdi_elan_do_callback(ftdi, target, 4 + ftdi->response,
- payload);
- ftdi->received = 0;
- ftdi->expected = 4;
- ftdi->ed_found = 0;
- return ftdi->response;
- } else {
- ftdi->expected = 4 + payload;
- ftdi->ed_found = 1;
- mutex_unlock(>u132_lock);
- return b;
- }
- } else if (ed_type == 0x01) {
- target->abandoning = 0;
- mutex_unlock(>u132_lock);
- ftdi_elan_do_callback(ftdi, target, 4 + ftdi->response,
- payload);
- ftdi->received = 0;
- ftdi->expected = 4;
- ftdi->ed_found = 0;
- return ftdi->response;
} else {
target->abandoning = 0;
mutex_unlock(>u132_lock);
--
2.5.0
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] usb: gadget: udc: renesas_usb3: fix error return code in renesas_usb3_probe()
Hi Yoshihiro,
On 08/09/2017 06:44 AM, Yoshihiro Shimoda wrote:
Hi Gustavo,
Thank you for the patch!
I'm glad to help :)
-Original Message-
From: Gustavo A. R. Silva
Sent: Wednesday, August 9, 2017 7:35 AM
platform_get_irq() returns an error code, but the renesas_usb3 driver
ignores it and always returns -ENODEV. This is not correct and,
prevents -EPROBE_DEFER from being propagated properly.
Thank you for the point. I got it.
Also, notice that platform_get_irq() no longer returns 0 on error:
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=e330b9a6bb35dc7097a4f02cb1ae7b6f96df92
af
I don't think this explanation needs.
After this is removed,
Acked-by: Yoshihiro Shimoda <yoshihiro.shimoda...@renesas.com>
Thank you
--
Gustavo A. R. Silva
Best regards,
Yoshihiro Shimoda
Print error message and propagate the return value of platform_get_irq
on failure.
This issue was detected with the help of Coccinelle.
Signed-off-by: Gustavo A. R. Silva <gust...@embeddedor.com>
---
drivers/usb/gadget/udc/renesas_usb3.c | 6 --
1 file changed, 4 insertions(+), 2 deletions(-)
diff --git a/drivers/usb/gadget/udc/renesas_usb3.c
b/drivers/usb/gadget/udc/renesas_usb3.c
index e1de8fe..616d053 100644
--- a/drivers/usb/gadget/udc/renesas_usb3.c
+++ b/drivers/usb/gadget/udc/renesas_usb3.c
@@ -2468,8 +2468,10 @@ static int renesas_usb3_probe(struct platform_device
*pdev)
priv = match->data;
irq = platform_get_irq(pdev, 0);
- if (irq < 0)
- return -ENODEV;
+ if (irq < 0) {
+ dev_err(>dev, "Failed to get IRQ: %d\n", irq);
+ return irq;
+ }
usb3 = devm_kzalloc(>dev, sizeof(*usb3), GFP_KERNEL);
if (!usb3)
--
2.5.0
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] usb: ehci-omap: fix error return code in ehci_hcd_omap_probe()
platform_get_irq() returns an error code, but the ehci-omap driver
ignores it and always returns -ENODEV. This is not correct and,
prevents -EPROBE_DEFER from being propagated properly.
Also, notice that platform_get_irq() no longer returns 0 on error:
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=e330b9a6bb35dc7097a4f02cb1ae7b6f96df92af
Print and propagate the return value of platform_get_irq on failure.
This issue was detected with the help of Coccinelle.
Signed-off-by: Gustavo A. R. Silva <gust...@embeddedor.com>
---
drivers/usb/host/ehci-omap.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/usb/host/ehci-omap.c b/drivers/usb/host/ehci-omap.c
index 94ea9ff..4d30853 100644
--- a/drivers/usb/host/ehci-omap.c
+++ b/drivers/usb/host/ehci-omap.c
@@ -130,8 +130,8 @@ static int ehci_hcd_omap_probe(struct platform_device *pdev)
irq = platform_get_irq(pdev, 0);
if (irq < 0) {
- dev_err(dev, "EHCI irq failed\n");
- return -ENODEV;
+ dev_err(dev, "EHCI irq failed: %d\n", irq);
+ return irq;
}
res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
--
2.5.0
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] usb: gadget: udc: renesas_usb3: fix error return code in renesas_usb3_probe()
platform_get_irq() returns an error code, but the renesas_usb3 driver
ignores it and always returns -ENODEV. This is not correct and,
prevents -EPROBE_DEFER from being propagated properly.
Also, notice that platform_get_irq() no longer returns 0 on error:
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=e330b9a6bb35dc7097a4f02cb1ae7b6f96df92af
Print error message and propagate the return value of platform_get_irq
on failure.
This issue was detected with the help of Coccinelle.
Signed-off-by: Gustavo A. R. Silva <gust...@embeddedor.com>
---
drivers/usb/gadget/udc/renesas_usb3.c | 6 --
1 file changed, 4 insertions(+), 2 deletions(-)
diff --git a/drivers/usb/gadget/udc/renesas_usb3.c
b/drivers/usb/gadget/udc/renesas_usb3.c
index e1de8fe..616d053 100644
--- a/drivers/usb/gadget/udc/renesas_usb3.c
+++ b/drivers/usb/gadget/udc/renesas_usb3.c
@@ -2468,8 +2468,10 @@ static int renesas_usb3_probe(struct platform_device
*pdev)
priv = match->data;
irq = platform_get_irq(pdev, 0);
- if (irq < 0)
- return -ENODEV;
+ if (irq < 0) {
+ dev_err(>dev, "Failed to get IRQ: %d\n", irq);
+ return irq;
+ }
usb3 = devm_kzalloc(>dev, sizeof(*usb3), GFP_KERNEL);
if (!usb3)
--
2.5.0
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] usb: dwc3: omap: fix error return code in dwc3_omap_probe()
platform_get_irq() returns an error code, but the dwc3-omap driver
ignores it and always returns -EINVAL. This is not correct and,
prevents -EPROBE_DEFER from being propagated properly.
Notice that platform_get_irq() no longer returns 0 on error:
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=e330b9a6bb35dc7097a4f02cb1ae7b6f96df92af
Print and propagate the return value of platform_get_irq on failure.
This issue was detected with the help of Coccinelle.
Signed-off-by: Gustavo A. R. Silva <gust...@embeddedor.com>
---
drivers/usb/dwc3/dwc3-omap.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/usb/dwc3/dwc3-omap.c b/drivers/usb/dwc3/dwc3-omap.c
index f5aaa0c..3530795 100644
--- a/drivers/usb/dwc3/dwc3-omap.c
+++ b/drivers/usb/dwc3/dwc3-omap.c
@@ -478,8 +478,8 @@ static int dwc3_omap_probe(struct platform_device *pdev)
irq = platform_get_irq(pdev, 0);
if (irq < 0) {
- dev_err(dev, "missing IRQ resource\n");
- return -EINVAL;
+ dev_err(dev, "missing IRQ resource: %d\n", irq);
+ return irq;
}
res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
--
2.5.0
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] usb: imx21-hcd: fix error return code in imx21_probe()
platform_get_irq() returns an error code, but the imx21-hcd driver
ignores it and always returns -ENXIO. This is not correct, and
prevents -EPROBE_DEFER from being propagated properly.
Notice that platform_get_irq() no longer returns 0 on error:
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=e330b9a6bb35dc7097a4f02cb1ae7b6f96df92af
Print error message and propagate the return value of platform_get_irq
on failure.
This issue was detected with the help of Coccinelle.
Signed-off-by: Gustavo A. R. Silva <gust...@embeddedor.com>
---
drivers/usb/host/imx21-hcd.c | 6 --
1 file changed, 4 insertions(+), 2 deletions(-)
diff --git a/drivers/usb/host/imx21-hcd.c b/drivers/usb/host/imx21-hcd.c
index f542045..e25d72e 100644
--- a/drivers/usb/host/imx21-hcd.c
+++ b/drivers/usb/host/imx21-hcd.c
@@ -1849,8 +1849,10 @@ static int imx21_probe(struct platform_device *pdev)
if (!res)
return -ENODEV;
irq = platform_get_irq(pdev, 0);
- if (irq < 0)
- return -ENXIO;
+ if (irq < 0) {
+ dev_err(>dev, "Failed to get IRQ: %d\n", irq);
+ return irq;
+ }
hcd = usb_create_hcd(_hc_driver,
>dev, dev_name(>dev));
--
2.5.0
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] usb: isp1760: compress return logic into one line
Hi Oliver, Quoting Oliver Neukum <oneu...@suse.com>: Am Sonntag, den 09.07.2017, 21:00 -0500 schrieb Gustavo A. R. Silva : Simplify return logic to avoid unnecessary variable assignment. This issue was detected using Coccinelle and the following semantic patch: Hi, I need to ask: Where is the improvement? The compiler does not bother and for the human reader you do not do anything obvious and you decreased grepability. The declaration of local variable _retval_ was removed also. So both, variable declaration and assignment removal are the improvements. Regarding the greability, I think that depends on the context. Thanks! -- Gustavo A. R. Silva -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] usb: gadget: fusb300_udc: compress return logic into one line
Simplify return logic to avoid unnecessary variable declaration
and assignment.
These issues were detected using Coccinelle and the following
semantic patch:
@@
local idexpression ret;
expression e;
@@
-ret =
+return
e;
-return ret;
Signed-off-by: Gustavo A. R. Silva <garsi...@embeddedor.com>
---
drivers/usb/gadget/udc/fusb300_udc.c | 10 ++
1 file changed, 2 insertions(+), 8 deletions(-)
diff --git a/drivers/usb/gadget/udc/fusb300_udc.c
b/drivers/usb/gadget/udc/fusb300_udc.c
index e0c1b00..8738f03 100644
--- a/drivers/usb/gadget/udc/fusb300_udc.c
+++ b/drivers/usb/gadget/udc/fusb300_udc.c
@@ -659,22 +659,16 @@ static void fusb300_rdfifo(struct fusb300_ep *ep,
static u8 fusb300_get_epnstall(struct fusb300 *fusb300, u8 ep)
{
- u8 value;
u32 reg = ioread32(fusb300->reg + FUSB300_OFFSET_EPSET0(ep));
- value = reg & FUSB300_EPSET0_STL;
-
- return value;
+ return reg & FUSB300_EPSET0_STL;
}
static u8 fusb300_get_cxstall(struct fusb300 *fusb300)
{
- u8 value;
u32 reg = ioread32(fusb300->reg + FUSB300_OFFSET_CSR);
- value = (reg & FUSB300_CSR_STL) >> 1;
-
- return value;
+ return (reg & FUSB300_CSR_STL) >> 1;
}
static void request_error(struct fusb300 *fusb300)
--
2.5.0
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] usb: gadget: fsl_udc_core: compress return logic into one line
Simplify return logic to avoid unnecessary variable assignment.
This issue was detected using Coccinelle and the following
semantic patch:
@@
local idexpression ret;
expression e;
@@
-ret =
+return
e;
-return ret;
Signed-off-by: Gustavo A. R. Silva <garsi...@embeddedor.com>
---
drivers/usb/gadget/udc/fsl_udc_core.c | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/drivers/usb/gadget/udc/fsl_udc_core.c
b/drivers/usb/gadget/udc/fsl_udc_core.c
index 6f2f71c..bf8ff5e 100644
--- a/drivers/usb/gadget/udc/fsl_udc_core.c
+++ b/drivers/usb/gadget/udc/fsl_udc_core.c
@@ -1642,8 +1642,7 @@ static int process_ep_req(struct fsl_udc *udc, int pipe,
} else if (hc32_to_cpu(curr_td->size_ioc_sts)
& DTD_STATUS_ACTIVE) {
VDBG("Request not complete");
- status = REQ_UNCOMPLETE;
- return status;
+ return REQ_UNCOMPLETE;
} else if (remaining_length) {
if (direction) {
VDBG("Transmit dTD remaining length not zero");
--
2.5.0
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] usb: gadget: udc-xilinx: compress return logic into one line
Simplify return logic to avoid unnecessary variable assignment.
This issue was detected using Coccinelle and the following
semantic patch:
@@
local idexpression ret;
expression e;
@@
-ret =
+return
e;
-return ret;
Signed-off-by: Gustavo A. R. Silva <garsi...@embeddedor.com>
---
drivers/usb/gadget/udc/udc-xilinx.c | 5 ++---
1 file changed, 2 insertions(+), 3 deletions(-)
diff --git a/drivers/usb/gadget/udc/udc-xilinx.c
b/drivers/usb/gadget/udc/udc-xilinx.c
index de207a9..552389d 100644
--- a/drivers/usb/gadget/udc/udc-xilinx.c
+++ b/drivers/usb/gadget/udc/udc-xilinx.c
@@ -1217,14 +1217,13 @@ static const struct usb_ep_ops xusb_ep_ops = {
static int xudc_get_frame(struct usb_gadget *gadget)
{
struct xusb_udc *udc;
- int frame;
if (!gadget)
return -ENODEV;
udc = to_udc(gadget);
- frame = udc->read_fn(udc->addr + XUSB_FRAMENUM_OFFSET);
- return frame;
+
+ return udc->read_fn(udc->addr + XUSB_FRAMENUM_OFFSET);
}
/**
--
2.5.0
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] usb: chipidea: udc: compress return logic into line
Simplify return logic to avoid unnecessary variable assignment.
This issue was detected using Coccinelle and the following
semantic patch:
@@
local idexpression ret;
expression e;
@@
-ret =
+return
e;
-return ret;
Signed-off-by: Gustavo A. R. Silva <garsi...@embeddedor.com>
---
drivers/usb/chipidea/udc.c | 5 +
1 file changed, 1 insertion(+), 4 deletions(-)
diff --git a/drivers/usb/chipidea/udc.c b/drivers/usb/chipidea/udc.c
index d68b125..6502c13 100644
--- a/drivers/usb/chipidea/udc.c
+++ b/drivers/usb/chipidea/udc.c
@@ -944,7 +944,6 @@ isr_setup_status_complete(struct usb_ep *ep, struct
usb_request *req)
*/
static int isr_setup_status_phase(struct ci_hdrc *ci)
{
- int retval;
struct ci_hw_ep *hwep;
/*
@@ -960,9 +959,7 @@ static int isr_setup_status_phase(struct ci_hdrc *ci)
ci->status->context = ci;
ci->status->complete = isr_setup_status_complete;
- retval = _ep_queue(>ep, ci->status, GFP_ATOMIC);
-
- return retval;
+ return _ep_queue(>ep, ci->status, GFP_ATOMIC);
}
/**
--
2.5.0
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] usb: gadget: legacy: compress return logic into one line
Simplify return logic to avoid unnecessary variable assignments.
These issues were detected using Coccinelle and the following
semantic patch:
@@
local idexpression ret;
expression e;
@@
-ret =
+return
e;
-return ret;
Signed-off-by: Gustavo A. R. Silva <garsi...@embeddedor.com>
---
drivers/usb/gadget/legacy/audio.c | 12
1 file changed, 4 insertions(+), 8 deletions(-)
diff --git a/drivers/usb/gadget/legacy/audio.c
b/drivers/usb/gadget/legacy/audio.c
index 1f5cdbe..c02f95e 100644
--- a/drivers/usb/gadget/legacy/audio.c
+++ b/drivers/usb/gadget/legacy/audio.c
@@ -200,10 +200,8 @@ static int audio_do_config(struct usb_configuration *c)
#ifdef CONFIG_GADGET_UAC1
f_uac1 = usb_get_function(fi_uac1);
- if (IS_ERR(f_uac1)) {
- status = PTR_ERR(f_uac1);
- return status;
- }
+ if (IS_ERR(f_uac1))
+ return PTR_ERR(f_uac1);
status = usb_add_function(c, f_uac1);
if (status < 0) {
@@ -212,10 +210,8 @@ static int audio_do_config(struct usb_configuration *c)
}
#else
f_uac2 = usb_get_function(fi_uac2);
- if (IS_ERR(f_uac2)) {
- status = PTR_ERR(f_uac2);
- return status;
- }
+ if (IS_ERR(f_uac2))
+ return PTR_ERR(f_uac2);
status = usb_add_function(c, f_uac2);
if (status < 0) {
--
2.5.0
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 1/2] usb: gadget: compress return logic into one line
Simplify return logic to avoid unnecessary variable declaration
and assignment.
This issue was detected using Coccinelle and the following
semantic patch:
@@
local idexpression ret;
expression e;
@@
-ret =
+return
e;
-return ret;
Signed-off-by: Gustavo A. R. Silva <garsi...@embeddedor.com>
---
drivers/usb/gadget/config.c | 4 +---
1 file changed, 1 insertion(+), 3 deletions(-)
diff --git a/drivers/usb/gadget/config.c b/drivers/usb/gadget/config.c
index 17a6077..23bfa7d 100644
--- a/drivers/usb/gadget/config.c
+++ b/drivers/usb/gadget/config.c
@@ -207,7 +207,6 @@ EXPORT_SYMBOL_GPL(usb_free_all_descriptors);
struct usb_descriptor_header *usb_otg_descriptor_alloc(
struct usb_gadget *gadget)
{
- struct usb_descriptor_header *otg_desc;
unsigned length = 0;
if (gadget->otg_caps && (gadget->otg_caps->otg_rev >= 0x0200))
@@ -215,8 +214,7 @@ struct usb_descriptor_header *usb_otg_descriptor_alloc(
else
length = sizeof(struct usb_otg_descriptor);
- otg_desc = kzalloc(length, GFP_KERNEL);
- return otg_desc;
+ return kzalloc(length, GFP_KERNEL);
}
EXPORT_SYMBOL_GPL(usb_otg_descriptor_alloc);
--
2.5.0
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 2/2] usb: gadget: use unsigned int instead of unsigned in usb_otg_descriptor_alloc()
Prefer unsigned int to bare use of unsigned.
Signed-off-by: Gustavo A. R. Silva <garsi...@embeddedor.com>
---
drivers/usb/gadget/config.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/usb/gadget/config.c b/drivers/usb/gadget/config.c
index 23bfa7d..968179d 100644
--- a/drivers/usb/gadget/config.c
+++ b/drivers/usb/gadget/config.c
@@ -207,7 +207,7 @@ EXPORT_SYMBOL_GPL(usb_free_all_descriptors);
struct usb_descriptor_header *usb_otg_descriptor_alloc(
struct usb_gadget *gadget)
{
- unsigned length = 0;
+ unsigned int length = 0;
if (gadget->otg_caps && (gadget->otg_caps->otg_rev >= 0x0200))
length = sizeof(struct usb_otg20_descriptor);
--
2.5.0
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] usb: gadget: legacy: ncm: compress return logic into one line
Simplify return logic to avoid unnecessary variable assignment.
This issue was detected using Coccinelle and the following
semantic patch:
@@
local idexpression ret;
expression e;
@@
-ret =
+return
e;
-return ret;
Signed-off-by: Gustavo A. R. Silva <garsi...@embeddedor.com>
---
drivers/usb/gadget/legacy/ncm.c | 6 ++
1 file changed, 2 insertions(+), 4 deletions(-)
diff --git a/drivers/usb/gadget/legacy/ncm.c b/drivers/usb/gadget/legacy/ncm.c
index 0aba682..cb0d84f 100644
--- a/drivers/usb/gadget/legacy/ncm.c
+++ b/drivers/usb/gadget/legacy/ncm.c
@@ -106,10 +106,8 @@ static int ncm_do_config(struct usb_configuration *c)
}
f_ncm = usb_get_function(f_ncm_inst);
- if (IS_ERR(f_ncm)) {
- status = PTR_ERR(f_ncm);
- return status;
- }
+ if (IS_ERR(f_ncm))
+ return PTR_ERR(f_ncm);
status = usb_add_function(c, f_ncm);
if (status < 0) {
--
2.5.0
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] usb: gadget: legacy: dbgp: compress return logic into one line
Simplify return logic to avoid unnecessary variable declaration
and assignment.
This issue was detected using Coccinelle and the following
semantic patch:
@@
local idexpression ret;
expression e;
@@
-ret =
+return
e;
-return ret;
Signed-off-by: Gustavo A. R. Silva <garsi...@embeddedor.com>
---
drivers/usb/gadget/legacy/dbgp.c | 5 ++---
1 file changed, 2 insertions(+), 3 deletions(-)
diff --git a/drivers/usb/gadget/legacy/dbgp.c b/drivers/usb/gadget/legacy/dbgp.c
index 99ca3da..f14a5d7 100644
--- a/drivers/usb/gadget/legacy/dbgp.c
+++ b/drivers/usb/gadget/legacy/dbgp.c
@@ -165,10 +165,9 @@ static int dbgp_enable_ep_req(struct usb_ep *ep)
static int __enable_ep(struct usb_ep *ep, struct usb_endpoint_descriptor *desc)
{
- int err;
ep->desc = desc;
- err = usb_ep_enable(ep);
- return err;
+
+ return usb_ep_enable(ep);
}
static int dbgp_enable_ep(void)
--
2.5.0
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] usb: gadget: configfs: compress return logic into one line
Simplify return logic to avoid unnecessary variable assignment.
This issue was detected using Coccinelle and the following
semantic patch:
@@
local idexpression ret;
expression e;
@@
-ret =
+return
e;
-return ret;
Signed-off-by: Gustavo A. R. Silva <garsi...@embeddedor.com>
---
drivers/usb/gadget/configfs.c | 5 +
1 file changed, 1 insertion(+), 4 deletions(-)
diff --git a/drivers/usb/gadget/configfs.c b/drivers/usb/gadget/configfs.c
index a22a892..00b4454 100644
--- a/drivers/usb/gadget/configfs.c
+++ b/drivers/usb/gadget/configfs.c
@@ -1502,12 +1502,9 @@ EXPORT_SYMBOL_GPL(unregister_gadget_item);
static int __init gadget_cfs_init(void)
{
- int ret;
-
config_group_init(_subsys.su_group);
- ret = configfs_register_subsystem(_subsys);
- return ret;
+ return configfs_register_subsystem(_subsys);
}
module_init(gadget_cfs_init);
--
2.5.0
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] usb: isp1760: compress return logic into one line
Simplify return logic to avoid unnecessary variable assignment.
This issue was detected using Coccinelle and the following
semantic patch:
@@
local idexpression ret;
expression e;
@@
-ret =
+return
e;
-return ret;
Signed-off-by: Gustavo A. R. Silva <garsi...@embeddedor.com>
---
drivers/usb/isp1760/isp1760-hcd.c | 6 ++
1 file changed, 2 insertions(+), 4 deletions(-)
diff --git a/drivers/usb/isp1760/isp1760-hcd.c
b/drivers/usb/isp1760/isp1760-hcd.c
index ac31d19..8e59e0c 100644
--- a/drivers/usb/isp1760/isp1760-hcd.c
+++ b/drivers/usb/isp1760/isp1760-hcd.c
@@ -396,7 +396,6 @@ static int handshake(struct usb_hcd *hcd, u32 reg,
/* reset a non-running (STS_HALT == 1) controller */
static int ehci_reset(struct usb_hcd *hcd)
{
- int retval;
struct isp1760_hcd *priv = hcd_to_priv(hcd);
u32 command = reg_read32(hcd->regs, HC_USBCMD);
@@ -405,9 +404,8 @@ static int ehci_reset(struct usb_hcd *hcd)
reg_write32(hcd->regs, HC_USBCMD, command);
hcd->state = HC_STATE_HALT;
priv->next_statechange = jiffies;
- retval = handshake(hcd, HC_USBCMD,
- CMD_RESET, 0, 250 * 1000);
- return retval;
+
+ return handshake(hcd, HC_USBCMD, CMD_RESET, 0, 250 * 1000);
}
static struct isp1760_qh *qh_alloc(gfp_t flags)
--
2.5.0
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] usb: gadget: function: constify snd_pcm_ops structure
Check for snd_pcm_ops structures that are only stored in the ops field of
a snd_soc_platform_driver structure or passed as the third argument to
snd_pcm_set_ops. The corresponding field or parameter is declared const,
so snd_pcm_ops structures that have this property can be declared as
const also.
This issue was detected using Coccinelle and the following semantic patch:
@r disable optional_qualifier@
identifier i;
position p;
@@
static struct snd_pcm_ops i@p = { ... };
@ok1@
identifier r.i;
struct snd_soc_platform_driver e;
position p;
@@
e.ops = @p;
@ok2@
identifier r.i;
expression e1, e2; position p;
@@
snd_pcm_set_ops(e1, e2, @p)
@bad@
position p != {r.p,ok1.p,ok2.p};
identifier r.i;
struct snd_pcm_ops e;
@@
e@i@p
@depends on !bad disable optional_qualifier@
identifier r.i;
@@
static
+const
struct snd_pcm_ops i = { ... };
Signed-off-by: Gustavo A. R. Silva <garsi...@embeddedor.com>
---
drivers/usb/gadget/function/u_audio.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/usb/gadget/function/u_audio.c
b/drivers/usb/gadget/function/u_audio.c
index 5dd73b9..d4caa21 100644
--- a/drivers/usb/gadget/function/u_audio.c
+++ b/drivers/usb/gadget/function/u_audio.c
@@ -354,7 +354,7 @@ static int uac_pcm_null(struct snd_pcm_substream *substream)
return 0;
}
-static struct snd_pcm_ops uac_pcm_ops = {
+static const struct snd_pcm_ops uac_pcm_ops = {
.open = uac_pcm_open,
.close = uac_pcm_null,
.ioctl = snd_pcm_lib_ioctl,
--
2.5.0
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] usb: gadget: udc-xilinx: remove useless variable assignment in xudc_read_fifo()
Value assigned to variable bufferspace at line 637 is overwritten
at line 613, before it can be used. This makes such variable
assignment useless.
Addresses-Coverity-ID: 1397677
Signed-off-by: Gustavo A. R. Silva <garsi...@embeddedor.com>
---
drivers/usb/gadget/udc/udc-xilinx.c | 1 -
1 file changed, 1 deletion(-)
diff --git a/drivers/usb/gadget/udc/udc-xilinx.c
b/drivers/usb/gadget/udc/udc-xilinx.c
index de207a9..7afdf8e 100644
--- a/drivers/usb/gadget/udc/udc-xilinx.c
+++ b/drivers/usb/gadget/udc/udc-xilinx.c
@@ -634,7 +634,6 @@ static int xudc_read_fifo(struct xusb_ep *ep, struct
xusb_req *req)
dev_dbg(udc->dev, "read %s, %d bytes%s req %p %d/%d\n",
ep->ep_usb.name, count, is_short ? "/S" : "", req,
req->usb_req.actual, req->usb_req.length);
- bufferspace -= count;
/* Completion */
if ((req->usb_req.actual == req->usb_req.length) || is_short) {
if (udc->dma_enabled && req->usb_req.length)
--
2.5.0
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] usb: musb: compress return logic into one line
Simplify return logic to avoid unnecessary variable assignment. Signed-off-by: Gustavo A. R. Silva <garsi...@embeddedor.com> --- drivers/usb/musb/musb_host.c | 6 ++ 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/drivers/usb/musb/musb_host.c b/drivers/usb/musb/musb_host.c index dbe617a..76decb8 100644 --- a/drivers/usb/musb/musb_host.c +++ b/drivers/usb/musb/musb_host.c @@ -1540,7 +1540,7 @@ static int musb_rx_dma_iso_cppi41(struct dma_controller *dma, struct dma_channel *channel = hw_ep->rx_channel; void __iomem *epio = hw_ep->regs; dma_addr_t *buf; - u32 length, res; + u32 length; u16 val; buf = (void *)urb->iso_frame_desc[qh->iso_idx].offset + @@ -1552,10 +1552,8 @@ static int musb_rx_dma_iso_cppi41(struct dma_controller *dma, val |= MUSB_RXCSR_DMAENAB; musb_writew(hw_ep->regs, MUSB_RXCSR, val); - res = dma->channel_program(channel, qh->maxpacket, 0, + return dma->channel_program(channel, qh->maxpacket, 0, (u32)buf, length); - - return res; } #else static inline int musb_rx_dma_iso_cppi41(struct dma_controller *dma, -- 2.5.0 -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] uwb: i1480: add missing goto
Add missing goto.
Addresses-Coverity-ID: 1226913
Signed-off-by: Gustavo A. R. Silva <garsi...@embeddedor.com>
---
drivers/uwb/i1480/dfu/phy.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/drivers/uwb/i1480/dfu/phy.c b/drivers/uwb/i1480/dfu/phy.c
index 3b1a87d..1ac8526 100644
--- a/drivers/uwb/i1480/dfu/phy.c
+++ b/drivers/uwb/i1480/dfu/phy.c
@@ -126,6 +126,7 @@ int i1480_mpi_read(struct i1480 *i1480, u8 *data, u16
srcaddr, size_t size)
dev_err(i1480->dev, "MPI-READ: command execution failed: %d\n",
reply->bResultCode);
result = -EIO;
+ goto out;
}
for (cnt = 0; cnt < size; cnt++) {
if (reply->data[cnt].page != (srcaddr + cnt) >> 8)
--
2.5.0
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [uwb-i1480] question about value overwrite
Hi Greg,
Quoting Greg KH <gre...@linuxfoundation.org>:
On Thu, May 18, 2017 at 06:00:06PM -0500, Gustavo A. R. Silva wrote:
Hello everybody,
While looking into Coverity ID 1226913 I ran into the following piece of
code at drivers/uwb/i1480/dfu/phy.c:99:
99static
100int i1480_mpi_read(struct i1480 *i1480, u8 *data, u16 srcaddr,
size_t size)
101{
102int result;
103struct i1480_cmd_mpi_read *cmd = i1480->cmd_buf;
104struct i1480_evt_mpi_read *reply = i1480->evt_buf;
105unsigned cnt;
106
107memset(i1480->cmd_buf, 0x69, 512);
108memset(i1480->evt_buf, 0x69, 512);
109
110BUG_ON(size > (i1480->buf_size - sizeof(*reply)) / 3);
111result = -ENOMEM;
112cmd->rccb.bCommandType = i1480_CET_VS1;
113cmd->rccb.wCommand = cpu_to_le16(i1480_CMD_MPI_READ);
114cmd->size = cpu_to_le16(3*size);
115for (cnt = 0; cnt < size; cnt++) {
116cmd->data[cnt].page = (srcaddr + cnt) >> 8;
117cmd->data[cnt].offset = (srcaddr + cnt) & 0xff;
118}
119reply->rceb.bEventType = i1480_CET_VS1;
120reply->rceb.wEvent = i1480_CMD_MPI_READ;
121result = i1480_cmd(i1480, "MPI-READ", sizeof(*cmd) + 2*size,
122sizeof(*reply) + 3*size);
123if (result < 0)
124goto out;
125if (reply->bResultCode != UWB_RC_RES_SUCCESS) {
126dev_err(i1480->dev, "MPI-READ: command execution failed:
%d\n",
127reply->bResultCode);
128result = -EIO;
129}
130for (cnt = 0; cnt < size; cnt++) {
131if (reply->data[cnt].page != (srcaddr + cnt) >> 8)
132dev_err(i1480->dev, "MPI-READ: page inconsistency
at "
133"index %u: expected 0x%02x, got
0x%02x\n", cnt,
134(srcaddr + cnt) >> 8,
reply->data[cnt].page);
135if (reply->data[cnt].offset != ((srcaddr + cnt)
& 0x00ff))
136dev_err(i1480->dev, "MPI-READ: offset
inconsistency at "
137"index %u: expected 0x%02x, got
0x%02x\n", cnt,
138(srcaddr + cnt) & 0x00ff,
139reply->data[cnt].offset);
140data[cnt] = reply->data[cnt].value;
141}
142result = 0;
143out:
144return result;
145}
The issue is that the value store in variable _result_ at line 128 is
overwritten by the one stored at line 142, before it can be used.
My question is if the original intention was to return this value
inmediately after the assignment at line 128, something like in the
following patch:
index 3b1a87d..1ac8526 100644
--- a/drivers/uwb/i1480/dfu/phy.c
+++ b/drivers/uwb/i1480/dfu/phy.c
@@ -126,6 +126,7 @@ int i1480_mpi_read(struct i1480 *i1480, u8 *data, u16
srcaddr, size_t size)
dev_err(i1480->dev, "MPI-READ: command execution failed:
%d\n",
reply->bResultCode);
result = -EIO;
+ goto out;
}
for (cnt = 0; cnt < size; cnt++) {
if (reply->data[cnt].page != (srcaddr + cnt) >> 8)
What do you think?
I'd really appreciate any comment on this.
I think you are correct, I'll take a patch to fix this up if you want to
write one :)
Absolutely, I'll send it shortly.
Thanks!
--
Gustavo A. R. Silva
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[uwb-i1480] question about value overwrite
Hello everybody,
While looking into Coverity ID 1226913 I ran into the following piece
of code at drivers/uwb/i1480/dfu/phy.c:99:
99static
100int i1480_mpi_read(struct i1480 *i1480, u8 *data, u16 srcaddr, size_t size)
101{
102int result;
103struct i1480_cmd_mpi_read *cmd = i1480->cmd_buf;
104struct i1480_evt_mpi_read *reply = i1480->evt_buf;
105unsigned cnt;
106
107memset(i1480->cmd_buf, 0x69, 512);
108memset(i1480->evt_buf, 0x69, 512);
109
110BUG_ON(size > (i1480->buf_size - sizeof(*reply)) / 3);
111result = -ENOMEM;
112cmd->rccb.bCommandType = i1480_CET_VS1;
113cmd->rccb.wCommand = cpu_to_le16(i1480_CMD_MPI_READ);
114cmd->size = cpu_to_le16(3*size);
115for (cnt = 0; cnt < size; cnt++) {
116cmd->data[cnt].page = (srcaddr + cnt) >> 8;
117cmd->data[cnt].offset = (srcaddr + cnt) & 0xff;
118}
119reply->rceb.bEventType = i1480_CET_VS1;
120reply->rceb.wEvent = i1480_CMD_MPI_READ;
121result = i1480_cmd(i1480, "MPI-READ", sizeof(*cmd) + 2*size,
122sizeof(*reply) + 3*size);
123if (result < 0)
124goto out;
125if (reply->bResultCode != UWB_RC_RES_SUCCESS) {
126dev_err(i1480->dev, "MPI-READ: command execution
failed: %d\n",
127reply->bResultCode);
128result = -EIO;
129}
130for (cnt = 0; cnt < size; cnt++) {
131if (reply->data[cnt].page != (srcaddr + cnt) >> 8)
132dev_err(i1480->dev, "MPI-READ: page
inconsistency at "
133"index %u: expected 0x%02x, got
0x%02x\n", cnt,
134(srcaddr + cnt) >> 8,
reply->data[cnt].page);
135if (reply->data[cnt].offset != ((srcaddr + cnt) & 0x00ff))
136dev_err(i1480->dev, "MPI-READ: offset
inconsistency at "
137"index %u: expected 0x%02x, got
0x%02x\n", cnt,
138(srcaddr + cnt) & 0x00ff,
139reply->data[cnt].offset);
140data[cnt] = reply->data[cnt].value;
141}
142result = 0;
143out:
144return result;
145}
The issue is that the value store in variable _result_ at line 128 is
overwritten by the one stored at line 142, before it can be used.
My question is if the original intention was to return this value
inmediately after the assignment at line 128, something like in the
following patch:
index 3b1a87d..1ac8526 100644
--- a/drivers/uwb/i1480/dfu/phy.c
+++ b/drivers/uwb/i1480/dfu/phy.c
@@ -126,6 +126,7 @@ int i1480_mpi_read(struct i1480 *i1480, u8 *data,
u16 srcaddr, size_t size)
dev_err(i1480->dev, "MPI-READ: command execution
failed: %d\n",
reply->bResultCode);
result = -EIO;
+ goto out;
}
for (cnt = 0; cnt < size; cnt++) {
if (reply->data[cnt].page != (srcaddr + cnt) >> 8)
What do you think?
I'd really appreciate any comment on this.
Thank you!
--
Gustavo A. R. Silva
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [usb-gadget-udc] question about null check after calling phys_to_virt() function
Hi Felipe,
Quoting Felipe Balbi <ba...@kernel.org>:
Hi,
"Gustavo A. R. Silva" <garsi...@embeddedor.com> writes:
Hello everybody,
While looking into Coverity ID 145958 I ran into the following piece
of code at drivers/usb/gadget/udc/amd5536udc.c:852:
} else if (i == buf_len) {
/* first td */
td = (struct udc_data_dma *)phys_to_virt(
req->td_data->next);
td->status = 0;
} else {
td = (struct udc_data_dma *)phys_to_virt(last->next);
td->status = 0;
}
if (td)
td->bufptr = req->req.dma + i; /* assign buffer */
else
break;
The issue here is that _td_ pointer is being dereferenced before null check.
After searching for calls to phys_to_virt() function, I've noticed
that is not common at all to test the returned address value.
So either the null check at line 862 is not needed or a null check
before each td->status = 0; needs to be added.
just remove the previous null check
I get it.
Thanks!
--
Gustavo A. R. Silva
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[usb-gadget-udc] question about null check after calling phys_to_virt() function
Hello everybody,
While looking into Coverity ID 145958 I ran into the following piece
of code at drivers/usb/gadget/udc/amd5536udc.c:852:
} else if (i == buf_len) {
/* first td */
td = (struct udc_data_dma *)phys_to_virt(
req->td_data->next);
td->status = 0;
} else {
td = (struct udc_data_dma *)phys_to_virt(last->next);
td->status = 0;
}
if (td)
td->bufptr = req->req.dma + i; /* assign buffer */
else
break;
The issue here is that _td_ pointer is being dereferenced before null check.
After searching for calls to phys_to_virt() function, I've noticed
that is not common at all to test the returned address value.
So either the null check at line 862 is not needed or a null check
before each td->status = 0; needs to be added.
I think it would be good to apply a patch like the following one:
- td->status = 0;
+
+ if (td)
+ td->status = 0;
+ else
+ break;
} else {
td = (struct udc_data_dma *)phys_to_virt(last->next);
- td->status = 0;
+
+ if (td)
+ td->status = 0;
+ else
+ break;
}
- if (td)
- td->bufptr = req->req.dma + i; /* assign buffer */
- else
- break;
+ td->bufptr = req->req.dma + i; /* assign buffer */
What do you think?
Thanks in advance for your comments
--
Gustavo A. R. Silva
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v2] usb: gadget: udc: add null check before pointer dereference
Add null check before dereferencing dev->regs pointer inside
net2280_led_shutdown() function.
Addresses-Coverity-ID: 101783
Cc: Alan Stern <st...@rowland.harvard.edu>
Signed-off-by: Gustavo A. R. Silva <garsi...@embeddedor.com>
---
Changes in v2:
Move the net2280_led_shutdown() call later.
drivers/usb/gadget/udc/net2280.c | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)
diff --git a/drivers/usb/gadget/udc/net2280.c b/drivers/usb/gadget/udc/net2280.c
index 3828c2e..4f5c0c4 100644
--- a/drivers/usb/gadget/udc/net2280.c
+++ b/drivers/usb/gadget/udc/net2280.c
@@ -3573,7 +3573,6 @@ static void net2280_remove(struct pci_dev *pdev)
BUG_ON(dev->driver);
/* then clean up the resources we allocated during probe() */
- net2280_led_shutdown(dev);
if (dev->requests) {
int i;
for (i = 1; i < 5; i++) {
@@ -3588,8 +3587,10 @@ static void net2280_remove(struct pci_dev *pdev)
free_irq(pdev->irq, dev);
if (dev->quirks & PLX_PCIE)
pci_disable_msi(pdev);
- if (dev->regs)
+ if (dev->regs) {
+ net2280_led_shutdown(dev);
iounmap(dev->regs);
+ }
if (dev->region)
release_mem_region(pci_resource_start(pdev, 0),
pci_resource_len(pdev, 0));
--
2.5.0
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] usb: host: remove unnecessary null check
Remove unnecessary null check. udev->tt cannot ever be NULL when this section of code runs. Addresses-Coverity-ID: 100828 Cc: Alan Stern <st...@rowland.harvard.edu> Signed-off-by: Gustavo A. R. Silva <garsi...@embeddedor.com> --- drivers/usb/host/ehci-sched.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/usb/host/ehci-sched.c b/drivers/usb/host/ehci-sched.c index 980a6b3..6bc6304 100644 --- a/drivers/usb/host/ehci-sched.c +++ b/drivers/usb/host/ehci-sched.c @@ -1105,7 +1105,7 @@ iso_stream_init( addr |= epnum << 8; addr |= dev->devnum; stream->ps.usecs = HS_USECS_ISO(maxp); - think_time = dev->tt ? dev->tt->think_time : 0; + think_time = dev->tt->think_time; stream->ps.tt_usecs = NS_TO_US(think_time + usb_calc_bus_time( dev->speed, is_input, 1, maxp)); hs_transfers = max(1u, (maxp + 187) / 188); -- 2.5.0 -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] usb: gadget: udc: add null check before pointer dereference
Hi Alan,
Quoting Alan Stern <st...@rowland.harvard.edu>:
On Mon, 1 May 2017, Gustavo A. R. Silva wrote:
Add null check before dereferencing dev->regs pointer inside
net2280_led_shutdown() function.
Addresses-Coverity-ID: 101783
Signed-off-by: Gustavo A. R. Silva <garsi...@embeddedor.com>
---
drivers/usb/gadget/udc/net2280.c | 8 +---
1 file changed, 5 insertions(+), 3 deletions(-)
diff --git a/drivers/usb/gadget/udc/net2280.c
b/drivers/usb/gadget/udc/net2280.c
index 3828c2e..1898a4b 100644
--- a/drivers/usb/gadget/udc/net2280.c
+++ b/drivers/usb/gadget/udc/net2280.c
@@ -3573,7 +3573,11 @@ static void net2280_remove(struct pci_dev *pdev)
BUG_ON(dev->driver);
/* then clean up the resources we allocated during probe() */
- net2280_led_shutdown(dev);
+
+ if (dev->regs) {
+ net2280_led_shutdown(dev);
+ iounmap(dev->regs);
+ }
if (dev->requests) {
int i;
for (i = 1; i < 5; i++) {
@@ -3588,8 +3592,6 @@ static void net2280_remove(struct pci_dev *pdev)
free_irq(pdev->irq, dev);
if (dev->quirks & PLX_PCIE)
pci_disable_msi(pdev);
- if (dev->regs)
- iounmap(dev->regs);
if (dev->region)
release_mem_region(pci_resource_start(pdev, 0),
pci_resource_len(pdev, 0));
No, you must not move the iounmap() call, because an interrupt could
theoretically occur at any time.
Yeah, I was suspicious about it.
Either just live with an extra test of dev->regs, or else move the
net2280_led_shutdown() call later.
In this case I think it is safe to move the net2280_led_shutdown()
call, as the function is only turning off the LEDs.
I'll send a patch shortly.
Thank you
--
Gustavo A. R. Silva
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [usb-host] question about pointer dereference before null check
Hi Alan, Quoting Alan Stern <st...@rowland.harvard.edu>: On Mon, 1 May 2017, Gustavo A. R. Silva wrote: Hello everybody, While taking a look into Coverity ID 100828 I ran into the following piece of code at drivers/usb/host/ehci-sched.c:1096: u32 addr; int think_time; int hs_transfers; addr = dev->ttport << 24; if (!ehci_is_TDI(ehci) || (dev->tt->hub != ehci_to_hcd(ehci)->self.root_hub)) addr |= dev->tt->hub->devnum << 16; addr |= epnum << 8; addr |= dev->devnum; stream->ps.usecs = HS_USECS_ISO(maxp); think_time = dev->tt ? dev->tt->think_time : 0; The issue here is that dev->tt is being dereferenced before null check. I was thinking on placing think_time = dev->tt ? dev->tt->think_time : 0; just before the _if_ statement. But this doesn't address the problem of dev->tt actually being NULL. While looking into the callers of the function containing this piece of code (iso_stream_init()) my impression is that dev->tt is not NULL at the time this function is called and, a very simple patch like the following can be applied in order to avoid the Coverity issue: -think_time = dev->tt ? dev->tt->think_time : 0; +think_time = dev->tt->think_time; But I can't tell for sure, so in case dev->tt is NULL, a good strategy to properly update the _addr_ variable would be needed. What do you think? I would really appreciate any comment on this, Thank you! You are right; udev->tt cannot ever be NULL when this section of code runs. The test should be removed. Thanks for confirming, I'll send a patch shortly. -- Gustavo A. R. Silva -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] usb: gadget: udc: add null check before pointer dereference
Add null check before dereferencing dev->regs pointer inside
net2280_led_shutdown() function.
Addresses-Coverity-ID: 101783
Signed-off-by: Gustavo A. R. Silva <garsi...@embeddedor.com>
---
drivers/usb/gadget/udc/net2280.c | 8 +---
1 file changed, 5 insertions(+), 3 deletions(-)
diff --git a/drivers/usb/gadget/udc/net2280.c b/drivers/usb/gadget/udc/net2280.c
index 3828c2e..1898a4b 100644
--- a/drivers/usb/gadget/udc/net2280.c
+++ b/drivers/usb/gadget/udc/net2280.c
@@ -3573,7 +3573,11 @@ static void net2280_remove(struct pci_dev *pdev)
BUG_ON(dev->driver);
/* then clean up the resources we allocated during probe() */
- net2280_led_shutdown(dev);
+
+ if (dev->regs) {
+ net2280_led_shutdown(dev);
+ iounmap(dev->regs);
+ }
if (dev->requests) {
int i;
for (i = 1; i < 5; i++) {
@@ -3588,8 +3592,6 @@ static void net2280_remove(struct pci_dev *pdev)
free_irq(pdev->irq, dev);
if (dev->quirks & PLX_PCIE)
pci_disable_msi(pdev);
- if (dev->regs)
- iounmap(dev->regs);
if (dev->region)
release_mem_region(pci_resource_start(pdev, 0),
pci_resource_len(pdev, 0));
--
2.5.0
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[usb-host] question about pointer dereference before null check
Hello everybody, While taking a look into Coverity ID 100828 I ran into the following piece of code at drivers/usb/host/ehci-sched.c:1096: u32 addr; int think_time; int hs_transfers; addr = dev->ttport << 24; if (!ehci_is_TDI(ehci) || (dev->tt->hub != ehci_to_hcd(ehci)->self.root_hub)) addr |= dev->tt->hub->devnum << 16; addr |= epnum << 8; addr |= dev->devnum; stream->ps.usecs = HS_USECS_ISO(maxp); think_time = dev->tt ? dev->tt->think_time : 0; The issue here is that dev->tt is being dereferenced before null check. I was thinking on placing think_time = dev->tt ? dev->tt->think_time : 0; just before the _if_ statement. But this doesn't address the problem of dev->tt actually being NULL. While looking into the callers of the function containing this piece of code (iso_stream_init()) my impression is that dev->tt is not NULL at the time this function is called and, a very simple patch like the following can be applied in order to avoid the Coverity issue: -think_time = dev->tt ? dev->tt->think_time : 0; +think_time = dev->tt->think_time; But I can't tell for sure, so in case dev->tt is NULL, a good strategy to properly update the _addr_ variable would be needed. What do you think? I would really appreciate any comment on this, Thank you! -- Gustavo A. R. Silva -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 2/2] usb: misc: refactor code
Hello,
Quoting Felipe Balbi <felipe.ba...@linux.intel.com>:
Hi,
"Gustavo A. R. Silva" <garsi...@embeddedor.com> writes:
Code refactoring to make the flow easier to follow.
Cc: Alan Stern <st...@rowloand.harvard.edu>
Cc: Greg Kroah-Hartman <gre...@linuxfoundation.org>
Signed-off-by: Gustavo A. R. Silva <garsi...@embeddedor.com>
---
drivers/usb/misc/usbtest.c | 67
+-
1 file changed, 30 insertions(+), 37 deletions(-)
diff --git a/drivers/usb/misc/usbtest.c b/drivers/usb/misc/usbtest.c
index 7bfb6b78..382491e 100644
--- a/drivers/usb/misc/usbtest.c
+++ b/drivers/usb/misc/usbtest.c
@@ -124,18 +124,32 @@ static struct usb_device
*testdev_to_usbdev(struct usbtest_dev *test)
/*-*/
+static inline void endpoint_update(int edi,
+ struct usb_host_endpoint **in,
+ struct usb_host_endpoint **out,
+ struct usb_host_endpoint *e)
+{
+ if (edi) {
+ if (!*in)
+ *in = e;
+ } else {
+ if (!*out)
+ *out = e;
+ }
+}
+
static int
get_endpoints(struct usbtest_dev *dev, struct usb_interface *intf)
{
- int tmp;
- struct usb_host_interface *alt;
- struct usb_host_endpoint*in, *out;
- struct usb_host_endpoint*iso_in, *iso_out;
- struct usb_host_endpoint*int_in, *int_out;
- struct usb_device *udev;
+ int tmp;
+ struct usb_host_interface *alt;
+ struct usb_host_endpoint*in, *out;
+ struct usb_host_endpoint*iso_in, *iso_out;
+ struct usb_host_endpoint*int_in, *int_out;
+ struct usb_device *udev;
unnecessary change
for (tmp = 0; tmp < intf->num_altsetting; tmp++) {
- unsignedep;
+ unsignedep;
unnecessary change
in = out = NULL;
iso_in = iso_out = NULL;
@@ -150,48 +164,27 @@ get_endpoints(struct usbtest_dev *dev, struct
usb_interface *intf)
* ignore other endpoints and altsettings.
*/
for (ep = 0; ep < alt->desc.bNumEndpoints; ep++) {
- struct usb_host_endpoint*e;
+ struct usb_host_endpoint*e;
unnecessary change
I already sent the version 2 of this patch: https://lkml.org/lkml/2017/4/3/856
Thanks
--
Gustavo A. R. Silva
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v2 2/2] usb: gadget: udc: remove unnecessary variable and update function prototype
Remove unnecessary variable and update function prototype.
Acked-by: Michal Nazarewicz <min...@mina86.com>
Reviewed-by: Greg Kroah-Hartman <gre...@linuxfoundation.org>
Signed-off-by: Gustavo A. R. Silva <garsi...@embeddedor.com>
---
Changes in v2:
None.
drivers/usb/gadget/udc/amd5536udc.c | 5 +
1 file changed, 1 insertion(+), 4 deletions(-)
diff --git a/drivers/usb/gadget/udc/amd5536udc.c
b/drivers/usb/gadget/udc/amd5536udc.c
index 821d088..67dd209 100644
--- a/drivers/usb/gadget/udc/amd5536udc.c
+++ b/drivers/usb/gadget/udc/amd5536udc.c
@@ -608,9 +608,8 @@ udc_alloc_request(struct usb_ep *usbep, gfp_t gfp)
}
/* frees pci pool descriptors of a DMA chain */
-static int udc_free_dma_chain(struct udc *dev, struct udc_request *req)
+static void udc_free_dma_chain(struct udc *dev, struct udc_request *req)
{
- int ret_val = 0;
struct udc_data_dma *td = req->td_data;
unsigned int i;
@@ -626,8 +625,6 @@ static int udc_free_dma_chain(struct udc *dev, struct
udc_request *req)
pci_pool_free(dev->data_requests, td, addr);
addr = addr_next;
}
-
- return ret_val;
}
/* Frees request packet, called by gadget driver */
--
2.5.0
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v2 1/2] usb: gadget: udc: avoid use of freed pointer
Rewrite udc_free_dma_chain() function to avoid use of pointer after free.
Addresses-Coverity-ID: 1091172
Acked-by: Michal Nazarewicz <min...@mina86.com>
Reviewed-by: Greg Kroah-Hartman <gre...@linuxfoundation.org>
Signed-off-by: Gustavo A. R. Silva <garsi...@embeddedor.com>
---
Changes in v2:
Remove 'td->next = 0x00' inside for loop.
Remove unnecessary pointer nullification after free.
Rename variable addr_aux to addr_next.
drivers/usb/gadget/udc/amd5536udc.c | 17 -
1 file changed, 8 insertions(+), 9 deletions(-)
diff --git a/drivers/usb/gadget/udc/amd5536udc.c
b/drivers/usb/gadget/udc/amd5536udc.c
index ea03ca7..821d088 100644
--- a/drivers/usb/gadget/udc/amd5536udc.c
+++ b/drivers/usb/gadget/udc/amd5536udc.c
@@ -611,21 +611,20 @@ udc_alloc_request(struct usb_ep *usbep, gfp_t gfp)
static int udc_free_dma_chain(struct udc *dev, struct udc_request *req)
{
int ret_val = 0;
- struct udc_data_dma *td;
- struct udc_data_dma *td_last = NULL;
+ struct udc_data_dma *td = req->td_data;
unsigned int i;
+ dma_addr_t addr_next = 0x00;
+ dma_addr_t addr = (dma_addr_t)td->next;
+
DBG(dev, "free chain req = %p\n", req);
/* do not free first desc., will be done by free for request */
- td_last = req->td_data;
- td = phys_to_virt(td_last->next);
-
for (i = 1; i < req->chain_len; i++) {
- pci_pool_free(dev->data_requests, td,
- (dma_addr_t)td_last->next);
- td_last = td;
- td = phys_to_virt(td_last->next);
+ td = phys_to_virt(addr);
+ addr_next = (dma_addr_t)td->next;
+ pci_pool_free(dev->data_requests, td, addr);
+ addr = addr_next;
}
return ret_val;
--
2.5.0
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v2 1/2] usb: misc: add missing continue in switch
Add missing continue in switch. Addresses-Coverity-ID: 1248733 Cc: Alan Stern <st...@rowloand.harvard.edu> Cc: Greg Kroah-Hartman <gre...@linuxfoundation.org> Signed-off-by: Gustavo A. R. Silva <garsi...@embeddedor.com> --- Changes in v2: None. drivers/usb/misc/usbtest.c | 1 + 1 file changed, 1 insertion(+) diff --git a/drivers/usb/misc/usbtest.c b/drivers/usb/misc/usbtest.c index 3525626..7bfb6b78 100644 --- a/drivers/usb/misc/usbtest.c +++ b/drivers/usb/misc/usbtest.c @@ -159,6 +159,7 @@ get_endpoints(struct usbtest_dev *dev, struct usb_interface *intf) case USB_ENDPOINT_XFER_INT: if (dev->info->intr) goto try_intr; + continue; case USB_ENDPOINT_XFER_ISOC: if (dev->info->iso) goto try_iso; -- 2.5.0 -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v2 2/2] usb: misc: refactor code
Code refactoring to make the flow easier to follow.
Cc: Alan Stern <st...@rowloand.harvard.edu>
Cc: Greg Kroah-Hartman <gre...@linuxfoundation.org>
Signed-off-by: Gustavo A. R. Silva <garsi...@embeddedor.com>
---
Changes in v2:
Remove unfruitful changes in previous patch.
Revert change to comment.
drivers/usb/misc/usbtest.c | 49 --
1 file changed, 21 insertions(+), 28 deletions(-)
diff --git a/drivers/usb/misc/usbtest.c b/drivers/usb/misc/usbtest.c
index 7bfb6b78..88f627e 100644
--- a/drivers/usb/misc/usbtest.c
+++ b/drivers/usb/misc/usbtest.c
@@ -124,6 +124,20 @@ static struct usb_device *testdev_to_usbdev(struct
usbtest_dev *test)
/*-*/
+static inline void endpoint_update(int edi,
+ struct usb_host_endpoint **in,
+ struct usb_host_endpoint **out,
+ struct usb_host_endpoint *e)
+{
+ if (edi) {
+ if (!*in)
+ *in = e;
+ } else {
+ if (!*out)
+ *out = e;
+ }
+}
+
static int
get_endpoints(struct usbtest_dev *dev, struct usb_interface *intf)
{
@@ -151,47 +165,26 @@ get_endpoints(struct usbtest_dev *dev, struct
usb_interface *intf)
*/
for (ep = 0; ep < alt->desc.bNumEndpoints; ep++) {
struct usb_host_endpoint*e;
+ int edi;
e = alt->endpoint + ep;
+ edi = usb_endpoint_dir_in(>desc);
+
switch (usb_endpoint_type(>desc)) {
case USB_ENDPOINT_XFER_BULK:
- break;
+ endpoint_update(edi, , , e);
+ continue;
case USB_ENDPOINT_XFER_INT:
if (dev->info->intr)
- goto try_intr;
+ endpoint_update(edi, _in, _out,
e);
continue;
case USB_ENDPOINT_XFER_ISOC:
if (dev->info->iso)
- goto try_iso;
+ endpoint_update(edi, _in, _out,
e);
/* FALLTHROUGH */
default:
continue;
}
- if (usb_endpoint_dir_in(>desc)) {
- if (!in)
- in = e;
- } else {
- if (!out)
- out = e;
- }
- continue;
-try_intr:
- if (usb_endpoint_dir_in(>desc)) {
- if (!int_in)
- int_in = e;
- } else {
- if (!int_out)
- int_out = e;
- }
- continue;
-try_iso:
- if (usb_endpoint_dir_in(>desc)) {
- if (!iso_in)
- iso_in = e;
- } else {
- if (!iso_out)
- iso_out = e;
- }
}
if ((in && out) || iso_in || iso_out || int_in || int_out)
goto found;
--
2.5.0
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 2/2] usb: misc: refactor code
Hi Alan,
Quoting Alan Stern <st...@rowland.harvard.edu>:
On Mon, 3 Apr 2017, Gustavo A. R. Silva wrote:
Code refactoring to make the flow easier to follow.
Cc: Alan Stern <st...@rowloand.harvard.edu>
Cc: Greg Kroah-Hartman <gre...@linuxfoundation.org>
Signed-off-by: Gustavo A. R. Silva <garsi...@embeddedor.com>
---
drivers/usb/misc/usbtest.c | 67
+-
1 file changed, 30 insertions(+), 37 deletions(-)
diff --git a/drivers/usb/misc/usbtest.c b/drivers/usb/misc/usbtest.c
index 7bfb6b78..382491e 100644
--- a/drivers/usb/misc/usbtest.c
+++ b/drivers/usb/misc/usbtest.c
@@ -124,18 +124,32 @@ static struct usb_device
*testdev_to_usbdev(struct usbtest_dev *test)
/*-*/
+static inline void endpoint_update(int edi,
+ struct usb_host_endpoint **in,
+ struct usb_host_endpoint **out,
+ struct usb_host_endpoint *e)
+{
+ if (edi) {
+ if (!*in)
+ *in = e;
+ } else {
+ if (!*out)
+ *out = e;
+ }
+}
+
static int
get_endpoints(struct usbtest_dev *dev, struct usb_interface *intf)
{
- int tmp;
- struct usb_host_interface *alt;
- struct usb_host_endpoint*in, *out;
- struct usb_host_endpoint*iso_in, *iso_out;
- struct usb_host_endpoint*int_in, *int_out;
- struct usb_device *udev;
+ int tmp;
+ struct usb_host_interface *alt;
+ struct usb_host_endpoint*in, *out;
+ struct usb_host_endpoint*iso_in, *iso_out;
+ struct usb_host_endpoint*int_in, *int_out;
+ struct usb_device *udev;
What's the difference between these 6 lines you added and the 6 lines
that were there originally?
Yeah, well, certainly none at all. This is what happened: I created a
local copy of my changes(this piece of code included) and fixed some
issues in a previous patch, then I did a git revert and moved my
changes back to the original file. During this process the tabs were
replaced with spaces in the original file, then I had to add the tabs
again and this was the resulting patch.
for (tmp = 0; tmp < intf->num_altsetting; tmp++) {
- unsignedep;
+ unsignedep;
And here?
Same as above.
in = out = NULL;
iso_in = iso_out = NULL;
@@ -150,48 +164,27 @@ get_endpoints(struct usbtest_dev *dev, struct
usb_interface *intf)
* ignore other endpoints and altsettings.
*/
for (ep = 0; ep < alt->desc.bNumEndpoints; ep++) {
- struct usb_host_endpoint*e;
+ struct usb_host_endpoint*e;
And here?
Same as above.
+ int edi;
e = alt->endpoint + ep;
+ edi = usb_endpoint_dir_in(>desc);
+
switch (usb_endpoint_type(>desc)) {
case USB_ENDPOINT_XFER_BULK:
- break;
+ endpoint_update(edi, , , e);
+ continue;
case USB_ENDPOINT_XFER_INT:
if (dev->info->intr)
- goto try_intr;
+ endpoint_update(edi, _in, _out,
e);
continue;
case USB_ENDPOINT_XFER_ISOC:
if (dev->info->iso)
- goto try_iso;
- /* FALLTHROUGH */
+ endpoint_update(edi, _in, _out,
e);
+ /* fall through */
Why change the comment?
Oh, I based this change in the following comment to another patch I
sent some weeks ago:
https://lkml.org/lkml/2017/2/10/293
It is due to Coding Style.
Alan Stern
default:
continue;
}
- if (usb_endpoint_dir_in(>desc)) {
- if (!in)
- in = e;
- } else {
- if (!out)
- out = e;
- }
- continue;
-try_intr:
- if (usb_endpoint_dir_in(>desc)) {
- if (!int_in)
- int_in = e;
- } else {
- if
[PATCH 2/2] usb: misc: refactor code
Code refactoring to make the flow easier to follow.
Cc: Alan Stern <st...@rowloand.harvard.edu>
Cc: Greg Kroah-Hartman <gre...@linuxfoundation.org>
Signed-off-by: Gustavo A. R. Silva <garsi...@embeddedor.com>
---
drivers/usb/misc/usbtest.c | 67 +-
1 file changed, 30 insertions(+), 37 deletions(-)
diff --git a/drivers/usb/misc/usbtest.c b/drivers/usb/misc/usbtest.c
index 7bfb6b78..382491e 100644
--- a/drivers/usb/misc/usbtest.c
+++ b/drivers/usb/misc/usbtest.c
@@ -124,18 +124,32 @@ static struct usb_device *testdev_to_usbdev(struct
usbtest_dev *test)
/*-*/
+static inline void endpoint_update(int edi,
+ struct usb_host_endpoint **in,
+ struct usb_host_endpoint **out,
+ struct usb_host_endpoint *e)
+{
+ if (edi) {
+ if (!*in)
+ *in = e;
+ } else {
+ if (!*out)
+ *out = e;
+ }
+}
+
static int
get_endpoints(struct usbtest_dev *dev, struct usb_interface *intf)
{
- int tmp;
- struct usb_host_interface *alt;
- struct usb_host_endpoint*in, *out;
- struct usb_host_endpoint*iso_in, *iso_out;
- struct usb_host_endpoint*int_in, *int_out;
- struct usb_device *udev;
+ int tmp;
+ struct usb_host_interface *alt;
+ struct usb_host_endpoint*in, *out;
+ struct usb_host_endpoint*iso_in, *iso_out;
+ struct usb_host_endpoint*int_in, *int_out;
+ struct usb_device *udev;
for (tmp = 0; tmp < intf->num_altsetting; tmp++) {
- unsignedep;
+ unsignedep;
in = out = NULL;
iso_in = iso_out = NULL;
@@ -150,48 +164,27 @@ get_endpoints(struct usbtest_dev *dev, struct
usb_interface *intf)
* ignore other endpoints and altsettings.
*/
for (ep = 0; ep < alt->desc.bNumEndpoints; ep++) {
- struct usb_host_endpoint*e;
+ struct usb_host_endpoint*e;
+ int edi;
e = alt->endpoint + ep;
+ edi = usb_endpoint_dir_in(>desc);
+
switch (usb_endpoint_type(>desc)) {
case USB_ENDPOINT_XFER_BULK:
- break;
+ endpoint_update(edi, , , e);
+ continue;
case USB_ENDPOINT_XFER_INT:
if (dev->info->intr)
- goto try_intr;
+ endpoint_update(edi, _in, _out,
e);
continue;
case USB_ENDPOINT_XFER_ISOC:
if (dev->info->iso)
- goto try_iso;
- /* FALLTHROUGH */
+ endpoint_update(edi, _in, _out,
e);
+ /* fall through */
default:
continue;
}
- if (usb_endpoint_dir_in(>desc)) {
- if (!in)
- in = e;
- } else {
- if (!out)
- out = e;
- }
- continue;
-try_intr:
- if (usb_endpoint_dir_in(>desc)) {
- if (!int_in)
- int_in = e;
- } else {
- if (!int_out)
- int_out = e;
- }
- continue;
-try_iso:
- if (usb_endpoint_dir_in(>desc)) {
- if (!iso_in)
- iso_in = e;
- } else {
- if (!iso_out)
- iso_out = e;
- }
}
if ((in && out) || iso_in || iso_out || int_in || int_out)
goto found;
--
2.5.0
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] usb: misc: add missing continue and refactor code
Quoting Alan Stern <st...@rowland.harvard.edu>: On Mon, 3 Apr 2017, Greg Kroah-Hartman wrote: On Mon, Apr 03, 2017 at 09:39:53AM -0500, Gustavo A. R. Silva wrote: > -Code refactoring to make the flow easier to follow. > -Add missing 'continue' for case USB_ENDPOINT_XFER_INT. Don't do multiple things in the same patch, please make these multiple patches. And do the "add missing continue" first, so it can be backported to other kernels easier please. OK, I will send a patchset shortly. Also, make sure your patch does not contain gratuitous whitespace changes. Does it have any? I ran it through checkpatch.pl before sending it and didn't see any. Thanks -- Gustavo A. R. Silva -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 1/2] usb: misc: add missing continue in switch
Add missing continue in switch. Addresses-Coverity-ID: 1248733 Cc: Alan Stern <st...@rowloand.harvard.edu> Cc: Greg Kroah-Hartman <gre...@linuxfoundation.org> Signed-off-by: Gustavo A. R. Silva <garsi...@embeddedor.com> --- drivers/usb/misc/usbtest.c | 1 + 1 file changed, 1 insertion(+) diff --git a/drivers/usb/misc/usbtest.c b/drivers/usb/misc/usbtest.c index 3525626..7bfb6b78 100644 --- a/drivers/usb/misc/usbtest.c +++ b/drivers/usb/misc/usbtest.c @@ -159,6 +159,7 @@ get_endpoints(struct usbtest_dev *dev, struct usb_interface *intf) case USB_ENDPOINT_XFER_INT: if (dev->info->intr) goto try_intr; + continue; case USB_ENDPOINT_XFER_ISOC: if (dev->info->iso) goto try_iso; -- 2.5.0 -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] usb: misc: add missing continue and refactor code
-Code refactoring to make the flow easier to follow.
-Add missing 'continue' for case USB_ENDPOINT_XFER_INT.
Addresses-Coverity-ID: 1248733
Cc: Alan Stern <st...@rowloand.harvard.edu>
Signed-off-by: Gustavo A. R. Silva <garsi...@embeddedor.com>
---
drivers/usb/misc/usbtest.c | 68 +-
1 file changed, 31 insertions(+), 37 deletions(-)
diff --git a/drivers/usb/misc/usbtest.c b/drivers/usb/misc/usbtest.c
index 3525626..382491e 100644
--- a/drivers/usb/misc/usbtest.c
+++ b/drivers/usb/misc/usbtest.c
@@ -124,18 +124,32 @@ static struct usb_device *testdev_to_usbdev(struct
usbtest_dev *test)
/*-*/
+static inline void endpoint_update(int edi,
+ struct usb_host_endpoint **in,
+ struct usb_host_endpoint **out,
+ struct usb_host_endpoint *e)
+{
+ if (edi) {
+ if (!*in)
+ *in = e;
+ } else {
+ if (!*out)
+ *out = e;
+ }
+}
+
static int
get_endpoints(struct usbtest_dev *dev, struct usb_interface *intf)
{
- int tmp;
- struct usb_host_interface *alt;
- struct usb_host_endpoint*in, *out;
- struct usb_host_endpoint*iso_in, *iso_out;
- struct usb_host_endpoint*int_in, *int_out;
- struct usb_device *udev;
+ int tmp;
+ struct usb_host_interface *alt;
+ struct usb_host_endpoint*in, *out;
+ struct usb_host_endpoint*iso_in, *iso_out;
+ struct usb_host_endpoint*int_in, *int_out;
+ struct usb_device *udev;
for (tmp = 0; tmp < intf->num_altsetting; tmp++) {
- unsignedep;
+ unsignedep;
in = out = NULL;
iso_in = iso_out = NULL;
@@ -150,47 +164,27 @@ get_endpoints(struct usbtest_dev *dev, struct
usb_interface *intf)
* ignore other endpoints and altsettings.
*/
for (ep = 0; ep < alt->desc.bNumEndpoints; ep++) {
- struct usb_host_endpoint*e;
+ struct usb_host_endpoint*e;
+ int edi;
e = alt->endpoint + ep;
+ edi = usb_endpoint_dir_in(>desc);
+
switch (usb_endpoint_type(>desc)) {
case USB_ENDPOINT_XFER_BULK:
- break;
+ endpoint_update(edi, , , e);
+ continue;
case USB_ENDPOINT_XFER_INT:
if (dev->info->intr)
- goto try_intr;
+ endpoint_update(edi, _in, _out,
e);
+ continue;
case USB_ENDPOINT_XFER_ISOC:
if (dev->info->iso)
- goto try_iso;
- /* FALLTHROUGH */
+ endpoint_update(edi, _in, _out,
e);
+ /* fall through */
default:
continue;
}
- if (usb_endpoint_dir_in(>desc)) {
- if (!in)
- in = e;
- } else {
- if (!out)
- out = e;
- }
- continue;
-try_intr:
- if (usb_endpoint_dir_in(>desc)) {
- if (!int_in)
- int_in = e;
- } else {
- if (!int_out)
- int_out = e;
- }
- continue;
-try_iso:
- if (usb_endpoint_dir_in(>desc)) {
- if (!iso_in)
- iso_in = e;
- } else {
- if (!iso_out)
- iso_out = e;
- }
}
if ((in && out) || iso_in || iso_out || int_in || int_out)
goto found;
--
2.5.0
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v4] usb: gadget: udc: remove pointer dereference after free
Remove pointer dereference after free. Addresses-Coverity-ID: 1091173 Acked-by: Michal Nazarewicz <min...@mina86.com> Signed-off-by: Gustavo A. R. Silva <garsi...@embeddedor.com> --- Changes in v2: Move pointer dereference before pci_pool_free() Set pointer to NULL after free Changes in v3: Remove 'td->next = 0x00' inside for loop. Remove unnecessary pointer nullification after free. Changes in v4: Fix line-wrapping in previous patch. drivers/usb/gadget/udc/pch_udc.c | 1 - 1 file changed, 1 deletion(-) diff --git a/drivers/usb/gadget/udc/pch_udc.c b/drivers/usb/gadget/udc/pch_udc.c index a97da64..8a365aa 100644 --- a/drivers/usb/gadget/udc/pch_udc.c +++ b/drivers/usb/gadget/udc/pch_udc.c @@ -1523,7 +1523,6 @@ static void pch_udc_free_dma_chain(struct pch_udc_dev *dev, td = phys_to_virt(addr); addr2 = (dma_addr_t)td->next; pci_pool_free(dev->data_requests, td, addr); - td->next = 0x00; addr = addr2; } req->chain_len = 1; -- 2.5.0 -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v3] usb: gadget: udc: remove pointer dereference after free
Hello, Quoting Felipe Balbi <ba...@kernel.org>: "Gustavo A. R. Silva" <garsi...@embeddedor.com> writes: Remove pointer dereference after free. Addresses-Coverity-ID: 1091173 Acked-by: Michal Nazarewicz <min...@mina86.com> Signed-off-by: Gustavo A. R. Silva <garsi...@embeddedor.com> --- Changes in v2: Move pointer dereference before pci_pool_free() Set pointer to NULL after free Changes in v3: Remove 'td->next = 0x00' inside for loop. Remove unnecessary pointer nullification after free. drivers/usb/gadget/udc/pch_udc.c | 1 - 1 file changed, 1 deletion(-) diff --git a/drivers/usb/gadget/udc/pch_udc.c b/drivers/usb/gadget/udc/pch_udc.c index a97da64..8a365aa 100644 --- a/drivers/usb/gadget/udc/pch_udc.c +++ b/drivers/usb/gadget/udc/pch_udc.c @@ -1523,7 +1523,6 @@ static void pch_udc_free_dma_chain(struct pch_udc_dev *dev, line wrapped. Can't apply. I'll fix it right away. Thanks -- Gustavo A. R. Silva -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] usb: misc: remove unnecessary code
Hi Peter, Quoting Peter Senna Tschudin <peter.se...@gmail.com>: On Mon, Feb 20, 2017 at 05:28:46PM -0600, Gustavo A. R. Silva wrote: 'val' is an unsigned variable, and less-than-zero comparison of an unsigned variable is never true. I would add that val is set by kstrtoul() that converts a string to an unsigned long. Addresses-Coverity-ID: 1230257 Reviewed-by: Peter Senna Tschudin <peter.se...@gmail.com> Signed-off-by: Gustavo A. R. Silva <garsi...@embeddedor.com> --- drivers/usb/misc/lvstest.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/usb/misc/lvstest.c b/drivers/usb/misc/lvstest.c index c7c2104..6f37610 100644 --- a/drivers/usb/misc/lvstest.c +++ b/drivers/usb/misc/lvstest.c @@ -193,7 +193,7 @@ static ssize_t u2_timeout_store(struct device *dev, return ret; } - if (val < 0 || val > 127) + if (val > 127) return -EINVAL; ret = lvs_rh_set_port_feature(hdev, lvs->portnum | (val << 8), -- 2.5.0 Thanks for your comments. -- Gustavo A. R. Silva -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] usb: misc: add a missing continue and refactor code
Quoting "Gustavo A. R. Silva" <garsi...@embeddedor.com>:
Hi Alan,
Quoting Alan Stern <st...@rowland.harvard.edu>:
On Tue, 21 Feb 2017, Gustavo A. R. Silva wrote:
Code refactoring to make the flow easier to follow and add missing
'continue' for case USB_ENDPOINT_XFER_INT.
Addresses-Coverity-ID: 1248733
Cc: Alan Stern <st...@rowland.harvard.edu>
Signed-off-by: Gustavo A. R. Silva <garsi...@embeddedor.com>
---
drivers/usb/misc/usbtest.c | 50
+++---
1 file changed, 30 insertions(+), 20 deletions(-)
diff --git a/drivers/usb/misc/usbtest.c b/drivers/usb/misc/usbtest.c
index 3525626..8723e33 100644
--- a/drivers/usb/misc/usbtest.c
+++ b/drivers/usb/misc/usbtest.c
@@ -124,6 +124,32 @@ static struct usb_device
*testdev_to_usbdev(struct usbtest_dev *test)
/*-*/
+static inline void try_intr(struct usb_host_endpoint *e,
+ struct usb_host_endpoint *int_in,
+ struct usb_host_endpoint *int_out)
+{
+ if (usb_endpoint_dir_in(>desc)) {
+ if (!int_in)
+ int_in = e;
+ } else {
+ if (!int_out)
+ int_out = e;
+ }
+}
+
+static inline void try_iso(struct usb_host_endpoint *e,
+ struct usb_host_endpoint *iso_in,
+ struct usb_host_endpoint *iso_out)
+{
+ if (usb_endpoint_dir_in(>desc)) {
+ if (!iso_in)
+ iso_in = e;
+ } else {
+ if (!iso_out)
+ iso_out = e;
+ }
+}
+
This is not at all what I had in mind. First, it's incorrect (can you
see why?). Second, by "inline" I meant moving the code to be actually
in-line next to the conditional, not some place else in a separate
subroutine (even if the subroutine is declared inline).
Interesting... let me double check.
I thought it would've been better to have separate inline
subroutines for those "goto".
Also, the code for the USB_ENDPOINT_XFER_BULK case should look like the
other two.
Do you mean a 'continue' instead of the 'break'?
Oh I see, the following piece of code should be part of the
USB_ENDPOINT_XFER_BULK case:
if (usb_endpoint_dir_in(>desc)) {
if (!in)
in = e;
} else {
if (!out)
out = e;
}
continue;
--
Gustavo A. R. Silva
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
