[PATCH 1/2] liquidio: remove unnecessary NULL check before kfree in delete_glists
NULL check before freeing functions like kfree is not needed.
This issue was detected with the help of Coccinelle.
Signed-off-by: Gustavo A. R. Silva <garsi...@embeddedor.com>
---
This code was tested by compilation only.
drivers/net/ethernet/cavium/liquidio/lio_vf_main.c | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/drivers/net/ethernet/cavium/liquidio/lio_vf_main.c
b/drivers/net/ethernet/cavium/liquidio/lio_vf_main.c
index 2e993ce..e4a112c 100644
--- a/drivers/net/ethernet/cavium/liquidio/lio_vf_main.c
+++ b/drivers/net/ethernet/cavium/liquidio/lio_vf_main.c
@@ -435,8 +435,7 @@ static void delete_glists(struct lio *lio)
do {
g = (struct octnic_gather *)
list_delete_head(>glist[i]);
- if (g)
- kfree(g);
+ kfree(g);
} while (g);
if (lio->glists_virt_base && lio->glists_virt_base[i] &&
--
2.7.4
[PATCH 2/2] liquidio: mark expected switch fall-through in octeon_destroy_resources
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> --- This code was tested by compilation only (GCC 7.2.0 was used). Please, verify if the actual intention of the code is to fall through. drivers/net/ethernet/cavium/liquidio/lio_vf_main.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/net/ethernet/cavium/liquidio/lio_vf_main.c b/drivers/net/ethernet/cavium/liquidio/lio_vf_main.c index e4a112c..4c3b568 100644 --- a/drivers/net/ethernet/cavium/liquidio/lio_vf_main.c +++ b/drivers/net/ethernet/cavium/liquidio/lio_vf_main.c @@ -747,7 +747,7 @@ static void octeon_destroy_resources(struct octeon_device *oct) if (lio_wait_for_oq_pkts(oct)) dev_err(>pci_dev->dev, "OQ had pending packets\n"); - + /* fall through */ case OCT_DEV_INTR_SET_DONE: /* Disable interrupts */ oct->fn_list.disable_interrupt(oct, OCTEON_ALL_INTR); -- 2.7.4
[PATCH] net: l2tp: mark expected switch fall-through
In preparation to enabling -Wimplicit-fallthrough, mark switch cases
where we are expecting to fall through.
Notice that in this particular case I replaced the "NOBREAK" comment with
a "fall through" comment, which is what GCC is expecting to find.
Signed-off-by: Gustavo A. R. Silva <garsi...@embeddedor.com>
---
This code was tested by compilation only (GCC 7.2.0 was used).
net/l2tp/l2tp_netlink.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/net/l2tp/l2tp_netlink.c b/net/l2tp/l2tp_netlink.c
index 7135f46..f517942 100644
--- a/net/l2tp/l2tp_netlink.c
+++ b/net/l2tp/l2tp_netlink.c
@@ -406,7 +406,7 @@ static int l2tp_nl_tunnel_send(struct sk_buff *skb, u32
portid, u32 seq, int fla
if (nla_put_u16(skb, L2TP_ATTR_UDP_SPORT,
ntohs(inet->inet_sport)) ||
nla_put_u16(skb, L2TP_ATTR_UDP_DPORT,
ntohs(inet->inet_dport)))
goto nla_put_failure;
- /* NOBREAK */
+ /* fall through */
case L2TP_ENCAPTYPE_IP:
#if IS_ENABLED(CONFIG_IPV6)
if (np) {
--
2.7.4
[PATCH] net: mac80211: mark expected switch fall-throughs
In preparation to enabling -Wimplicit-fallthrough, mark switch cases
where we are expecting to fall through.
Notice that in some cases I replaced "fall through on else" and
"otherwise fall through" comments with just a "fall through" comment,
which is what GCC is expecting to find.
Signed-off-by: Gustavo A. R. Silva <garsi...@embeddedor.com>
---
This code was tested by compilation only (GCC 7.2.0 was used).
Please, verify that the actual intention of the code is to fall through.
net/mac80211/cfg.c| 3 +++
net/mac80211/ht.c | 1 +
net/mac80211/iface.c | 2 +-
net/mac80211/mesh.c | 2 ++
net/mac80211/mesh_hwmp.c | 1 +
net/mac80211/mesh_plink.c | 2 +-
net/mac80211/mlme.c | 1 +
net/mac80211/offchannel.c | 4 ++--
net/mac80211/tdls.c | 1 +
net/mac80211/wme.c| 1 +
10 files changed, 14 insertions(+), 4 deletions(-)
diff --git a/net/mac80211/cfg.c b/net/mac80211/cfg.c
index a354f19..9bd8bef 100644
--- a/net/mac80211/cfg.c
+++ b/net/mac80211/cfg.c
@@ -573,10 +573,12 @@ static int ieee80211_get_key(struct wiphy *wiphy, struct
net_device *dev,
case WLAN_CIPHER_SUITE_BIP_CMAC_256:
BUILD_BUG_ON(offsetof(typeof(kseq), ccmp) !=
offsetof(typeof(kseq), aes_cmac));
+ /* fall through */
case WLAN_CIPHER_SUITE_BIP_GMAC_128:
case WLAN_CIPHER_SUITE_BIP_GMAC_256:
BUILD_BUG_ON(offsetof(typeof(kseq), ccmp) !=
offsetof(typeof(kseq), aes_gmac));
+ /* fall through */
case WLAN_CIPHER_SUITE_GCMP:
case WLAN_CIPHER_SUITE_GCMP_256:
BUILD_BUG_ON(offsetof(typeof(kseq), ccmp) !=
@@ -2205,6 +2207,7 @@ static int ieee80211_scan(struct wiphy *wiphy,
* for now fall through to allow scanning only when
* beaconing hasn't been configured yet
*/
+ /* fall through */
case NL80211_IFTYPE_AP:
/*
* If the scan has been forced (and the driver supports
diff --git a/net/mac80211/ht.c b/net/mac80211/ht.c
index 41f5e48..e55dabf 100644
--- a/net/mac80211/ht.c
+++ b/net/mac80211/ht.c
@@ -491,6 +491,7 @@ int ieee80211_send_smps_action(struct ieee80211_sub_if_data
*sdata,
case IEEE80211_SMPS_AUTOMATIC:
case IEEE80211_SMPS_NUM_MODES:
WARN_ON(1);
+ /* fall through */
case IEEE80211_SMPS_OFF:
action_frame->u.action.u.ht_smps.smps_control =
WLAN_HT_SMPS_CONTROL_DISABLED;
diff --git a/net/mac80211/iface.c b/net/mac80211/iface.c
index 13b16f9..435e735 100644
--- a/net/mac80211/iface.c
+++ b/net/mac80211/iface.c
@@ -1633,7 +1633,7 @@ static void ieee80211_assign_perm_addr(struct
ieee80211_local *local,
goto out_unlock;
}
}
- /* otherwise fall through */
+ /* fall through */
default:
/* assign a new address if possible -- try n_addresses first */
for (i = 0; i < local->hw.wiphy->n_addresses; i++) {
diff --git a/net/mac80211/mesh.c b/net/mac80211/mesh.c
index 7a76c4a..d29a545 100644
--- a/net/mac80211/mesh.c
+++ b/net/mac80211/mesh.c
@@ -988,8 +988,10 @@ ieee80211_mesh_process_chnswitch(struct
ieee80211_sub_if_data *sdata,
switch (sdata->vif.bss_conf.chandef.width) {
case NL80211_CHAN_WIDTH_20_NOHT:
sta_flags |= IEEE80211_STA_DISABLE_HT;
+ /* fall through */
case NL80211_CHAN_WIDTH_20:
sta_flags |= IEEE80211_STA_DISABLE_40MHZ;
+ /* fall through */
case NL80211_CHAN_WIDTH_40:
sta_flags |= IEEE80211_STA_DISABLE_VHT;
break;
diff --git a/net/mac80211/mesh_hwmp.c b/net/mac80211/mesh_hwmp.c
index 146ec6c..0e75abf 100644
--- a/net/mac80211/mesh_hwmp.c
+++ b/net/mac80211/mesh_hwmp.c
@@ -1247,6 +1247,7 @@ void mesh_path_tx_root_frame(struct ieee80211_sub_if_data
*sdata)
break;
case IEEE80211_PROACTIVE_PREQ_WITH_PREP:
flags |= IEEE80211_PREQ_PROACTIVE_PREP_FLAG;
+ /* fall through */
case IEEE80211_PROACTIVE_PREQ_NO_PREP:
interval = ifmsh->mshcfg.dot11MeshHWMPactivePathToRootTimeout;
target_flags |= IEEE80211_PREQ_TO_FLAG |
diff --git a/net/mac80211/mesh_plink.c b/net/mac80211/mesh_plink.c
index e2d00cc..0f6c9ca 100644
--- a/net/mac80211/mesh_plink.c
+++ b/net/mac80211/mesh_plink.c
@@ -672,7 +672,7 @@ void mesh_plink_timer(struct timer_list *t)
break;
}
reason = WLAN_REASON_MESH_MAX_RETRIES;
- /* fall through on else */
+ /* fall through */
case NL80211_PLINK_CNF_RCVD:
/* confirm timer */
if (!reason)
diff --git a/n
Re: [PATCH 00/20] mark expected switch fall-throughs
Quoting "Gustavo A. R. Silva" <garsi...@embeddedor.com>: 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 "no break" or "fall thru" comments already in place. So I replaced them with proper "fall through" comments, which is what GCC is expecting to find. For the rest of the cases, please double check if the actual intention of the code is to fall through. Thanks! Gustavo A. R. Silva (20): staging: ks7010: ks_wlan_net: mark expected switch fall-throughs staging: lustre: lnet: socklnd: mark expected switch fall-through staging: rtl8192e: mark expected switch fall-through staging: rtl8723bs: rtw_mlme_ext: mark expected switch fall-through staging: lustre: lnet: net_fault: mark expected switch fall-through staging: comedi: s526: mark expected switch fall-through staging: rtl8188eu: usb_halinit: mark expected switch fall-through staging: vc04_services: vchiq_core: mark expected switch fall-through staging: vt6656: card: mark expected switch fall-throughs staging: rtl8188eu: usb_ops_linux: mark expected switch fall-through staging: r8822be: mark expected switch fall-throughs staging: lustre: lnet: selftest: mark expected switch fall-through staging: rtlwifi: halmac: mark expected switch fall-through staging: lustre: lnet: selftest: mark expected switch fall-throughs staging: lustre: llite: mark expected switch fall-through staging: lustre: lprocfs: mark expected switch fall-throughs staging: lustre: ldlm: mark expected switch fall-through staging: lustre: osc: mark expected switch fall-through staging: lustre: ptlrpc: mark expected switch fall-throughs staging: lustre: rpc: mark expected switch fall-throughs drivers/staging/comedi/drivers/s526.c | 5 ++--- drivers/staging/ks7010/ks_wlan_net.c| 10 ++ drivers/staging/lustre/lnet/klnds/socklnd/socklnd.c | 2 +- drivers/staging/lustre/lnet/lnet/net_fault.c| 1 + drivers/staging/lustre/lnet/selftest/conctl.c | 1 + drivers/staging/lustre/lnet/selftest/module.c | 5 - drivers/staging/lustre/lnet/selftest/rpc.c | 13 + drivers/staging/lustre/lustre/ldlm/ldlm_request.c | 1 + drivers/staging/lustre/lustre/llite/namei.c | 4 +++- drivers/staging/lustre/lustre/obdclass/lprocfs_status.c | 4 drivers/staging/lustre/lustre/osc/osc_cache.c | 1 + drivers/staging/lustre/lustre/ptlrpc/pack_generic.c | 6 +++--- drivers/staging/rtl8188eu/hal/usb_halinit.c | 1 + drivers/staging/rtl8188eu/os_dep/usb_ops_linux.c| 1 + drivers/staging/rtl8192e/rtllib_wx.c| 3 +-- drivers/staging/rtl8723bs/core/rtw_mlme_ext.c | 2 +- .../staging/rtlwifi/halmac/halmac_88xx/halmac_api_88xx.c| 2 ++ drivers/staging/rtlwifi/halmac/rtl_halmac.c | 2 +- .../staging/vc04_services/interface/vchiq_arm/vchiq_core.c | 3 +-- drivers/staging/vt6656/card.c | 2 ++ 20 files changed, 50 insertions(+), 19 deletions(-) -- 2.7.4 Andreas, Stefan: Thank you for your reviews and ACKs Greg: Thank you for applying the patches. -- Gustavo A. R. Silva
[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
[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
[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
[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
[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
[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
[PATCH] i40e/virtchnl: fix application of sizeof to pointer
sizeof when applied to a pointer typed expression gives the size of the pointer. The proper fix in this particular case is to code sizeof(*vfres) instead of sizeof(vfres). This issue was detected with the help of Coccinelle. Signed-off-by: Gustavo A. R. Silva <garsi...@embeddedor.com> --- This code was tested by compilation only. drivers/net/ethernet/intel/i40e/i40e_virtchnl_pf.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/net/ethernet/intel/i40e/i40e_virtchnl_pf.c b/drivers/net/ethernet/intel/i40e/i40e_virtchnl_pf.c index f8a794b..56dd1e8 100644 --- a/drivers/net/ethernet/intel/i40e/i40e_virtchnl_pf.c +++ b/drivers/net/ethernet/intel/i40e/i40e_virtchnl_pf.c @@ -2086,7 +2086,7 @@ static int i40e_vc_request_queues_msg(struct i40e_vf *vf, u8 *msg, int msglen) } return i40e_vc_send_msg_to_vf(vf, VIRTCHNL_OP_REQUEST_QUEUES, 0, - (u8 *)vfres, sizeof(vfres)); + (u8 *)vfres, sizeof(*vfres)); } /** -- 2.7.4
Re: [PATCH] net: rxrpc: mark expected switch fall-throughs
Quoting David Howells <dhowe...@redhat.com>: Gustavo A. R. Silva <garsi...@embeddedor.com> wrote: + /* fall through */ All new comments in rxrpc should begin with a capital letter; I'm switching to this as I modify the lines with comments on. Fix checkpatch or gcc or whatever takes -Wimplicit-fallthrough to stop being silly. - /* Fall through */ - + /* fall through */ No. Firstly, it should be 'F'; secondly, don't remove the blank line - it's there for a reason. What is the reason? + /* fall through */ Capital 'F'. - + /* fall through */ Don't remove the blank line. Capital 'F'. + /* fall through */ Capital 'F'. David Thanks -- Gustavo A. R. Silva
[PATCH] net: netrom: nr_in: 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> --- This code was tested by compilation only (GCC 7.2.0 was used). Please, verify if the actual intention of the code is to fall through. net/netrom/nr_in.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/net/netrom/nr_in.c b/net/netrom/nr_in.c index 80dbd0b..fbfdae4 100644 --- a/net/netrom/nr_in.c +++ b/net/netrom/nr_in.c @@ -125,7 +125,7 @@ static int nr_state2_machine(struct sock *sk, struct sk_buff *skb, case NR_DISCREQ: nr_write_internal(sk, NR_DISCACK); - + /* fall through */ case NR_DISCACK: nr_disconnect(sk, 0); break; -- 2.7.4
[PATCH] openvswitch: conntrack: mark expected switch fall-through
In preparation to enabling -Wimplicit-fallthrough, mark switch cases where we are expecting to fall through. Notice that in this particular case I placed a "fall through" comment on its own line, which is what GCC is expecting to find. Signed-off-by: Gustavo A. R. Silva <garsi...@embeddedor.com> --- net/openvswitch/conntrack.c | 1 + 1 file changed, 1 insertion(+) diff --git a/net/openvswitch/conntrack.c b/net/openvswitch/conntrack.c index fe861e2..b27c5c6 100644 --- a/net/openvswitch/conntrack.c +++ b/net/openvswitch/conntrack.c @@ -752,6 +752,7 @@ static int ovs_ct_nat_execute(struct sk_buff *skb, struct nf_conn *ct, } } /* Non-ICMP, fall thru to initialize if needed. */ + /* fall through */ case IP_CT_NEW: /* Seen it before? This can happen for loopback, retrans, * or local packets. -- 2.7.4
[PATCH] net: rose: 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>
---
This code was tested by compilation only (GCC 7.2.0 was used).
Please, verify if the actual intention of the code is to fall through.
net/rose/rose_in.c| 1 +
net/rose/rose_route.c | 2 ++
2 files changed, 3 insertions(+)
diff --git a/net/rose/rose_in.c b/net/rose/rose_in.c
index 0a63947..9bbbfe3 100644
--- a/net/rose/rose_in.c
+++ b/net/rose/rose_in.c
@@ -219,6 +219,7 @@ static int rose_state4_machine(struct sock *sk, struct
sk_buff *skb, int framety
switch (frametype) {
case ROSE_RESET_REQUEST:
rose_write_internal(sk, ROSE_RESET_CONFIRMATION);
+ /* fall through */
case ROSE_RESET_CONFIRMATION:
rose_stop_timer(sk);
rose_start_idletimer(sk);
diff --git a/net/rose/rose_route.c b/net/rose/rose_route.c
index 452bbb3..ac0f733 100644
--- a/net/rose/rose_route.c
+++ b/net/rose/rose_route.c
@@ -346,6 +346,7 @@ static int rose_del_node(struct rose_route_struct
*rose_route,
case 0:
rose_node->neighbour[0] =
rose_node->neighbour[1];
+ /* fall through */
case 1:
rose_node->neighbour[1] =
rose_node->neighbour[2];
@@ -507,6 +508,7 @@ void rose_rt_device_down(struct net_device *dev)
switch (i) {
case 0:
t->neighbour[0] = t->neighbour[1];
+ /* fall through */
case 1:
t->neighbour[1] = t->neighbour[2];
case 2:
--
2.7.4
[PATCH] net: rxrpc: 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> --- This code was tested by compilation only (GCC 7.2.0 was used). Please, verify if the actual intention of the code is to fall through. net/rxrpc/af_rxrpc.c | 5 +++-- net/rxrpc/input.c| 2 +- net/rxrpc/sendmsg.c | 1 + 3 files changed, 5 insertions(+), 3 deletions(-) diff --git a/net/rxrpc/af_rxrpc.c b/net/rxrpc/af_rxrpc.c index fb17552..df4f7d5 100644 --- a/net/rxrpc/af_rxrpc.c +++ b/net/rxrpc/af_rxrpc.c @@ -246,6 +246,7 @@ static int rxrpc_listen(struct socket *sock, int backlog) ret = 0; break; } + /* fall through */ default: ret = -EBUSY; break; @@ -528,8 +529,7 @@ static int rxrpc_sendmsg(struct socket *sock, struct msghdr *m, size_t len) rx->local = local; rx->sk.sk_state = RXRPC_CLIENT_UNBOUND; - /* Fall through */ - + /* fall through */ case RXRPC_CLIENT_UNBOUND: case RXRPC_CLIENT_BOUND: if (!m->msg_name && @@ -537,6 +537,7 @@ static int rxrpc_sendmsg(struct socket *sock, struct msghdr *m, size_t len) m->msg_name = >connect_srx; m->msg_namelen = sizeof(rx->connect_srx); } + /* fall through */ case RXRPC_SERVER_BOUND: case RXRPC_SERVER_LISTENING: ret = rxrpc_do_sendmsg(rx, m, len); diff --git a/net/rxrpc/input.c b/net/rxrpc/input.c index e56e23e..8bc6daf 100644 --- a/net/rxrpc/input.c +++ b/net/rxrpc/input.c @@ -1125,7 +1125,7 @@ void rxrpc_data_ready(struct sock *udp_sk) case RXRPC_PACKET_TYPE_BUSY: if (sp->hdr.flags & RXRPC_CLIENT_INITIATED) goto discard; - + /* fall through */ case RXRPC_PACKET_TYPE_DATA: if (sp->hdr.callNumber == 0) goto bad_message; diff --git a/net/rxrpc/sendmsg.c b/net/rxrpc/sendmsg.c index 9ea6f97..d245782 100644 --- a/net/rxrpc/sendmsg.c +++ b/net/rxrpc/sendmsg.c @@ -166,6 +166,7 @@ static void rxrpc_queue_packet(struct rxrpc_sock *rx, struct rxrpc_call *call, ktime_get_real()); if (!last) break; + /* fall through */ case RXRPC_CALL_SERVER_SEND_REPLY: call->state = RXRPC_CALL_SERVER_AWAIT_ACK; rxrpc_notify_end_tx(rx, call, notify_end_tx); -- 2.7.4
Re: [PATCH] net: xfrm_user: use BUG_ON instead of if condition followed by BUG
Quoting Herbert Xu <herb...@gondor.apana.org.au>: On Mon, Oct 23, 2017 at 10:50:43PM -0500, Gustavo A. R. Silva wrote: Quoting Herbert Xu <herb...@gondor.apana.org.au>: >On Mon, Oct 23, 2017 at 01:18:57PM -0500, Gustavo A. R. Silva wrote: >>Use BUG_ON instead of if condition followed by BUG. >> >>This issue was detected with the help of Coccinelle. >> >>Signed-off-by: Gustavo A. R. Silva <garsi...@embeddedor.com> > >I think this patch is terrible. Why on earth is Coccinelle even >warning about this? > >If anything we should be converting these constructs to not use >BUG. > Yeah, and under this assumption the original code is even worse. No your patch makes it worse. The original construct makes it clear that the conditional is real code and not just some debugging aid. With your patch, real code is now inside BUG_ON. What is the reason for BUG_ON to exist then if it makes the code worse than an _if_ condition followed by BUG? Thanks -- Gustavo A. R. Silva
Re: [PATCH] net: xfrm_user: use BUG_ON instead of if condition followed by BUG
Quoting Herbert Xu <herb...@gondor.apana.org.au>: On Mon, Oct 23, 2017 at 01:18:57PM -0500, Gustavo A. R. Silva wrote: Use BUG_ON instead of if condition followed by BUG. This issue was detected with the help of Coccinelle. Signed-off-by: Gustavo A. R. Silva <garsi...@embeddedor.com> I think this patch is terrible. Why on earth is Coccinelle even warning about this? If anything we should be converting these constructs to not use BUG. Yeah, and under this assumption the original code is even worse. Thanks -- Gustavo A. R. Silva
[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
[PATCH] ipv4: esp4: use BUG_ON instead of if condition followed by BUG
Use BUG_ON instead of if condition followed by BUG in esp_remove_trailer. This issue was detected with the help of Coccinelle. Signed-off-by: Gustavo A. R. Silva <garsi...@embeddedor.com> --- net/ipv4/esp4.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/net/ipv4/esp4.c b/net/ipv4/esp4.c index b00e4a4..89b6c5e 100644 --- a/net/ipv4/esp4.c +++ b/net/ipv4/esp4.c @@ -521,8 +521,7 @@ static inline int esp_remove_trailer(struct sk_buff *skb) goto out; } - if (skb_copy_bits(skb, skb->len - alen - 2, nexthdr, 2)) - BUG(); + BUG_ON(skb_copy_bits(skb, skb->len - alen - 2, nexthdr, 2)); ret = -EINVAL; padlen = nexthdr[0]; -- 2.7.4
[PATCH] net: xfrm_user: use BUG_ON instead of if condition followed by BUG
Use BUG_ON instead of if condition followed by BUG. This issue was detected with the help of Coccinelle. Signed-off-by: Gustavo A. R. Silva <garsi...@embeddedor.com> --- net/xfrm/xfrm_user.c | 28 ++-- 1 file changed, 10 insertions(+), 18 deletions(-) diff --git a/net/xfrm/xfrm_user.c b/net/xfrm/xfrm_user.c index 465f3ec..9e8851f 100644 --- a/net/xfrm/xfrm_user.c +++ b/net/xfrm/xfrm_user.c @@ -1152,8 +1152,7 @@ static int xfrm_get_spdinfo(struct sk_buff *skb, struct nlmsghdr *nlh, if (r_skb == NULL) return -ENOMEM; - if (build_spdinfo(r_skb, net, sportid, seq, *flags) < 0) - BUG(); + BUG_ON(build_spdinfo(r_skb, net, sportid, seq, *flags) < 0); return nlmsg_unicast(net->xfrm.nlsk, r_skb, sportid); } @@ -1210,8 +1209,7 @@ static int xfrm_get_sadinfo(struct sk_buff *skb, struct nlmsghdr *nlh, if (r_skb == NULL) return -ENOMEM; - if (build_sadinfo(r_skb, net, sportid, seq, *flags) < 0) - BUG(); + BUG_ON(build_sadinfo(r_skb, net, sportid, seq, *flags) < 0); return nlmsg_unicast(net->xfrm.nlsk, r_skb, sportid); } @@ -1958,8 +1956,8 @@ static int xfrm_get_ae(struct sk_buff *skb, struct nlmsghdr *nlh, c.seq = nlh->nlmsg_seq; c.portid = nlh->nlmsg_pid; - if (build_aevent(r_skb, x, ) < 0) - BUG(); + BUG_ON(build_aevent(r_skb, x, ) < 0); + err = nlmsg_unicast(net->xfrm.nlsk, r_skb, NETLINK_CB(skb).portid); spin_unlock_bh(>lock); xfrm_state_put(x); @@ -2393,8 +2391,7 @@ static int xfrm_send_migrate(const struct xfrm_selector *sel, u8 dir, u8 type, return -ENOMEM; /* build migrate */ - if (build_migrate(skb, m, num_migrate, k, sel, encap, dir, type) < 0) - BUG(); + BUG_ON(build_migrate(skb, m, num_migrate, k, sel, encap, dir, type) < 0); return xfrm_nlmsg_multicast(net, skb, 0, XFRMNLGRP_MIGRATE); } @@ -2623,8 +2620,7 @@ static int xfrm_aevent_state_notify(struct xfrm_state *x, const struct km_event if (skb == NULL) return -ENOMEM; - if (build_aevent(skb, x, c) < 0) - BUG(); + BUG_ON(build_aevent(skb, x, c) < 0); return xfrm_nlmsg_multicast(net, skb, 0, XFRMNLGRP_AEVENTS); } @@ -2836,8 +2832,7 @@ static int xfrm_send_acquire(struct xfrm_state *x, struct xfrm_tmpl *xt, if (skb == NULL) return -ENOMEM; - if (build_acquire(skb, x, xt, xp) < 0) - BUG(); + BUG_ON(build_acquire(skb, x, xt, xp) < 0); return xfrm_nlmsg_multicast(net, skb, 0, XFRMNLGRP_ACQUIRE); } @@ -2951,8 +2946,7 @@ static int xfrm_exp_policy_notify(struct xfrm_policy *xp, int dir, const struct if (skb == NULL) return -ENOMEM; - if (build_polexpire(skb, xp, dir, c) < 0) - BUG(); + BUG_ON(build_polexpire(skb, xp, dir, c) < 0); return xfrm_nlmsg_multicast(net, skb, 0, XFRMNLGRP_EXPIRE); } @@ -3112,8 +3106,7 @@ static int xfrm_send_report(struct net *net, u8 proto, if (skb == NULL) return -ENOMEM; - if (build_report(skb, proto, sel, addr) < 0) - BUG(); + BUG_ON(build_report(skb, proto, sel, addr) < 0); return xfrm_nlmsg_multicast(net, skb, 0, XFRMNLGRP_REPORT); } @@ -3165,8 +3158,7 @@ static int xfrm_send_mapping(struct xfrm_state *x, xfrm_address_t *ipaddr, if (skb == NULL) return -ENOMEM; - if (build_mapping(skb, x, ipaddr, sport) < 0) - BUG(); + BUG_ON(build_mapping(skb, x, ipaddr, sport) < 0); return xfrm_nlmsg_multicast(net, skb, 0, XFRMNLGRP_MAPPING); } -- 2.7.4
[PATCH] ipv4: icmp: use BUG_ON instead of if condition followed by BUG
Use BUG_ON instead of if condition followed by BUG in icmp_timestamp. This issue was detected with the help of Coccinelle. Signed-off-by: Gustavo A. R. Silva <garsi...@embeddedor.com> --- net/ipv4/icmp.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/net/ipv4/icmp.c b/net/ipv4/icmp.c index 3c1570d..1617604 100644 --- a/net/ipv4/icmp.c +++ b/net/ipv4/icmp.c @@ -968,8 +968,9 @@ static bool icmp_timestamp(struct sk_buff *skb) */ icmp_param.data.times[1] = inet_current_timestamp(); icmp_param.data.times[2] = icmp_param.data.times[1]; - if (skb_copy_bits(skb, 0, _param.data.times[0], 4)) - BUG(); + + BUG_ON(skb_copy_bits(skb, 0, _param.data.times[0], 4)); + icmp_param.data.icmph = *icmp_hdr(skb); icmp_param.data.icmph.type = ICMP_TIMESTAMPREPLY; icmp_param.data.icmph.code = 0; -- 2.7.4
[PATCH] ipv4: tcp_minisocks: use BUG_ON instead of if condition followed by BUG
Use BUG_ON instead of if condition followed by BUG in tcp_time_wait.
This issue was detected with the help of Coccinelle.
Signed-off-by: Gustavo A. R. Silva <garsi...@embeddedor.com>
---
net/ipv4/tcp_minisocks.c | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/net/ipv4/tcp_minisocks.c b/net/ipv4/tcp_minisocks.c
index 2341b9f..a952357 100644
--- a/net/ipv4/tcp_minisocks.c
+++ b/net/ipv4/tcp_minisocks.c
@@ -298,8 +298,7 @@ void tcp_time_wait(struct sock *sk, int state, int timeo)
key = tp->af_specific->md5_lookup(sk, sk);
if (key) {
tcptw->tw_md5_key = kmemdup(key, sizeof(*key),
GFP_ATOMIC);
- if (tcptw->tw_md5_key &&
!tcp_alloc_md5sig_pool())
- BUG();
+ BUG_ON(tcptw->tw_md5_key &&
!tcp_alloc_md5sig_pool());
}
} while (0);
#endif
--
2.7.4
[PATCH] net: sunrpc: svcauth_gss: use BUG_ON instead of if condition followed by BUG
Use BUG_ON instead of if condition followed by BUG.
This issue was detected with the help of Coccinelle.
Signed-off-by: Gustavo A. R. Silva <garsi...@embeddedor.com>
---
net/sunrpc/auth_gss/svcauth_gss.c | 9 +++--
1 file changed, 3 insertions(+), 6 deletions(-)
diff --git a/net/sunrpc/auth_gss/svcauth_gss.c
b/net/sunrpc/auth_gss/svcauth_gss.c
index 7b1ee5a..a10ce43 100644
--- a/net/sunrpc/auth_gss/svcauth_gss.c
+++ b/net/sunrpc/auth_gss/svcauth_gss.c
@@ -855,11 +855,9 @@ unwrap_integ_data(struct svc_rqst *rqstp, struct xdr_buf
*buf, u32 seq, struct g
return stat;
if (integ_len > buf->len)
return stat;
- if (xdr_buf_subsegment(buf, _buf, 0, integ_len))
- BUG();
+ BUG_ON(xdr_buf_subsegment(buf, _buf, 0, integ_len));
/* copy out mic... */
- if (read_u32_from_xdr_buf(buf, integ_len, ))
- BUG();
+ BUG_ON(read_u32_from_xdr_buf(buf, integ_len, ));
if (mic.len > RPC_MAX_AUTH_SIZE)
return stat;
mic.data = kmalloc(mic.len, GFP_KERNEL);
@@ -1611,8 +1609,7 @@ svcauth_gss_wrap_resp_integ(struct svc_rqst *rqstp)
BUG_ON(integ_len % 4);
*p++ = htonl(integ_len);
*p++ = htonl(gc->gc_seq);
- if (xdr_buf_subsegment(resbuf, _buf, integ_offset, integ_len))
- BUG();
+ BUG_ON(xdr_buf_subsegment(resbuf, _buf, integ_offset, integ_len));
if (resbuf->tail[0].iov_base == NULL) {
if (resbuf->head[0].iov_len + RPC_MAX_AUTH_SIZE > PAGE_SIZE)
goto out_err;
--
2.7.4
[PATCH] net: rxrpc: call_object: use BUG_ON instead of if condition followed by BUG
Use BUG_ON instead of if condition followed by BUG in rxrpc_release_call. This issue was detected with the help of Coccinelle. Signed-off-by: Gustavo A. R. Silva <garsi...@embeddedor.com> --- net/rxrpc/call_object.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/net/rxrpc/call_object.c b/net/rxrpc/call_object.c index fcdd655..b4b1a63 100644 --- a/net/rxrpc/call_object.c +++ b/net/rxrpc/call_object.c @@ -457,8 +457,7 @@ void rxrpc_release_call(struct rxrpc_sock *rx, struct rxrpc_call *call) ASSERTCMP(call->state, ==, RXRPC_CALL_COMPLETE); spin_lock_bh(>lock); - if (test_and_set_bit(RXRPC_CALL_RELEASED, >flags)) - BUG(); + BUG_ON(test_and_set_bit(RXRPC_CALL_RELEASED, >flags)); spin_unlock_bh(>lock); del_timer_sync(>timer); -- 2.7.4
[PATCH] openvswitch: meter: fix NULL pointer dereference in ovs_meter_cmd_reply_start
It seems that the intention of the code is to null check the value
returned by function genlmsg_put. But the current code is null
checking the address of the pointer that holds the value returned
by genlmsg_put.
Fix this by properly null checking the value returned by function
genlmsg_put in order to avoid a pontential null pointer dereference.
Addresses-Coverity-ID: 1461561 ("Dereference before null check")
Addresses-Coverity-ID: 1461562 ("Dereference null return value")
Fixes: 96fbc13d7e77 ("openvswitch: Add meter infrastructure")
Signed-off-by: Gustavo A. R. Silva <garsi...@embeddedor.com>
---
net/openvswitch/meter.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/net/openvswitch/meter.c b/net/openvswitch/meter.c
index 2a5ba35..bc0b6fc 100644
--- a/net/openvswitch/meter.c
+++ b/net/openvswitch/meter.c
@@ -106,7 +106,7 @@ ovs_meter_cmd_reply_start(struct genl_info *info, u8 cmd,
*ovs_reply_header = genlmsg_put(skb, info->snd_portid,
info->snd_seq,
_meter_genl_family, 0, cmd);
- if (!ovs_reply_header) {
+ if (!*ovs_reply_header) {
nlmsg_free(skb);
return ERR_PTR(-EMSGSIZE);
}
--
2.7.4
[PATCH] drm/amd/display: Fix potential NULL pointer dereferences in amdgpu_dm_atomic_commit_tail
dm_new_crtc_state->stream and disconnected_acrtc are being dereferenced
before they are null checked, hence there is a potential null pointer
dereference.
Fix this by null checking such pointers before they are dereferenced.
Addresses-Coverity-ID: 1423745 ("Dereference before null check")
Addresses-Coverity-ID: 1423833 ("Dereference before null check")
Fixes: e7b07ceef2a6 ("drm/amd/display: Merge amdgpu_dm_types and amdgpu_dm")
Fixes: 54d765752467 ("drm/amd/display: Unify amdgpu_dm state variable namings.")
Signed-off-by: Gustavo A. R. Silva <garsi...@embeddedor.com>
---
drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 12 +++-
1 file changed, 7 insertions(+), 5 deletions(-)
diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
index 889ed24..3bdd137 100644
--- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
+++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
@@ -4190,6 +4190,9 @@ static void amdgpu_dm_atomic_commit_tail(struct
drm_atomic_state *state)
dm_new_crtc_state = to_dm_crtc_state(new_crtc_state);
+ if (!dm_new_crtc_state->stream)
+ continue;
+
update_stream_scaling_settings(_new_con_state->base.crtc->mode,
dm_new_con_state, (struct dc_stream_state
*)dm_new_crtc_state->stream);
@@ -4197,9 +4200,6 @@ static void amdgpu_dm_atomic_commit_tail(struct
drm_atomic_state *state)
WARN_ON(!status);
WARN_ON(!status->plane_count);
- if (!dm_new_crtc_state->stream)
- continue;
-
/*TODO How it works with MPO ?*/
if (!dc_commit_planes_to_stream(
dm->dc,
@@ -4332,9 +4332,11 @@ void dm_restore_drm_connector_state(struct drm_device
*dev,
return;
disconnected_acrtc = to_amdgpu_crtc(connector->encoder->crtc);
- acrtc_state = to_dm_crtc_state(disconnected_acrtc->base.state);
+ if (!disconnected_acrtc)
+ return;
- if (!disconnected_acrtc || !acrtc_state->stream)
+ acrtc_state = to_dm_crtc_state(disconnected_acrtc->base.state);
+ if (!acrtc_state->stream)
return;
/*
--
2.7.4
Re: [PATCH] net: openvswitch: datapath: fix data type in queue_gso_packets
Quoting David Miller <da...@davemloft.net>:
From: Willem de Bruijn <willemdebruijn.ker...@gmail.com>
Date: Sat, 25 Nov 2017 16:15:01 -0500
On Sat, Nov 25, 2017 at 2:14 PM, Gustavo A. R. Silva
<garsi...@embeddedor.com> wrote:
gso_type is being used in binary AND operations together with SKB_GSO_UDP.
The issue is that variable gso_type is of type unsigned short and
SKB_GSO_UDP expands to more than 16 bits:
SKB_GSO_UDP = 1 << 16
this makes any binary AND operation between gso_type and SKB_GSO_UDP to
be always zero, hence making some code unreachable and likely causing
undesired behavior.
Fix this by changing the data type of variable gso_type to unsigned int.
Addresses-Coverity-ID: 1462223
Fixes: 0c19f846d582 ("net: accept UFO datagrams from tuntap and packet")
Signed-off-by: Gustavo A. R. Silva <garsi...@embeddedor.com>
Acked-by: Willem de Bruijn <will...@google.com>
Applied and I'll queued this up with Willem's changes for -stable.
Thanks!
Glad to help :)
Thanks
--
Gustavo A. R. Silva
Re: [PATCH] mfd: sm501: Add NULL check on devm_kzalloc return value
On 11/23/2017 03:37 AM, Linus Walleij wrote:
On Thu, Nov 23, 2017 at 4:50 AM, Gustavo A. R. Silva
<garsi...@embeddedor.com> wrote:
Check return value from call to devm_kzalloc() in order to prevent
a NULL pointer dereference.
This issue was detected with the help of Coccinelle.
Fixes: b2e6392f ("i2c: gpio: Convert to use descriptors")
Signed-off-by: Gustavo A. R. Silva <garsi...@embeddedor.com>
Acked-by: Linus Walleij <linus.wall...@linaro.org>
Thank you, Linus.
--
Gustavo A. R. Silva
[PATCH] scsi: lpfc: Fix potential NULL pointer dereference in lpfc_nvme_fcp_io_submit
pnvme_lport is being dereferenced before it is null checked, hence there
is a potential null pointer dereference.
Fix this by null checking pnvme_lport before it is dereferenced.
Addresses-Coverity-ID: 1423709 ("Dereference before null check")
Fixes: b7672ae681f8 ("scsi: lpfc: Fix crash in lpfc_nvme_fcp_io_submit during
LIP")
Signed-off-by: Gustavo A. R. Silva <garsi...@embeddedor.com>
---
Also, I wonder if the right pointer to check at line:
if (!pnvme_rport || !freqpriv) {
is pnvme_fcreq instead of freqpriv
drivers/scsi/lpfc/lpfc_nvme.c | 7 ++-
1 file changed, 6 insertions(+), 1 deletion(-)
diff --git a/drivers/scsi/lpfc/lpfc_nvme.c b/drivers/scsi/lpfc/lpfc_nvme.c
index 517ae57..68cba7d 100644
--- a/drivers/scsi/lpfc/lpfc_nvme.c
+++ b/drivers/scsi/lpfc/lpfc_nvme.c
@@ -1251,6 +1251,11 @@ lpfc_nvme_fcp_io_submit(struct nvme_fc_local_port
*pnvme_lport,
uint64_t start = 0;
#endif
+ if (!pnvme_lport) {
+ ret = -ENODEV;
+ goto out_fail;
+ }
+
lport = (struct lpfc_nvme_lport *)pnvme_lport->private;
vport = lport->vport;
phba = vport->phba;
@@ -1261,7 +1266,7 @@ lpfc_nvme_fcp_io_submit(struct nvme_fc_local_port
*pnvme_lport,
}
/* Validate pointers. */
- if (!pnvme_lport || !pnvme_rport || !freqpriv) {
+ if (!pnvme_rport || !freqpriv) {
lpfc_printf_vlog(vport, KERN_INFO, LOG_NVME_IOERR | LOG_NODE,
"6117 No Send:IO submit ptrs NULL, lport %p, "
"rport %p fcreq_priv %p\n",
--
2.7.4
[PATCH] rxrpc: call_event: Fix variable overwrite in __rxrpc_propose_ACK
Value assigned to variable ack_at is overwritten before it can be used.
Fix this by removing the value overwrite as it seems that this is a
leftover code.
Addresses-Coverity-ID: 1462263
Addresses-Coverity-ID: 1462264
Fixes: beb8e5e4f38c ("rxrpc: Express protocol timeouts in terms of RTT")
Signed-off-by: Gustavo A. R. Silva <garsi...@embeddedor.com>
---
net/rxrpc/call_event.c | 1 -
1 file changed, 1 deletion(-)
diff --git a/net/rxrpc/call_event.c b/net/rxrpc/call_event.c
index bda952f..03074ba 100644
--- a/net/rxrpc/call_event.c
+++ b/net/rxrpc/call_event.c
@@ -123,7 +123,6 @@ static void __rxrpc_propose_ACK(struct rxrpc_call *call, u8
ack_reason,
else
ack_at = expiry;
- ack_at = jiffies + expiry;
if (time_before(ack_at, call->ack_at)) {
WRITE_ONCE(call->ack_at, ack_at);
rxrpc_reduce_call_timer(call, ack_at, now,
--
2.7.4
[PATCH] rxrpc: sendmsg: Fix variable overwrite in rxrpc_queue_packet
Value assigned to variable resend_at is overwritten before it can be used.
Fix this by removing the value overwrite as it seems that this is a
leftover code.
Addresses-Coverity-ID: 1462262
Fixes: beb8e5e4f38c ("rxrpc: Express protocol timeouts in terms of RTT")
Signed-off-by: Gustavo A. R. Silva <garsi...@embeddedor.com>
---
net/rxrpc/sendmsg.c | 1 -
1 file changed, 1 deletion(-)
diff --git a/net/rxrpc/sendmsg.c b/net/rxrpc/sendmsg.c
index a1c53ac..42060dd 100644
--- a/net/rxrpc/sendmsg.c
+++ b/net/rxrpc/sendmsg.c
@@ -233,7 +233,6 @@ static void rxrpc_queue_packet(struct rxrpc_sock *rx,
struct rxrpc_call *call,
if (resend_at < 1)
resend_at = 1;
- resend_at = now + rxrpc_resend_timeout;
WRITE_ONCE(call->resend_at, resend_at);
rxrpc_reduce_call_timer(call, resend_at, now,
rxrpc_timer_set_for_send);
--
2.7.4
Re: [PATCH] rxrpc: sendmsg: Fix variable overwrite in rxrpc_queue_packet
Hi David, Quoting David Howells <dhowe...@redhat.com>: Gustavo A. R. Silva <garsi...@embeddedor.com> wrote: Value assigned to variable resend_at is overwritten before it can be used. Fix this by removing the value overwrite as it seems that this is a leftover code. NAK. Your fix will actually cause the code to break. The resend_at value used for the timer must be based on the current time (ie. jiffies in this case), so we can't simply remove that line as the previously calculated resend_at value is a duration, not a time. What you need to do is to instead modify the line you wanted to remove to add 'now' to the previously-computed value. You mean something like the following? --- a/net/rxrpc/sendmsg.c +++ b/net/rxrpc/sendmsg.c @@ -233,7 +233,7 @@ static void rxrpc_queue_packet(struct rxrpc_sock *rx, struct rxrpc_call *call, if (resend_at < 1) resend_at = 1; - resend_at = now + rxrpc_resend_timeout; + resend_at += now; WRITE_ONCE(call->resend_at, resend_at); rxrpc_reduce_call_timer(call, resend_at, now, rxrpc_timer_set_for_send); Thanks -- Gustavo A. R. Silva
Re: [PATCH] x86/syscalls: Mark expected switch fall-throughs
Quoting Thomas Gleixner <t...@linutronix.de>: On Tue, 28 Nov 2017, Thomas Gleixner wrote: On Tue, 28 Nov 2017, Gustavo A. R. Silva wrote: > Quoting Thomas Gleixner <t...@linutronix.de>: > > > On Mon, 27 Nov 2017, Gustavo A. R. Silva wrote: > > > > > In preparation to enabling -Wimplicit-fallthrough, mark switch cases > > > where we are expecting to fall through. > > > > > case 0: > > > if (!n--) break; > > > *args++ = regs->bx; > > > +/* fall through */ > > > > And these gazillions of pointless comments help enabling of > > -Wimplicit-fallthrough in which way? > > > > The -Wimplicit-fallthrough option was added to GCC 7. We want to add that > option to the top-level Makefile so we can have the compiler help us not make > mistakes as missing "break"s or "continue"s. This also documents the intention > for humans and provides a way for analyzers to report issues or ignore False > Positives. > > So prior to adding such option to the Makefile, we have to properly add a code > comment wherever the code is intended to fall through. > > During the process of placing these comments I have identified actual bugs > (missing "break"s/"continue"s) in a variety of components in the kernel, so I > think this effort is valuable. Lastly, such a simple comment in the code can > save a person plenty of time during a code review. To be honest, such comments annoy me during a code review especially when the fallthrough is so obvious as in this case. There might be cases where its worth to document because it's non obvious, but documenting the obvious just for the sake of documenting it is just wrong. I understand that and I agree that in this particular case it is just obvious. The thing is that if we want to benefit from having the compiler help us to spot these kind of issues before committing our code, we have to address every place in the whole code-base. And _IF_ at all then you want a fixed macro for this and not a comment which will be formatted as people see it fit. GCC supports: __attribute__ ((fallthrough)) which we can wrap into a macro, e.g. falltrough() That'd be useful, but adding all these comments and then having to chase a gazillion of warning instances to figure out whether there is a comment or not is just backwards. I have run into this before and people find what you suggest even uglier. Thanks -- Gustavo A. R. Silva
Re: [PATCH] rxrpc: call_event: Fix variable overwrite in __rxrpc_propose_ACK
Quoting David Howells <dhowe...@redhat.com>: Gustavo A. R. Silva <garsi...@embeddedor.com> wrote: - ack_at = jiffies + expiry; Same issue as with the other patch. Can you just combine the two please? Sure. I'll send a patch shortly. Thanks -- Gustavo A. R. Silva
[PATCH v3] rxrpc: Fix variable overwrite
Values assigned to both variable resend_at and ack_at are overwritten
before they can be used.
The correct fix here is to add 'now' to the previously computed value in
resend_at and ack_at.
Addresses-Coverity-ID: 1462262
Addresses-Coverity-ID: 1462263
Addresses-Coverity-ID: 1462264
Fixes: beb8e5e4f38c ("rxrpc: Express protocol timeouts in terms of RTT")
Link: https://marc.info/?i=17004.1511808959%40warthog.procyon.org.uk
Signed-off-by: Gustavo A. R. Silva <garsi...@embeddedor.com>
---
Changes in v2:
- Add 'now' to the previously-computed value stored in resend_at instead
of removing the code.
Changes in v3:
- Add 'now' to the previously-computed value stored in ack_at instead of
removing the code.
- Combine two previously sent patches into this one.
net/rxrpc/call_event.c | 2 +-
net/rxrpc/sendmsg.c| 2 +-
2 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/net/rxrpc/call_event.c b/net/rxrpc/call_event.c
index bda952f..9cc1225 100644
--- a/net/rxrpc/call_event.c
+++ b/net/rxrpc/call_event.c
@@ -123,7 +123,7 @@ static void __rxrpc_propose_ACK(struct rxrpc_call *call, u8
ack_reason,
else
ack_at = expiry;
- ack_at = jiffies + expiry;
+ ack_at += now;
if (time_before(ack_at, call->ack_at)) {
WRITE_ONCE(call->ack_at, ack_at);
rxrpc_reduce_call_timer(call, ack_at, now,
diff --git a/net/rxrpc/sendmsg.c b/net/rxrpc/sendmsg.c
index a1c53ac..09f2a3e 100644
--- a/net/rxrpc/sendmsg.c
+++ b/net/rxrpc/sendmsg.c
@@ -233,7 +233,7 @@ static void rxrpc_queue_packet(struct rxrpc_sock *rx,
struct rxrpc_call *call,
if (resend_at < 1)
resend_at = 1;
- resend_at = now + rxrpc_resend_timeout;
+ resend_at += now;
WRITE_ONCE(call->resend_at, resend_at);
rxrpc_reduce_call_timer(call, resend_at, now,
rxrpc_timer_set_for_send);
--
2.7.4
Re: [PATCH] x86/syscalls: Mark expected switch fall-throughs
Quoting Thomas Gleixner <t...@linutronix.de>: On Mon, 27 Nov 2017, Gustavo A. R. Silva wrote: In preparation to enabling -Wimplicit-fallthrough, mark switch cases where we are expecting to fall through. case 0: if (!n--) break; *args++ = regs->bx; + /* fall through */ And these gazillions of pointless comments help enabling of -Wimplicit-fallthrough in which way? The -Wimplicit-fallthrough option was added to GCC 7. We want to add that option to the top-level Makefile so we can have the compiler help us not make mistakes as missing "break"s or "continue"s. This also documents the intention for humans and provides a way for analyzers to report issues or ignore False Positives. So prior to adding such option to the Makefile, we have to properly add a code comment wherever the code is intended to fall through. During the process of placing these comments I have identified actual bugs (missing "break"s/"continue"s) in a variety of components in the kernel, so I think this effort is valuable. Lastly, such a simple comment in the code can save a person plenty of time during a code review. Thanks -- Gustavo A. R. Silva
Re: [PATCH] rxrpc: sendmsg: Fix variable overwrite in rxrpc_queue_packet
Quoting David Howells <dhowe...@redhat.com>: Gustavo A. R. Silva <garsi...@embeddedor.com> wrote: - resend_at = now + rxrpc_resend_timeout; + resend_at += now; Yep. :-) Great! What about this one: https://lkml.org/lkml/2017/11/27/810 applies the same? Thanks -- Gustavo A. R. Silva
Re: [PATCH] x86/syscalls: Mark expected switch fall-throughs
Quoting Thomas Gleixner <t...@linutronix.de>: On Tue, 28 Nov 2017, Alan Cox wrote: > I have no idea who came up with that brilliant idea of parsing comments in > the code. It's so simple to make this parser completely fail that it's not Stephen Johnson (author of the V7 portable C compiler), which is where it's from (the lint tool). He also wrote yacc so he does know a bit about parsers 8). I don't doubt that. > even funny anymore. The notation in question has been standard in tools like lint since the end of the 1970s Fair enough. Still that does not make the GCC implementation which defaults to take 'any comment' as valid any better and does not solve other parsing issues which have been pointed out in various GCC bugs. Using the macro annotation is distinct and has no ifs and buts. The thing about taking 'any comment' as valid is false if you add the following to your Makefile: KBUILD_CFLAGS += $(call cc-option,-Wimplicit-fallthrough) This option takes the following comments as valid: /* fall through */ /* Fall through */ /* fall through - ... */ /* Fall through - ... */ Comments as fallthru, fallthrough, FALLTHRU are invalid. And of course if you intentionally change the option to: KBUILD_CFLAGS += $(call cc-option,-Wimplicit-fallthrough=1) it means that you obviously want to ignore any warning. Thanks -- Gustavo A. R. Silva
Re: [PATCH] x86/syscalls: Mark expected switch fall-throughs
Quoting Thomas Gleixner <t...@linutronix.de>: On Tue, 28 Nov 2017, Thomas Gleixner wrote: +CC Linus. On Tue, 28 Nov 2017, Thomas Gleixner wrote: > On Tue, 28 Nov 2017, Gustavo A. R. Silva wrote: > > Quoting Thomas Gleixner <t...@linutronix.de>: > > > > To be honest, such comments annoy me during a code review especially when > > > > the fallthrough is so obvious as in this case. There might be cases where > > > > its worth to document because it's non obvious, but documenting the > > > > obvious > > > > just for the sake of documenting it is just wrong. > > > > > > > I understand that and I agree that in this particular case it is just obvious. > > The thing is that if we want to benefit from having the compiler help us to > > spot these kind of issues before committing our code, we have to address every > > place in the whole code-base. > > > > > And _IF_ at all then you want a fixed macro for this and not a comment > > > which will be formatted as people see it fit. > > > > > > GCC supports: __attribute__ ((fallthrough)) which we can wrap into a macro, > > > e.g. falltrough() > > > > > > That'd be useful, but adding all these comments and then having to chase a > > > gazillion of warning instances to figure out whether there is a comment or > > > not is just backwards. > > > > > > > I have run into this before and people find what you suggest even uglier. > > It's not about ugly. It's about _USEFULL_. > > The comments are ugly AND completely useless for the compiler and they are > going to be malformatted so checker tools can't differentiate the false > positives. > > The macro, in which more or less ugly form written, is both documentation > and helps the compiler NOT to emit the same crap over and over. Just checked and GCC really supports analyzing the comment to some extent. But just look at https://gcc.gnu.org/bugzilla/show_bug.cgi?id=77817 " It is not really possible. __attribute__((fallthrough)) has precise rules on where it can appear, while /* FALLTHRU */ comments, being comments, can appear anywhere. Especially with -Wimplicit-fallthrough=1 when all comments are considered fallthru comments... " This is what we want to add: # Warn about missing switch break or fall-through comment. KBUILD_CFLAGS += $(call cc-option,-Wimplicit-fallthrough) I have no idea who came up with that brilliant idea of parsing comments in the code. It's so simple to make this parser completely fail that it's not even funny anymore. I don't get why someone would want to do that to himself. :/ I don't care what other people prefer. The code base I'm responsible for gets either proper annotations or nothing. And in fact we want ONE solution for the whole kernel. And comments are obviously the wrong one. OK. I'll discuss this and see how we can come up with the best solution. Thank you for your feedback -- Gustavo A. R. Silva
Re: [PATCH] x86/syscalls: Mark expected switch fall-throughs
Quoting Linus Torvalds <torva...@linux-foundation.org>: On Tue, Nov 28, 2017 at 11:00 AM, Alan Cox <gno...@lxorguk.ukuu.org.uk> wrote: The notation in question has been standard in tools like lint since the end of the 1970s Yes. That said, maybe one option would be to annotate the "case:" and "default:" statements if that makes people happier. IOW, we could do something like #define fallthrough __atttibute__((fallthrough)) and then write fallthrough case 1: ... which while absolutely not traditional, might look and read a bit more logical to people. I mean, it literally _is_ a "fallthrough case", so it makes semantic sense. This is elegant. The thing is that this makes it appear as if there is an unconditional fall through. It is not uncommon to have multiple break statements in the same case block and to fall through also. Thanks -- Gustavo A. R. Silva
[PATCH] net: openvswitch: datapath: fix data type in queue_gso_packets
gso_type is being used in binary AND operations together with SKB_GSO_UDP.
The issue is that variable gso_type is of type unsigned short and
SKB_GSO_UDP expands to more than 16 bits:
SKB_GSO_UDP = 1 << 16
this makes any binary AND operation between gso_type and SKB_GSO_UDP to
be always zero, hence making some code unreachable and likely causing
undesired behavior.
Fix this by changing the data type of variable gso_type to unsigned int.
Addresses-Coverity-ID: 1462223
Fixes: 0c19f846d582 ("net: accept UFO datagrams from tuntap and packet")
Signed-off-by: Gustavo A. R. Silva <garsi...@embeddedor.com>
---
net/openvswitch/datapath.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/net/openvswitch/datapath.c b/net/openvswitch/datapath.c
index 99cfafc..ef38e5a 100644
--- a/net/openvswitch/datapath.c
+++ b/net/openvswitch/datapath.c
@@ -308,7 +308,7 @@ static int queue_gso_packets(struct datapath *dp, struct
sk_buff *skb,
const struct dp_upcall_info *upcall_info,
uint32_t cutlen)
{
- unsigned short gso_type = skb_shinfo(skb)->gso_type;
+ unsigned int gso_type = skb_shinfo(skb)->gso_type;
struct sw_flow_key later_key;
struct sk_buff *segs, *nskb;
int err;
--
2.7.4
Re: [PATCH] au0828: fix use-after-free at USB probing
Hi Andrey,
I have successfully installed and tested syzkaller with QEMU. Can you
please tell me how to reproduce this bug or share with me the full
crash report?
Also, can you point me out to the PoC file?
Much appreciated
Thank you!
--
Gustavo A. R. Silva
Quoting Andrey Konovalov <andreyk...@google.com>:
On Fri, Nov 10, 2017 at 6:35 PM, Gustavo A. R. Silva
<garsi...@embeddedor.com> wrote:
Quoting Andrey Konovalov <andreyk...@google.com>:
On Fri, Nov 10, 2017 at 1:21 AM, Gustavo A. R. Silva
<garsi...@embeddedor.com> wrote:
Hi Andrey,
Could you please try this patch?
Thank you
Hi!
Sorry for the delay.
With this patch I still see the same report:
au0828: recv_control_msg() Failed receiving control message, error -71.
au0828: recv_control_msg() Failed receiving control message, error -71.
au0828: recv_control_msg() Failed receiving control message, error -71.
au8522_writereg: writereg error (reg == 0x106, val == 0x0001, ret == -5)
usb 1-1: selecting invalid altsetting 5
au0828: Failure setting usb interface0 to as5
au0828: au0828_usb_probe() au0282_dev_register failed to register on V4L2
au0828: probe of 1-1:0.0 failed with error -22
usb 1-1: USB disconnect, device number 3
==
BUG: KASAN: use-after-free in __list_del_entry_valid+0xda/0xf3
Read of size 8 at addr 880062a74410 by task kworker/0:1/24
CPU: 0 PID: 24 Comm: kworker/0:1 Not tainted
4.14.0-rc8-44455-ge2105594a876-dirty #111
Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS Bochs 01/01/2011
Workqueue: usb_hub_wq hub_event
Call Trace:
__dump_stack lib/dump_stack.c:17
dump_stack+0xe1/0x157 lib/dump_stack.c:53
print_address_description+0x71/0x234 mm/kasan/report.c:252
kasan_report_error mm/kasan/report.c:351
kasan_report+0x173/0x270 mm/kasan/report.c:409
__asan_report_load8_noabort+0x19/0x20 mm/kasan/report.c:430
__list_del_entry_valid+0xda/0xf3 lib/list_debug.c:54
__list_del_entry ./include/linux/list.h:117
list_del_init ./include/linux/list.h:159
device_pm_remove+0x4a/0x1e7 drivers/base/power/main.c:149
device_del+0x599/0xa70 drivers/base/core.c:1986
usb_disable_device+0x223/0x710 drivers/usb/core/message.c:1170
usb_disconnect+0x285/0x7f0 drivers/usb/core/hub.c:2205
hub_port_connect drivers/usb/core/hub.c:4838
hub_port_connect_change drivers/usb/core/hub.c:5093
port_event drivers/usb/core/hub.c:5199
hub_event_impl+0x10ec/0x3440 drivers/usb/core/hub.c:5311
hub_event+0x38/0x50 drivers/usb/core/hub.c:5209
process_one_work+0x925/0x15d0 kernel/workqueue.c:2113
process_scheduled_works kernel/workqueue.c:2173
worker_thread+0x72e/0x10d0 kernel/workqueue.c:2249
kthread+0x346/0x410 kernel/kthread.c:231
ret_from_fork+0x2a/0x40 arch/x86/entry/entry_64.S:432
The buggy address belongs to the page:
page:ea00018a9d00 count:0 mapcount:-127 mapping: (null)
index:0x0
flags: 0x100()
raw: 0100 ff80
raw: 88007fffa690 ea00018e6120 0002
page dumped because: kasan: bad access detected
Memory state around the buggy address:
880062a74300: ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff
880062a74380: ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff
880062a74400: ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff
^
880062a74480: ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff
880062a74500: ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff
=
Thanks!
Hi Gustavo,
With your patch I get a different crash. Not sure if it's another bug
or the same one manifesting differently.
That's the same one. It seems that the best solution is to remove the kfree
after the mutex_unlock and let the device resources be freed in
au0828_usb_disconnect.
Please try the following patch instead.
I appreciate your help.
Thank you, Andrey.
---
drivers/media/usb/au0828/au0828-core.c | 1 -
1 file changed, 1 deletion(-)
diff --git a/drivers/media/usb/au0828/au0828-core.c
b/drivers/media/usb/au0828/au0828-core.c
index cd363a2..257ae0d 100644
--- a/drivers/media/usb/au0828/au0828-core.c
+++ b/drivers/media/usb/au0828/au0828-core.c
@@ -629,7 +629,6 @@ static int au0828_usb_probe(struct usb_interface
*interface,
pr_err("%s() au0282_dev_register failed to register on
V4L2\n",
__func__);
mutex_unlock(>lock);
- kfree(dev);
goto done;
}
--
2.7.4
au0828: recv_control_msg() Failed receiving control message, error -71.
au0828: recv_control_msg() Failed receiving control message, error -71.
au8522_writereg: writereg error (reg == 0x106, val == 0x0001, ret == -5)
usb 1-1: selecting invalid altsetting 5
au0828: Failure setting usb interface0 to as5
au0828: au0828_usb_probe() au0282_dev_register failed to register on
[PATCH] mfd: sm501: Add NULL check on devm_kzalloc return value
Check return value from call to devm_kzalloc() in order to prevent
a NULL pointer dereference.
This issue was detected with the help of Coccinelle.
Fixes: b2e6392f ("i2c: gpio: Convert to use descriptors")
Signed-off-by: Gustavo A. R. Silva <garsi...@embeddedor.com>
---
drivers/mfd/sm501.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/drivers/mfd/sm501.c b/drivers/mfd/sm501.c
index ad77416..08bee41 100644
--- a/drivers/mfd/sm501.c
+++ b/drivers/mfd/sm501.c
@@ -1144,6 +1144,9 @@ static int sm501_register_gpio_i2c_instance(struct
sm501_devdata *sm,
lookup = devm_kzalloc(>dev,
sizeof(*lookup) + 3 * sizeof(struct gpiod_lookup),
GFP_KERNEL);
+ if (!lookup)
+ return -ENOMEM;
+
lookup->dev_id = "i2c-gpio";
if (iic->pin_sda < 32)
lookup->table[0].chip_label = "SM501-LOW";
--
2.7.4
[PATCH] davinci: vpif_capture: add NULL check on devm_kzalloc return value
Check return value from call to devm_kzalloc() in order to prevent
a NULL pointer dereference.
This issue was detected with the help of Coccinelle.
Fixes: 4a5f8ae50b66 ("[media] davinci: vpif_capture: get subdevs from DT when
available")
Signed-off-by: Gustavo A. R. Silva <garsi...@embeddedor.com>
---
drivers/media/platform/davinci/vpif_capture.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/drivers/media/platform/davinci/vpif_capture.c
b/drivers/media/platform/davinci/vpif_capture.c
index a89367a..3879c7c 100644
--- a/drivers/media/platform/davinci/vpif_capture.c
+++ b/drivers/media/platform/davinci/vpif_capture.c
@@ -1550,6 +1550,8 @@ vpif_capture_get_pdata(struct platform_device *pdev)
sizeof(*chan->inputs) *
VPIF_CAPTURE_NUM_CHANNELS,
GFP_KERNEL);
+ if (!chan->inputs)
+ return NULL;
chan->input_count++;
chan->inputs[i].input.type = V4L2_INPUT_TYPE_CAMERA;
--
2.7.4
Re: [PATCH] scsi: ufs: ufshcd: fix potential NULL pointer dereference in ufshcd_config_vreg
On 11/21/2017 10:01 PM, Martin K. Petersen wrote: Gustavo A., _vreg_ is being dereferenced before it is null checked, hence there is a potential null pointer dereference. Fix this by moving the pointer dereference after _vreg_ has been null checked. Applied to 4.15/scsi-fixes, thank you! Glad to help. :) Thanks -- Gustavo A. R. Silva
Re: [PATCH] c8sectpfe: fix potential NULL pointer dereference in c8sectpfe_timer_interrupt
On 11/21/2017 02:22 AM, Patrice CHOTARD wrote:
Hi Gustavo
On 11/20/2017 03:00 PM, Gustavo A. R. Silva wrote:
_channel_ is being dereferenced before it is null checked, hence there is a
potential null pointer dereference. Fix this by moving the pointer dereference
after _channel_ has been null checked.
This issue was detected with the help of Coccinelle.
Fixes: c5f5d0f99794 ("[media] c8sectpfe: STiH407/10 Linux DVB demux support")
Signed-off-by: Gustavo A. R. Silva <garsi...@embeddedor.com>
---
drivers/media/platform/sti/c8sectpfe/c8sectpfe-core.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/drivers/media/platform/sti/c8sectpfe/c8sectpfe-core.c
b/drivers/media/platform/sti/c8sectpfe/c8sectpfe-core.c
index 59280ac..23d0ced 100644
--- a/drivers/media/platform/sti/c8sectpfe/c8sectpfe-core.c
+++ b/drivers/media/platform/sti/c8sectpfe/c8sectpfe-core.c
@@ -83,7 +83,7 @@ static void c8sectpfe_timer_interrupt(unsigned long
ac8sectpfei)
static void channel_swdemux_tsklet(unsigned long data)
{
struct channel_info *channel = (struct channel_info *)data;
- struct c8sectpfei *fei = channel->fei;
+ struct c8sectpfei *fei;
unsigned long wp, rp;
int pos, num_packets, n, size;
u8 *buf;
@@ -91,6 +91,8 @@ static void channel_swdemux_tsklet(unsigned long data)
if (unlikely(!channel || !channel->irec))
return;
+ fei = channel->fei;
+
wp = readl(channel->irec + DMA_PRDS_BUSWP_TP(0));
rp = readl(channel->irec + DMA_PRDS_BUSRP_TP(0));
Acked-by: Patrice Chotard <patrice.chot...@st.com>
Thanks
Thank you, Patrice.
--
Gustavo A. R. Silva
Re: [PATCH] x86/syscalls: Mark expected switch fall-throughs
Quoting Thomas Gleixner <t...@linutronix.de>: So I have to ask WHY this information was not in the changelog of the patch in question: 1) How it works 2) Why comments have been chosen over macros I will add this info and send the patch again. In preparation to enabling -Wimplicit-fallthrough, mark switch cases where we are expecting to fall through. It's not a reviewers job to chase that information down. While I can understand that the comments are intentional due to existing tools, I still prefer the macro/annotation. But I'm not religious about it when there is common consensus. :) Awesome Thanks, Thomas. -- Gustavo A. R. Silva
[PATCH] lib: zstd: bitstream: 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>
---
lib/zstd/bitstream.h | 6 ++
1 file changed, 6 insertions(+)
diff --git a/lib/zstd/bitstream.h b/lib/zstd/bitstream.h
index a826b99..18d8200 100644
--- a/lib/zstd/bitstream.h
+++ b/lib/zstd/bitstream.h
@@ -259,11 +259,17 @@ ZSTD_STATIC size_t BIT_initDStream(BIT_DStream_t *bitD,
const void *srcBuffer, s
bitD->bitContainer = *(const BYTE *)(bitD->start);
switch (srcSize) {
case 7: bitD->bitContainer += (size_t)(((const BYTE
*)(srcBuffer))[6]) << (sizeof(bitD->bitContainer) * 8 - 16);
+ /* fall through */
case 6: bitD->bitContainer += (size_t)(((const BYTE
*)(srcBuffer))[5]) << (sizeof(bitD->bitContainer) * 8 - 24);
+ /* fall through */
case 5: bitD->bitContainer += (size_t)(((const BYTE
*)(srcBuffer))[4]) << (sizeof(bitD->bitContainer) * 8 - 32);
+ /* fall through */
case 4: bitD->bitContainer += (size_t)(((const BYTE
*)(srcBuffer))[3]) << 24;
+ /* fall through */
case 3: bitD->bitContainer += (size_t)(((const BYTE
*)(srcBuffer))[2]) << 16;
+ /* fall through */
case 2: bitD->bitContainer += (size_t)(((const BYTE
*)(srcBuffer))[1]) << 8;
+ /* fall through */
default:;
}
{
--
2.7.4
[PATCH] x86/syscalls: 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> --- arch/x86/include/asm/syscall.h | 28 1 file changed, 28 insertions(+) diff --git a/arch/x86/include/asm/syscall.h b/arch/x86/include/asm/syscall.h index e3c95e8..63b01b1 100644 --- a/arch/x86/include/asm/syscall.h +++ b/arch/x86/include/asm/syscall.h @@ -121,23 +121,30 @@ static inline void syscall_get_arguments(struct task_struct *task, case 0: if (!n--) break; *args++ = regs->bx; + /* fall through */ case 1: if (!n--) break; *args++ = regs->cx; + /* fall through */ case 2: if (!n--) break; *args++ = regs->dx; + /* fall through */ case 3: if (!n--) break; *args++ = regs->si; + /* fall through */ case 4: if (!n--) break; *args++ = regs->di; + /* fall through */ case 5: if (!n--) break; *args++ = regs->bp; + /* fall through */ case 6: if (!n--) break; + /* fall through */ default: BUG(); break; @@ -148,23 +155,30 @@ static inline void syscall_get_arguments(struct task_struct *task, case 0: if (!n--) break; *args++ = regs->di; + /* fall through */ case 1: if (!n--) break; *args++ = regs->si; + /* fall through */ case 2: if (!n--) break; *args++ = regs->dx; + /* fall through */ case 3: if (!n--) break; *args++ = regs->r10; + /* fall through */ case 4: if (!n--) break; *args++ = regs->r8; + /* fall through */ case 5: if (!n--) break; *args++ = regs->r9; + /* fall through */ case 6: if (!n--) break; + /* fall through */ default: BUG(); break; @@ -182,23 +196,30 @@ static inline void syscall_set_arguments(struct task_struct *task, case 0: if (!n--) break; regs->bx = *args++; + /* fall through */ case 1: if (!n--) break; regs->cx = *args++; + /* fall through */ case 2: if (!n--) break; regs->dx = *args++; + /* fall through */ case 3: if (!n--) break; regs->si = *args++; + /* fall through */ case 4: if (!n--) break; regs->di = *args++; + /* fall through */ case 5: if (!n--) break; regs->bp = *args++; + /* fall through */ case 6: if (!n--) break; + /* fall through */ default: BUG(); break; @@ -209,23 +230,30 @@ static inline void syscall_set_arguments(struct task_struct *task, case 0: if (!n--) break; regs->di = *args++; + /* fall through */ case 1: if (!n--) break; regs->si = *args++; + /* fall through */ case 2: if (!n--) break; regs->dx = *args++; + /* fall through */ case 3: if (!n--) break; regs->r10 = *args++; + /* fall through */ case 4: if (!n--) break; regs->r8 = *args++; + /* fall through */ case 5
[PATCH v2] rxrpc: sendmsg: Fix variable overwrite in rxrpc_queue_packet
Value assigned to variable resend_at is overwritten before it can be used.
The correct fix here is to add _now_ to the previously computed value in
resend_at.
Addresses-Coverity-ID: 1462262
Fixes: beb8e5e4f38c ("rxrpc: Express protocol timeouts in terms of RTT")
Link: https://marc.info/?i=17004.1511808959%40warthog.procyon.org.uk
Signed-off-by: Gustavo A. R. Silva <garsi...@embeddedor.com>
---
Changes in v2:
Update value in resend_at instead of removing the code.
net/rxrpc/sendmsg.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/net/rxrpc/sendmsg.c b/net/rxrpc/sendmsg.c
index a1c53ac..09f2a3e 100644
--- a/net/rxrpc/sendmsg.c
+++ b/net/rxrpc/sendmsg.c
@@ -233,7 +233,7 @@ static void rxrpc_queue_packet(struct rxrpc_sock *rx,
struct rxrpc_call *call,
if (resend_at < 1)
resend_at = 1;
- resend_at = now + rxrpc_resend_timeout;
+ resend_at += now;
WRITE_ONCE(call->resend_at, resend_at);
rxrpc_reduce_call_timer(call, resend_at, now,
rxrpc_timer_set_for_send);
--
2.7.4
[PATCH] drm/i915: Mark expected switch fall-throughs
In preparation to enabling -Wimplicit-fallthrough, mark switch cases
where we are expecting to fall through.
Addresses-Coverity-ID: 141432
Addresses-Coverity-ID: 141433
Addresses-Coverity-ID: 141434
Addresses-Coverity-ID: 141435
Addresses-Coverity-ID: 141436
Addresses-Coverity-ID: 1357360
Addresses-Coverity-ID: 1357403
Addresses-Coverity-ID: 1357433
Addresses-Coverity-ID: 1392622
Addresses-Coverity-ID: 1415273
Addresses-Coverity-ID: 1435752
Addresses-Coverity-ID: 1441500
Addresses-Coverity-ID: 1454596
Signed-off-by: Gustavo A. R. Silva <garsi...@embeddedor.com>
---
drivers/gpu/drm/i915/i915_gem.c | 3 ++-
drivers/gpu/drm/i915/intel_cdclk.c | 4
drivers/gpu/drm/i915/intel_display.c| 2 ++
drivers/gpu/drm/i915/intel_drv.h| 1 +
drivers/gpu/drm/i915/intel_engine_cs.c | 2 ++
drivers/gpu/drm/i915/intel_runtime_pm.c | 1 +
drivers/gpu/drm/i915/intel_sdvo.c | 6 ++
7 files changed, 18 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index 3a140ee..326cf16 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -1977,6 +1977,7 @@ int i915_gem_fault(struct vm_fault *vmf)
ret = VM_FAULT_SIGBUS;
break;
}
+ /* fall through */
case -EAGAIN:
/*
* EAGAIN means the gpu is hung and we'll wait for the error
@@ -2644,7 +2645,7 @@ static void *i915_gem_object_map(const struct
drm_i915_gem_object *obj,
switch (type) {
default:
MISSING_CASE(type);
- /* fallthrough to use PAGE_KERNEL anyway */
+ /* fall through - To use PAGE_KERNEL anyway */
case I915_MAP_WB:
pgprot = PAGE_KERNEL;
break;
diff --git a/drivers/gpu/drm/i915/intel_cdclk.c
b/drivers/gpu/drm/i915/intel_cdclk.c
index b2a6d62..5879cd3 100644
--- a/drivers/gpu/drm/i915/intel_cdclk.c
+++ b/drivers/gpu/drm/i915/intel_cdclk.c
@@ -316,6 +316,7 @@ static void pnv_get_cdclk(struct drm_i915_private *dev_priv,
break;
default:
DRM_ERROR("Unknown pnv display core clock 0x%04x\n", gcfgc);
+ /* fall through */
case GC_DISPLAY_CLOCK_133_MHZ_PNV:
cdclk_state->cdclk = 13;
break;
@@ -1110,6 +,7 @@ static int bxt_de_pll_vco(struct drm_i915_private
*dev_priv, int cdclk)
switch (cdclk) {
default:
MISSING_CASE(cdclk);
+ /* fall through */
case 144000:
case 288000:
case 384000:
@@ -1134,6 +1136,7 @@ static int glk_de_pll_vco(struct drm_i915_private
*dev_priv, int cdclk)
switch (cdclk) {
default:
MISSING_CASE(cdclk);
+ /* fall through */
case 79200:
case 158400:
case 316800:
@@ -1592,6 +1595,7 @@ static int cnl_cdclk_pll_vco(struct drm_i915_private
*dev_priv, int cdclk)
switch (cdclk) {
default:
MISSING_CASE(cdclk);
+ /* fall through */
case 168000:
case 336000:
ratio = dev_priv->cdclk.hw.ref == 19200 ? 35 : 28;
diff --git a/drivers/gpu/drm/i915/intel_display.c
b/drivers/gpu/drm/i915/intel_display.c
index 878acc4..1f7907f 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -9049,6 +9049,7 @@ static bool hsw_get_transcoder_state(struct intel_crtc
*crtc,
switch (tmp & TRANS_DDI_EDP_INPUT_MASK) {
default:
WARN(1, "unknown pipe linked to edp transcoder\n");
+ /* fall through */
case TRANS_DDI_EDP_INPUT_A_ONOFF:
case TRANS_DDI_EDP_INPUT_A_ON:
trans_edp_pipe = PIPE_A;
@@ -10751,6 +10752,7 @@ static bool check_digital_port_conflicts(struct
drm_atomic_state *state)
case INTEL_OUTPUT_UNKNOWN:
if (WARN_ON(!HAS_DDI(to_i915(dev
break;
+ /* fall through */
case INTEL_OUTPUT_DP:
case INTEL_OUTPUT_HDMI:
case INTEL_OUTPUT_EDP:
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index 7bc60c8..4c3696c 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -1149,6 +1149,7 @@ enc_to_dig_port(struct drm_encoder *encoder)
switch (intel_encoder->type) {
case INTEL_OUTPUT_UNKNOWN:
WARN_ON(!HAS_DDI(to_i915(encoder->dev)));
+ /* fall through */
case INTEL_OUTPUT_DP:
case INTEL_OUTPUT_EDP:
case INTEL_OUTPUT_HDMI:
diff --git a/drivers/gpu/drm/i915/intel_engine_cs.c
b/drivers/gpu/drm/i915/intel_engine_cs.c
index ab5bf4e..d02f37d 100644
--- a/drivers/gpu/drm/i9
[PATCH] drm/vmwgfx_kms: Fix potential NULL pointer dereference
crtc_state is being null checked in a previous code block, which implies
that such pointer might be null.
crtc_state is dereferenced in drm_atomic_helper_check_plane_state, hence
there is a potential null pointer dereference.
Fix this by warning-on and returning -EINVAL in case crtc_state is null.
Addresses-Coverity-ID: 1462412 ("Dereference after null check")
Fixes: a01cb8ba3f62 ("drm: Move drm_plane_helper_check_state() into
drm_atomic_helper.c")
Signed-off-by: Gustavo A. R. Silva <garsi...@embeddedor.com>
---
drivers/gpu/drm/vmwgfx/vmwgfx_kms.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c
b/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c
index a2a93d7..72c3b290 100644
--- a/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c
+++ b/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c
@@ -454,6 +454,9 @@ int vmw_du_primary_plane_atomic_check(struct drm_plane
*plane,
clip.y2 = crtc_state->adjusted_mode.vdisplay;
}
+ if (WARN_ON(!crtc_state))
+ return -EINVAL;
+
ret = drm_atomic_helper_check_plane_state(state, crtc_state, ,
DRM_PLANE_HELPER_NO_SCALING,
DRM_PLANE_HELPER_NO_SCALING,
--
2.7.4
Re: [PATCH] au0828: fix use-after-free at USB probing
Hey Andrey, Quoting Andrey Konovalov <andreyk...@google.com>: On Thu, Nov 23, 2017 at 2:31 AM, Gustavo A. R. Silva <garsi...@embeddedor.com> wrote: Hi Andrey, I have successfully installed and tested syzkaller with QEMU. Can you please tell me how to reproduce this bug or share with me the full crash report? Also, can you point me out to the PoC file? Hi Gustavo, Sorry for the delay. No worries. I've now published the USB fuzzing prototype, so here's how you can reproduce this: 1. Get Linux 4.15-rc3 upstream kernel (50c4c4e268a2d7a3e58ebb698ac74da0de40ae36). 2. Apply this patch (it adds a new interface to emulate USB devices): https://github.com/google/syzkaller/blob/usb-fuzzer/tools/usb/0002-usb-fuzzer-main-usb-gadget-fuzzer-driver.patch 3. Build the kernel with the attached .config (you need relatively new GCC to make KASAN work). 4. Run the attached reproducer. Also attaching the full kernel log. Awesome. :D I'll try this. Thank you! -- Gustavo A. R. Silva
Re: [PATCH] drm/i915/gvt/fb_decoder: Fix out-of-bounds read
Hi Zhenyu,
Quoting Zhenyu Wang <zhen...@linux.intel.com>:
On 2017.12.09 00:37:59 -0600, Gustavo A. R. Silva wrote:
In case function skl_format_to_drm returns -EINVAL, fmt turns into a huge
number as fmt is of type u32, hence there is an out-of-bounds read when
using fmt as an index for array skl_pixel_formats at line 225:
plane->bpp = skl_pixel_formats[fmt].bpp;
Fix this by comparing the value returned by function skl_format_to_drm
against the size of array skl_pixel_formats, so in case it is greater than
or equal to the number of items contained in skl_pixel_formats, print an
error message and return -EINVAL.
Addresses-Coverity-ID: 1462495
Addresses-Coverity-ID: 1462502 ("Out-of-bounds read")
Fixes: 9f31d1063b43 ("drm/i915/gvt: Add framebuffer decoder support")
Signed-off-by: Gustavo A. R. Silva <garsi...@embeddedor.com>
---
drivers/gpu/drm/i915/gvt/fb_decoder.c | 6 ++
1 file changed, 6 insertions(+)
diff --git a/drivers/gpu/drm/i915/gvt/fb_decoder.c
b/drivers/gpu/drm/i915/gvt/fb_decoder.c
index 72f4217..aed578b 100644
--- a/drivers/gpu/drm/i915/gvt/fb_decoder.c
+++ b/drivers/gpu/drm/i915/gvt/fb_decoder.c
@@ -222,6 +222,12 @@ int intel_vgpu_decode_primary_plane(struct
intel_vgpu *vgpu,
val & PLANE_CTL_ORDER_RGBX,
val & PLANE_CTL_ALPHA_MASK,
val & PLANE_CTL_YUV422_ORDER_MASK);
+
+ if (fmt >= ARRAY_SIZE(skl_pixel_formats)) {
+ gvt_vgpu_err("Out-of-bounds pixel format index\n");
+ return -EINVAL;
+ }
+
plane->bpp = skl_pixel_formats[fmt].bpp;
plane->drm_format = skl_pixel_formats[fmt].drm_format;
} else {
--
Applied this, thanks!
Glad to help. :)
Thanks
--
Gustavo A. R. Silva
[PATCH] ACPI / CPPC: Fix negative array index read in cppc_set_perf
If pcc_ss_id is less than 0, there is a negative array index read
before verifying pcc_ss_id is not a negative value.
Fix this by removing the code that triggers this issue.
Notice that this code is already properly placed after the check
on pcc_ss_id at line 1182: pcc_ss_data = pcc_data[pcc_ss_id];
Addresses-Coverity-ID: 1426090 ("Negative array index read")
Fixes: 1ecbd7170d65 ("ACPI / CPPC: Fix KASAN global out of bounds warning")
Signed-off-by: Gustavo A. R. Silva <garsi...@embeddedor.com>
---
drivers/acpi/cppc_acpi.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/acpi/cppc_acpi.c b/drivers/acpi/cppc_acpi.c
index 30e84cc..06ea474 100644
--- a/drivers/acpi/cppc_acpi.c
+++ b/drivers/acpi/cppc_acpi.c
@@ -1171,7 +1171,7 @@ int cppc_set_perf(int cpu, struct cppc_perf_ctrls
*perf_ctrls)
struct cpc_desc *cpc_desc = per_cpu(cpc_desc_ptr, cpu);
struct cpc_register_resource *desired_reg;
int pcc_ss_id = per_cpu(cpu_pcc_subspace_idx, cpu);
- struct cppc_pcc_data *pcc_ss_data = pcc_data[pcc_ss_id];
+ struct cppc_pcc_data *pcc_ss_data;
int ret = 0;
if (!cpc_desc || pcc_ss_id < 0) {
--
2.7.4
[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
Logically dead code at fs/afs/cell.c:206
Hi David,
Today Coverity reported a "Logically dead code" issue at fs/afs/cell.c:206:
if (!excl) {
rcu_read_lock();
cell = afs_lookup_cell_rcu(net, name, namesz);
rcu_read_unlock();
if (!IS_ERR(cell)) {
if (excl) {
afs_put_cell(net, cell);
return ERR_PTR(-EEXIST);
}
goto wait_for_cell;
}
}
The problem is that when this code block is executed, the code block starting
at line 211 makes no sense, as _excl_ can never be true.
I was wondering if the original intention was to null check _cell_ instead of
checking _excl_. So I took a look into function afs_lookup_cell_rcu to see if
_cell_ can be returned as a null pointer and at the same time the if condition
at line 210 be true, but I couldn't see how that could be possible. It seems to
me that when _ret_ is equal to zero, _cell_ cannot be null in
afs_lookup_cell_rcu. But is case I'm wrong here and _cell_ could be null at
line 210, then I think line 211 should be changed as follows:
diff --git a/fs/afs/cell.c b/fs/afs/cell.c
index 1858c91..a69a11f 100644
--- a/fs/afs/cell.c
+++ b/fs/afs/cell.c
@@ -208,7 +208,7 @@ struct afs_cell *afs_lookup_cell(struct afs_net *net,
cell = afs_lookup_cell_rcu(net, name, namesz);
rcu_read_unlock();
if (!IS_ERR(cell)) {
- if (excl) {
+ if (cell) {
afs_put_cell(net, cell);
return ERR_PTR(-EEXIST);
}
But I'm suspicious about it.
What do you think?
Thanks
--
Gustavo A. R. Silva
Re: [PATCH] au0828: fix use-after-free at USB probing
Quoting Andrey Konovalov <andreyk...@google.com>:
On Fri, Nov 10, 2017 at 1:21 AM, Gustavo A. R. Silva
<garsi...@embeddedor.com> wrote:
Hi Andrey,
Could you please try this patch?
Thank you
Hi Gustavo,
With your patch I get a different crash. Not sure if it's another bug
or the same one manifesting differently.
That's the same one. It seems that the best solution is to remove the
kfree after the mutex_unlock and let the device resources be freed in
au0828_usb_disconnect.
Please try the following patch instead.
I appreciate your help.
Thank you, Andrey.
---
drivers/media/usb/au0828/au0828-core.c | 1 -
1 file changed, 1 deletion(-)
diff --git a/drivers/media/usb/au0828/au0828-core.c
b/drivers/media/usb/au0828/au0828-core.c
index cd363a2..257ae0d 100644
--- a/drivers/media/usb/au0828/au0828-core.c
+++ b/drivers/media/usb/au0828/au0828-core.c
@@ -629,7 +629,6 @@ static int au0828_usb_probe(struct usb_interface
*interface,
pr_err("%s() au0282_dev_register failed to register
on V4L2\n",
__func__);
mutex_unlock(>lock);
- kfree(dev);
goto done;
}
--
2.7.4
au0828: recv_control_msg() Failed receiving control message, error -71.
au0828: recv_control_msg() Failed receiving control message, error -71.
au8522_writereg: writereg error (reg == 0x106, val == 0x0001, ret == -5)
usb 1-1: selecting invalid altsetting 5
au0828: Failure setting usb interface0 to as5
au0828: au0828_usb_probe() au0282_dev_register failed to register on V4L2
au0828: probe of 1-1:0.0 failed with error -22
usb 1-1: USB disconnect, device number 2
==
BUG: KASAN: use-after-free in __list_del_entry_valid+0xda/0xf3
Read of size 8 at addr 8800641d0410 by task kworker/0:1/24
CPU: 0 PID: 24 Comm: kworker/0:1 Not tainted
4.14.0-rc5-43687-g72e555fa3d2e-dirty #105
Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS Bochs 01/01/2011
Workqueue: usb_hub_wq hub_event
Call Trace:
__dump_stack lib/dump_stack.c:16
dump_stack+0xc1/0x11f lib/dump_stack.c:52
print_address_description+0x71/0x234 mm/kasan/report.c:252
kasan_report_error mm/kasan/report.c:351
kasan_report+0x173/0x270 mm/kasan/report.c:409
__asan_report_load8_noabort+0x19/0x20 mm/kasan/report.c:430
__list_del_entry_valid+0xda/0xf3 lib/list_debug.c:54
__list_del_entry ./include/linux/list.h:116
list_del_init ./include/linux/list.h:158
device_pm_remove+0x4a/0x1da drivers/base/power/main.c:149
device_del+0x55f/0xa30 drivers/base/core.c:1986
usb_disable_device+0x1df/0x670 drivers/usb/core/message.c:1170
usb_disconnect+0x260/0x7a0 drivers/usb/core/hub.c:2124
hub_port_connect drivers/usb/core/hub.c:4754
hub_port_connect_change drivers/usb/core/hub.c:5009
port_event drivers/usb/core/hub.c:5115
hub_event+0xe09/0x2eb0 drivers/usb/core/hub.c:5195
process_one_work+0x86d/0x13e0 kernel/workqueue.c:2119
process_scheduled_works kernel/workqueue.c:2179
worker_thread+0x689/0xea0 kernel/workqueue.c:2255
kthread+0x334/0x400 kernel/kthread.c:231
ret_from_fork+0x2a/0x40 arch/x86/entry/entry_64.S:431
The buggy address belongs to the page:
page:ea0001907400 count:0 mapcount:-127 mapping: (null)
index:0x0
flags: 0x100()
raw: 0100 ff80
raw: ea00018a8f20 88007fffa690 0002
page dumped because: kasan: bad access detected
Memory state around the buggy address:
8800641d0300: ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff
8800641d0380: ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff
8800641d0400: ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff
^
8800641d0480: ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff
8800641d0500: ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff
==
Thanks!
The device is typically freed on failure after trying to set
USB interface0 to as5 in function au0828_analog_register.
Fix use-after-free by returning the error value inmediately
after failure, instead of jumping to au0828_usb_disconnect
where _dev_ is also freed.
Signed-off-by: Gustavo A. R. Silva <garsi...@embeddedor.com>
---
drivers/media/usb/au0828/au0828-core.c | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/drivers/media/usb/au0828/au0828-core.c
b/drivers/media/usb/au0828/au0828-core.c
index cd363a2..b4abd90 100644
--- a/drivers/media/usb/au0828/au0828-core.c
+++ b/drivers/media/usb/au0828/au0828-core.c
@@ -630,7 +630,7 @@ static int au0828_usb_probe(struct
usb_interface *interface,
__func__);
mutex_unlock(>lock);
kfree(dev);
- goto done;
+ return retval;
}
/* Digital TV */
@@ -655,7 +655,6 @@ static int au0828_usb_probe
[PATCH] drm/amd/display/dc/dce110/dce110_mem_input_v: use swap macro in program_size_and_rotation
Make use of the swap macro instead of _manually_ swapping values
and remove unnecessary variable swap.
This makes the code easier to read and maintain.
This code was detected with the help of Coccinelle.
Signed-off-by: Gustavo A. R. Silva <garsi...@embeddedor.com>
---
.../drm/amd/display/dc/dce110/dce110_mem_input_v.c | 28 +++---
1 file changed, 8 insertions(+), 20 deletions(-)
diff --git a/drivers/gpu/drm/amd/display/dc/dce110/dce110_mem_input_v.c
b/drivers/gpu/drm/amd/display/dc/dce110/dce110_mem_input_v.c
index a06c602..7bab8c6 100644
--- a/drivers/gpu/drm/amd/display/dc/dce110/dce110_mem_input_v.c
+++ b/drivers/gpu/drm/amd/display/dc/dce110/dce110_mem_input_v.c
@@ -237,26 +237,14 @@ static void program_size_and_rotation(
if (rotation == ROTATION_ANGLE_90 ||
rotation == ROTATION_ANGLE_270) {
- uint32_t swap;
- swap = local_size.video.luma_size.x;
- local_size.video.luma_size.x =
- local_size.video.luma_size.y;
- local_size.video.luma_size.y = swap;
-
- swap = local_size.video.luma_size.width;
- local_size.video.luma_size.width =
- local_size.video.luma_size.height;
- local_size.video.luma_size.height = swap;
-
- swap = local_size.video.chroma_size.x;
- local_size.video.chroma_size.x =
- local_size.video.chroma_size.y;
- local_size.video.chroma_size.y = swap;
-
- swap = local_size.video.chroma_size.width;
- local_size.video.chroma_size.width =
- local_size.video.chroma_size.height;
- local_size.video.chroma_size.height = swap;
+ swap(local_size.video.luma_size.x,
+local_size.video.luma_size.y);
+ swap(local_size.video.luma_size.width,
+local_size.video.luma_size.height);
+ swap(local_size.video.chroma_size.x,
+local_size.video.chroma_size.y);
+ swap(local_size.video.chroma_size.width,
+local_size.video.chroma_size.height);
}
value = 0;
--
2.7.4
[PATCH] drm/amd/display/dc/core/dc_resource: use swap macro in rect_swap_helper
Make use of the swap macro instead of _manually_ swapping values
and remove unnecessary variable temp.
This makes the code easier to read and maintain.
This code was detected with the help of Coccinelle.
Signed-off-by: Gustavo A. R. Silva <garsi...@embeddedor.com>
---
drivers/gpu/drm/amd/display/dc/core/dc_resource.c | 11 ++-
1 file changed, 2 insertions(+), 9 deletions(-)
diff --git a/drivers/gpu/drm/amd/display/dc/core/dc_resource.c
b/drivers/gpu/drm/amd/display/dc/core/dc_resource.c
index d1cdf9f..ee216f2 100644
--- a/drivers/gpu/drm/amd/display/dc/core/dc_resource.c
+++ b/drivers/gpu/drm/amd/display/dc/core/dc_resource.c
@@ -426,15 +426,8 @@ static enum pixel_format
convert_pixel_format_to_dalsurface(
static void rect_swap_helper(struct rect *rect)
{
- uint32_t temp = 0;
-
- temp = rect->height;
- rect->height = rect->width;
- rect->width = temp;
-
- temp = rect->x;
- rect->x = rect->y;
- rect->y = temp;
+ swap(rect->height, rect->width);
+ swap(rect->x, rect->y);
}
static void calculate_viewport(struct pipe_ctx *pipe_ctx)
--
2.7.4
[PATCH] tty: vt: replace _manual_ swap with swap macro in set_selection
Make use of the swap macro instead of _manually_ swapping values
and remove unnecessary variable tmp.
This makes the code easier to read and maintain.
This code was detected with the help of Coccinelle.
Signed-off-by: Gustavo A. R. Silva <garsi...@embeddedor.com>
---
drivers/tty/vt/selection.c | 6 +-
1 file changed, 1 insertion(+), 5 deletions(-)
diff --git a/drivers/tty/vt/selection.c b/drivers/tty/vt/selection.c
index af4da95..7851383 100644
--- a/drivers/tty/vt/selection.c
+++ b/drivers/tty/vt/selection.c
@@ -186,11 +186,7 @@ int set_selection(const struct tiocl_selection __user
*sel, struct tty_struct *t
}
if (ps > pe)/* make sel_start <= sel_end */
- {
- int tmp = ps;
- ps = pe;
- pe = tmp;
- }
+ swap(ps, pe);
if (sel_cons != vc_cons[fg_console].d) {
clear_selection();
--
2.7.4
[PATCH] staging: speakup: selection: replace _manual_ swap with swap macro
Make use of the swap macro instead of _manually_ swapping values
and remove unnecessary variable tmp.
This makes the code easier to read and maintain.
This code was detected with the help of Coccinelle.
Signed-off-by: Gustavo A. R. Silva <garsi...@embeddedor.com>
---
drivers/staging/speakup/selection.c | 9 ++---
1 file changed, 2 insertions(+), 7 deletions(-)
diff --git a/drivers/staging/speakup/selection.c
b/drivers/staging/speakup/selection.c
index 66061b5..0ed1fef 100644
--- a/drivers/staging/speakup/selection.c
+++ b/drivers/staging/speakup/selection.c
@@ -64,13 +64,8 @@ int speakup_set_selection(struct tty_struct *tty)
ps = spk_ys * vc->vc_size_row + (spk_xs << 1);
pe = spk_ye * vc->vc_size_row + (spk_xe << 1);
- if (ps > pe) {
- /* make sel_start <= sel_end */
- int tmp = ps;
-
- ps = pe;
- pe = tmp;
- }
+ if (ps > pe)/* make sel_start <= sel_end */
+ swap(ps, pe);
if (spk_sel_cons != vc_cons[fg_console].d) {
speakup_clear_selection();
--
2.7.4
[PATCH] scsi: aic7xxx_core: remove unnecessary null check before kfree
NULL check before freeing functions like kfree is not needed. This code was detected with the help of Coccinelle. Signed-off-by: Gustavo A. R. Silva <garsi...@embeddedor.com> --- drivers/scsi/aic7xxx/aic7xxx_core.c | 6 ++ 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/drivers/scsi/aic7xxx/aic7xxx_core.c b/drivers/scsi/aic7xxx/aic7xxx_core.c index 6612ff3..8d96e11 100644 --- a/drivers/scsi/aic7xxx/aic7xxx_core.c +++ b/drivers/scsi/aic7xxx/aic7xxx_core.c @@ -4549,10 +4549,8 @@ ahc_free(struct ahc_softc *ahc) kfree(ahc->black_hole); } #endif - if (ahc->name != NULL) - kfree(ahc->name); - if (ahc->seep_config != NULL) - kfree(ahc->seep_config); + kfree(ahc->name); + kfree(ahc->seep_config); #ifndef __FreeBSD__ kfree(ahc); #endif -- 2.7.4
[PATCH] usb: uvc_debugfs: remove unnecessary NULL check before debugfs_remove_recursive
NULL check before freeing functions like debugfs_remove_recursive
is not needed.
This issue was detected with the help of Coccinelle.
Signed-off-by: Gustavo A. R. Silva <garsi...@embeddedor.com>
---
drivers/media/usb/uvc/uvc_debugfs.c | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/drivers/media/usb/uvc/uvc_debugfs.c
b/drivers/media/usb/uvc/uvc_debugfs.c
index 368f8f8..6995aeb 100644
--- a/drivers/media/usb/uvc/uvc_debugfs.c
+++ b/drivers/media/usb/uvc/uvc_debugfs.c
@@ -128,6 +128,5 @@ void uvc_debugfs_init(void)
void uvc_debugfs_cleanup(void)
{
- if (uvc_debugfs_root_dir != NULL)
- debugfs_remove_recursive(uvc_debugfs_root_dir);
+ debugfs_remove_recursive(uvc_debugfs_root_dir);
}
--
2.7.4
[PATCH] PM / devfreq: Remove unnecessary NULL check before kfree
NULL check before freeing functions like kfree is not needed. This issue was detected with the help of Coccinelle. Signed-off-by: Gustavo A. R. Silva <garsi...@embeddedor.com> --- drivers/devfreq/devfreq.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/drivers/devfreq/devfreq.c b/drivers/devfreq/devfreq.c index 78fb496..21164a9 100644 --- a/drivers/devfreq/devfreq.c +++ b/drivers/devfreq/devfreq.c @@ -672,8 +672,7 @@ struct devfreq *devfreq_add_device(struct device *dev, device_unregister(>dev); err_dev: - if (devfreq) - kfree(devfreq); + kfree(devfreq); err_out: return ERR_PTR(err); } -- 2.7.4
[PATCH] nvme: host: core: fix NULL pointer dereference in nvme_pr_command
_head_ pointer is being dereferenced when it is NULL.
Fix this by moving the code that dereferences such pointer after it has
been properly initialized in function nvme_get_ns_from_disk.
Addresses-Coverity-ID: 1461344
Fixes: 32acab3181c7 ("nvme: implement multipath access to nvme subsystems")
Signed-off-by: Gustavo A. R. Silva <garsi...@embeddedor.com>
---
drivers/nvme/host/core.c | 14 +++---
1 file changed, 7 insertions(+), 7 deletions(-)
diff --git a/drivers/nvme/host/core.c b/drivers/nvme/host/core.c
index 25da74d..9b9c324 100644
--- a/drivers/nvme/host/core.c
+++ b/drivers/nvme/host/core.c
@@ -1452,16 +1452,16 @@ static int nvme_pr_command(struct block_device *bdev,
u32 cdw10,
put_unaligned_le64(key, [0]);
put_unaligned_le64(sa_key, [8]);
- memset(, 0, sizeof(c));
- c.common.opcode = op;
- c.common.nsid = cpu_to_le32(head->ns_id);
- c.common.cdw10[0] = cpu_to_le32(cdw10);
-
ns = nvme_get_ns_from_disk(bdev->bd_disk, , _idx);
- if (unlikely(!ns))
+ if (unlikely(!ns)) {
ret = -EWOULDBLOCK;
- else
+ } else {
+ memset(, 0, sizeof(c));
+ c.common.opcode = op;
+ c.common.nsid = cpu_to_le32(head->ns_id);
+ c.common.cdw10[0] = cpu_to_le32(cdw10);
ret = nvme_submit_sync_cmd(ns->queue, , data, 16);
+ }
nvme_put_ns_from_disk(head, srcu_idx);
return ret;
}
--
2.7.4
[PATCH] watchdog: ib700wdt: mark expected switch fall-through
In preparation to enabling -Wimplicit-fallthrough, mark switch cases where we are expecting to fall through. Notice that in this particular case I replaced "Fall" with a proper "fall through" comment, which is what GCC is expecting to find. Signed-off-by: Gustavo A. R. Silva <garsi...@embeddedor.com> --- drivers/watchdog/ib700wdt.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/watchdog/ib700wdt.c b/drivers/watchdog/ib700wdt.c index f2e4e1e..cc26228 100644 --- a/drivers/watchdog/ib700wdt.c +++ b/drivers/watchdog/ib700wdt.c @@ -218,7 +218,7 @@ static long ibwdt_ioctl(struct file *file, unsigned int cmd, unsigned long arg) if (ibwdt_set_heartbeat(new_margin)) return -EINVAL; ibwdt_ping(); - /* Fall */ + /* fall through */ case WDIOC_GETTIMEOUT: return put_user(timeout, p); -- 2.7.4
[PATCH] watchdog: eurotechwdt: mark expected switch fall-through
In preparation to enabling -Wimplicit-fallthrough, mark switch cases where we are expecting to fall through. Notice that in this particular case I replaced "Fall" with a proper "fall through" comment, which is what GCC is expecting to find. Signed-off-by: Gustavo A. R. Silva <garsi...@embeddedor.com> --- drivers/watchdog/eurotechwdt.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/watchdog/eurotechwdt.c b/drivers/watchdog/eurotechwdt.c index 38e9671..47f77a6 100644 --- a/drivers/watchdog/eurotechwdt.c +++ b/drivers/watchdog/eurotechwdt.c @@ -290,7 +290,7 @@ static long eurwdt_ioctl(struct file *file, eurwdt_timeout = time; eurwdt_set_timeout(time); spin_unlock(_lock); - /* Fall */ + /* fall through */ case WDIOC_GETTIMEOUT: return put_user(eurwdt_timeout, p); -- 2.7.4
Re: [PATCH] scsi: bnx2i: bnx2i_hwi: use swap macro in bnx2i_send_iscsi_nopout
Quoting "Rangankar, Manish" <manish.rangan...@cavium.com>:
On 04/11/17 1:28 AM, "Gustavo A. R. Silva" <garsi...@embeddedor.com> wrote:
Make use of the swap macro and remove unnecessary variable tmp.
This makes the code easier to read and maintain.
This code was detected with the help of Coccinelle.
Signed-off-by: Gustavo A. R. Silva <garsi...@embeddedor.com>
---
drivers/scsi/bnx2i/bnx2i_hwi.c | 9 +++--
1 file changed, 3 insertions(+), 6 deletions(-)
diff --git a/drivers/scsi/bnx2i/bnx2i_hwi.c
b/drivers/scsi/bnx2i/bnx2i_hwi.c
index e0640e0..9e3bf53 100644
--- a/drivers/scsi/bnx2i/bnx2i_hwi.c
+++ b/drivers/scsi/bnx2i/bnx2i_hwi.c
@@ -547,12 +547,9 @@ int bnx2i_send_iscsi_nopout(struct bnx2i_conn
*bnx2i_conn,
nopout_wqe->op_attr = ISCSI_FLAG_CMD_FINAL;
memcpy(nopout_wqe->lun, _hdr->lun, 8);
- if (test_bit(BNX2I_NX2_DEV_57710, >hba->cnic_dev_type)) {
- u32 tmp = nopout_wqe->lun[0];
- /* 57710 requires LUN field to be swapped */
- nopout_wqe->lun[0] = nopout_wqe->lun[1];
- nopout_wqe->lun[1] = tmp;
- }
+ /* 57710 requires LUN field to be swapped */
+ if (test_bit(BNX2I_NX2_DEV_57710, >hba->cnic_dev_type))
+ swap(nopout_wqe->lun[0], nopout_wqe->lun[1]);
nopout_wqe->itt = ((u16)task->itt |
(ISCSI_TASK_TYPE_MPATH <<
--
2.7.4
Thanks,
Acked-by: Manish Rangankar <manish.rangan...@cavium.com>
Thank you, Manish.
--
Gustavo A. R. Silva
Re: [PATCH] drm/amd/display/dc/dce110/dce110_mem_input_v: use swap macro in program_size_and_rotation
Quoting Harry Wentland <harry.wentl...@amd.com>: On 2017-11-10 05:31 PM, Gustavo A. R. Silva wrote: Make use of the swap macro instead of _manually_ swapping values and remove unnecessary variable swap. This makes the code easier to read and maintain. This code was detected with the help of Coccinelle. Signed-off-by: Gustavo A. R. Silva <garsi...@embeddedor.com> Reviewed-by: Harry Wentland <harry.wentl...@amd.com> Thank you, Harry. -- Gustavo A. R. Silva
[PATCH] c8sectpfe: fix potential NULL pointer dereference in c8sectpfe_timer_interrupt
_channel_ is being dereferenced before it is null checked, hence there is a
potential null pointer dereference. Fix this by moving the pointer dereference
after _channel_ has been null checked.
This issue was detected with the help of Coccinelle.
Fixes: c5f5d0f99794 ("[media] c8sectpfe: STiH407/10 Linux DVB demux support")
Signed-off-by: Gustavo A. R. Silva <garsi...@embeddedor.com>
---
drivers/media/platform/sti/c8sectpfe/c8sectpfe-core.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/drivers/media/platform/sti/c8sectpfe/c8sectpfe-core.c
b/drivers/media/platform/sti/c8sectpfe/c8sectpfe-core.c
index 59280ac..23d0ced 100644
--- a/drivers/media/platform/sti/c8sectpfe/c8sectpfe-core.c
+++ b/drivers/media/platform/sti/c8sectpfe/c8sectpfe-core.c
@@ -83,7 +83,7 @@ static void c8sectpfe_timer_interrupt(unsigned long
ac8sectpfei)
static void channel_swdemux_tsklet(unsigned long data)
{
struct channel_info *channel = (struct channel_info *)data;
- struct c8sectpfei *fei = channel->fei;
+ struct c8sectpfei *fei;
unsigned long wp, rp;
int pos, num_packets, n, size;
u8 *buf;
@@ -91,6 +91,8 @@ static void channel_swdemux_tsklet(unsigned long data)
if (unlikely(!channel || !channel->irec))
return;
+ fei = channel->fei;
+
wp = readl(channel->irec + DMA_PRDS_BUSWP_TP(0));
rp = readl(channel->irec + DMA_PRDS_BUSRP_TP(0));
--
2.7.4
[PATCH] scsi: ufs: ufshcd: fix potential NULL pointer dereference in ufshcd_config_vreg
_vreg_ is being dereferenced before it is null checked, hence there is a
potential null pointer dereference.
Fix this by moving the pointer dereference after _vreg_ has been null
checked.
This issue was detected with the help of Coccinelle.
Fixes: aa4976130934 ("ufs: Add regulator enable support")
Signed-off-by: Gustavo A. R. Silva <garsi...@embeddedor.com>
---
drivers/scsi/ufs/ufshcd.c | 7 +--
1 file changed, 5 insertions(+), 2 deletions(-)
diff --git a/drivers/scsi/ufs/ufshcd.c b/drivers/scsi/ufs/ufshcd.c
index 011c336..a355d98 100644
--- a/drivers/scsi/ufs/ufshcd.c
+++ b/drivers/scsi/ufs/ufshcd.c
@@ -6559,12 +6559,15 @@ static int ufshcd_config_vreg(struct device *dev,
struct ufs_vreg *vreg, bool on)
{
int ret = 0;
- struct regulator *reg = vreg->reg;
- const char *name = vreg->name;
+ struct regulator *reg;
+ const char *name;
int min_uV, uA_load;
BUG_ON(!vreg);
+ reg = vreg->reg;
+ name = vreg->name;
+
if (regulator_count_voltages(reg) > 0) {
min_uV = on ? vreg->min_uV : 0;
ret = regulator_set_voltage(reg, min_uV, vreg->max_uV);
--
2.7.4
[PATCH] dmaengine: at_hdmac: fix potential NULL pointer dereference in atc_prep_dma_interleaved
_xt_ is being dereferenced before it is null checked, hence there is a
potential null pointer dereference.
Fix this by moving the pointer dereference after _xt_ has been null
checked.
This issue was detected with the help of Coccinelle.
Fixes: 4483320e241c ("dmaengine: Use Pointer xt after NULL check.")
Signed-off-by: Gustavo A. R. Silva <garsi...@embeddedor.com>
---
drivers/dma/at_hdmac.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/drivers/dma/at_hdmac.c b/drivers/dma/at_hdmac.c
index fbab271..a861b5b 100644
--- a/drivers/dma/at_hdmac.c
+++ b/drivers/dma/at_hdmac.c
@@ -708,7 +708,7 @@ atc_prep_dma_interleaved(struct dma_chan *chan,
unsigned long flags)
{
struct at_dma_chan *atchan = to_at_dma_chan(chan);
- struct data_chunk *first = xt->sgl;
+ struct data_chunk *first;
struct at_desc *desc = NULL;
size_t xfer_count;
unsigned intdwidth;
@@ -720,6 +720,8 @@ atc_prep_dma_interleaved(struct dma_chan *chan,
if (unlikely(!xt || xt->numf != 1 || !xt->frame_size))
return NULL;
+ first = xt->sgl;
+
dev_info(chan2dev(chan),
"%s: src=%pad, dest=%pad, numf=%d, frame_size=%d,
flags=0x%lx\n",
__func__, >src_start, >dst_start, xt->numf,
--
2.7.4
[PATCH] afs: cell: Remove unnecessary code in afs_lookup_cell
Due to recent changes this piece of code is no longer needed.
Addresses-Coverity-ID: 1462033
Link: https://lkml.kernel.org/r/4923.1510957...@warthog.procyon.org.uk
Signed-off-by: Gustavo A. R. Silva <garsi...@embeddedor.com>
---
fs/afs/cell.c | 7 +--
1 file changed, 1 insertion(+), 6 deletions(-)
diff --git a/fs/afs/cell.c b/fs/afs/cell.c
index 1858c91..9bb921d 100644
--- a/fs/afs/cell.c
+++ b/fs/afs/cell.c
@@ -207,13 +207,8 @@ struct afs_cell *afs_lookup_cell(struct afs_net *net,
rcu_read_lock();
cell = afs_lookup_cell_rcu(net, name, namesz);
rcu_read_unlock();
- if (!IS_ERR(cell)) {
- if (excl) {
- afs_put_cell(net, cell);
- return ERR_PTR(-EEXIST);
- }
+ if (!IS_ERR(cell))
goto wait_for_cell;
- }
}
/* Assume we're probably going to create a cell and preallocate and
--
2.7.4
Re: Logically dead code at fs/afs/cell.c:206
Quoting David Howells <dhowe...@redhat.com>:
Gustavo A. R. Silva <garsi...@embeddedor.com> wrote:
Today Coverity reported a "Logically dead code" issue at fs/afs/cell.c:206:
if (!excl) {
rcu_read_lock();
cell = afs_lookup_cell_rcu(net, name, namesz);
rcu_read_unlock();
if (!IS_ERR(cell)) {
if (excl) {
afs_put_cell(net, cell);
return ERR_PTR(-EEXIST);
}
goto wait_for_cell;
}
}
The problem is that when this code block is executed, the code block
starting at line 211 makes no sense, as _excl_ can never be true.
Good catch. The interior "if (excl) { ... }" statement and body needs
removing entirely. Originally the outer "if (!excl)" wrapping it wasn't
there.
I'll send a patch to remove that code.
Thanks for clarifying.
--
Gustavo A. R. Silva
Re: [PATCH] crypto: qat: qat_common: qat_uclo - mark expected switch fall-throughs
Quoting Herbert Xu <herb...@gondor.apana.org.au>: On Thu, Oct 12, 2017 at 05:55:29PM -0500, Gustavo A. R. Silva wrote: 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> Patch applied. Thanks. Thank you, Herbert. -- Gustavo A. R. Silva
Re: watchdog: f71808e_wdt: mark expected switch fall-throughs
Quoting Guenter Roeck <li...@roeck-us.net>: On Thu, Nov 02, 2017 at 02:28:17PM -0500, Gustavo A. R. Silva wrote: In preparation to enabling -Wimplicit-fallthrough, mark switch cases where we are expecting to fall through. Notice that in this particular case I replaced "Fall" with a proper "fall through" comment, which is what GCC is expecting to find. Signed-off-by: Gustavo A. R. Silva <garsi...@embeddedor.com> Reviewed-by: Guenter Roeck <li...@roeck-us.net> Thanks! -- Gustavo A. R. Silva --- drivers/watchdog/f71808e_wdt.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/watchdog/f71808e_wdt.c b/drivers/watchdog/f71808e_wdt.c index 8658dba..e0678c1 100644 --- a/drivers/watchdog/f71808e_wdt.c +++ b/drivers/watchdog/f71808e_wdt.c @@ -627,7 +627,7 @@ static long watchdog_ioctl(struct file *file, unsigned int cmd, if (new_options & WDIOS_ENABLECARD) return watchdog_start(); - + /* fall through */ case WDIOC_KEEPALIVE: watchdog_keepalive(); @@ -641,7 +641,7 @@ static long watchdog_ioctl(struct file *file, unsigned int cmd, return -EINVAL; watchdog_keepalive(); - /* Fall */ + /* fall through */ case WDIOC_GETTIMEOUT: return put_user(watchdog.timeout, uarg.i);
[PATCH] watchdog: pcwd_pci: mark expected switch fall-through
In preparation to enabling -Wimplicit-fallthrough, mark switch cases where we are expecting to fall through. Notice that in this particular case I replaced "Fall" with a proper "fall through" comment, which is what GCC is expecting to find. Signed-off-by: Gustavo A. R. Silva <garsi...@embeddedor.com> --- drivers/watchdog/pcwd_pci.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/watchdog/pcwd_pci.c b/drivers/watchdog/pcwd_pci.c index c0d07ee..c882252 100644 --- a/drivers/watchdog/pcwd_pci.c +++ b/drivers/watchdog/pcwd_pci.c @@ -545,7 +545,7 @@ static long pcipcwd_ioctl(struct file *file, unsigned int cmd, return -EINVAL; pcipcwd_keepalive(); - /* Fall */ + /* fall through */ } case WDIOC_GETTIMEOUT: -- 2.7.4
[PATCH] net: plip: mark expected switch fall-throughs
In preparation to enabling -Wimplicit-fallthrough, mark switch cases
where we are expecting to fall through.
Addresses-Coverity-ID: 114893
Addresses-Coverity-ID: 114894
Addresses-Coverity-ID: 114895
Addresses-Coverity-ID: 114896
Addresses-Coverity-ID: 114897
Addresses-Coverity-ID: 114898
Addresses-Coverity-ID: 114899
Addresses-Coverity-ID: 114900
Addresses-Coverity-ID: 114901
Addresses-Coverity-ID: 114902
Addresses-Coverity-ID: 114903
Addresses-Coverity-ID: 114904
Addresses-Coverity-ID: 114905
Signed-off-by: Gustavo A. R. Silva <garsi...@embeddedor.com>
---
drivers/net/plip/plip.c | 13 +
1 file changed, 13 insertions(+)
diff --git a/drivers/net/plip/plip.c b/drivers/net/plip/plip.c
index 3c55ea3..feb92ec 100644
--- a/drivers/net/plip/plip.c
+++ b/drivers/net/plip/plip.c
@@ -502,6 +502,7 @@ plip_receive(unsigned short nibble_timeout, struct
net_device *dev,
*data_p = (c0 >> 3) & 0x0f;
write_data (dev, 0x10); /* send ACK */
*ns_p = PLIP_NB_1;
+ /* fall through */
case PLIP_NB_1:
cx = nibble_timeout;
@@ -597,6 +598,7 @@ plip_receive_packet(struct net_device *dev, struct
net_local *nl,
printk(KERN_DEBUG "%s: receive start\n", dev->name);
rcv->state = PLIP_PK_LENGTH_LSB;
rcv->nibble = PLIP_NB_BEGIN;
+ /* fall through */
case PLIP_PK_LENGTH_LSB:
if (snd->state != PLIP_PK_DONE) {
@@ -617,6 +619,7 @@ plip_receive_packet(struct net_device *dev, struct
net_local *nl,
return TIMEOUT;
}
rcv->state = PLIP_PK_LENGTH_MSB;
+ /* fall through */
case PLIP_PK_LENGTH_MSB:
if (plip_receive(nibble_timeout, dev,
@@ -639,6 +642,7 @@ plip_receive_packet(struct net_device *dev, struct
net_local *nl,
rcv->state = PLIP_PK_DATA;
rcv->byte = 0;
rcv->checksum = 0;
+ /* fall through */
case PLIP_PK_DATA:
lbuf = rcv->skb->data;
@@ -651,6 +655,7 @@ plip_receive_packet(struct net_device *dev, struct
net_local *nl,
rcv->checksum += lbuf[--rcv->byte];
} while (rcv->byte);
rcv->state = PLIP_PK_CHECKSUM;
+ /* fall through */
case PLIP_PK_CHECKSUM:
if (plip_receive(nibble_timeout, dev,
@@ -663,6 +668,7 @@ plip_receive_packet(struct net_device *dev, struct
net_local *nl,
return ERROR;
}
rcv->state = PLIP_PK_DONE;
+ /* fall through */
case PLIP_PK_DONE:
/* Inform the upper layer for the arrival of a packet. */
@@ -708,6 +714,7 @@ plip_send(unsigned short nibble_timeout, struct net_device
*dev,
case PLIP_NB_BEGIN:
write_data (dev, data & 0x0f);
*ns_p = PLIP_NB_1;
+ /* fall through */
case PLIP_NB_1:
write_data (dev, 0x10 | (data & 0x0f));
@@ -722,6 +729,7 @@ plip_send(unsigned short nibble_timeout, struct net_device
*dev,
}
write_data (dev, 0x10 | (data >> 4));
*ns_p = PLIP_NB_2;
+ /* fall through */
case PLIP_NB_2:
write_data (dev, (data >> 4));
@@ -810,6 +818,7 @@ plip_send_packet(struct net_device *dev, struct net_local
*nl,
>nibble, snd->length.b.lsb))
return TIMEOUT;
snd->state = PLIP_PK_LENGTH_MSB;
+ /* fall through */
case PLIP_PK_LENGTH_MSB:
if (plip_send(nibble_timeout, dev,
@@ -818,6 +827,7 @@ plip_send_packet(struct net_device *dev, struct net_local
*nl,
snd->state = PLIP_PK_DATA;
snd->byte = 0;
snd->checksum = 0;
+ /* fall through */
case PLIP_PK_DATA:
do {
@@ -829,6 +839,7 @@ plip_send_packet(struct net_device *dev, struct net_local
*nl,
snd->checksum += lbuf[--snd->byte];
} while (snd->byte);
snd->state = PLIP_PK_CHECKSUM;
+ /* fall through */
case PLIP_PK_CHECKSUM:
if (plip_send(nibble_timeout, dev,
@@ -839,6 +850,7 @@ plip_send_packet(struct net_device *dev, struct net_local
*nl,
dev_kfree_skb(snd->skb);
dev->stats.tx_packets++;
snd->state = PLIP_PK_DONE;
+ /* fall through */
case PLIP_PK_DONE:
/* Close the connection */
@@ -927,6 +939,7 @@ plip_interrupt(void *dev_id)
switch (nl->connection) {
case PLIP_CN_CLOSING:
netif_wake_queue (dev);
[PATCH] mISDN: l1oip_core: replace _manual_ swap with swap macro
Make use of the swap macro and remove unnecessary variables skb and cnt.
This makes the code easier to read and maintain.
This code was detected with the help of Coccinelle.
Signed-off-by: Gustavo A. R. Silva <garsi...@embeddedor.com>
---
drivers/isdn/mISDN/l1oip_core.c | 10 ++
1 file changed, 2 insertions(+), 8 deletions(-)
diff --git a/drivers/isdn/mISDN/l1oip_core.c b/drivers/isdn/mISDN/l1oip_core.c
index b5d590e..e365478 100644
--- a/drivers/isdn/mISDN/l1oip_core.c
+++ b/drivers/isdn/mISDN/l1oip_core.c
@@ -440,14 +440,8 @@ l1oip_socket_recv(struct l1oip *hc, u8 remotecodec, u8
channel, u16 timebase,
#ifdef REORDER_DEBUG
if (hc->chan[channel].disorder_flag) {
- struct sk_buff *skb;
- int cnt;
- skb = hc->chan[channel].disorder_skb;
- hc->chan[channel].disorder_skb = nskb;
- nskb = skb;
- cnt = hc->chan[channel].disorder_cnt;
- hc->chan[channel].disorder_cnt = rx_counter;
- rx_counter = cnt;
+ swap(hc->chan[channel].disorder_skb, nskb);
+ swap(hc->chan[channel].disorder_cnt, rx_counter);
}
hc->chan[channel].disorder_flag ^= 1;
if (nskb)
--
2.7.4
[PATCH] crypto: chcr - Replace _manual_ swap with swap macro
Make use of the swap macro and remove unnecessary variable temp.
This makes the code easier to read and maintain.
This code was detected with the help of Coccinelle.
Signed-off-by: Gustavo A. R. Silva <garsi...@embeddedor.com>
---
drivers/crypto/chelsio/chcr_algo.c | 5 +
1 file changed, 1 insertion(+), 4 deletions(-)
diff --git a/drivers/crypto/chelsio/chcr_algo.c
b/drivers/crypto/chelsio/chcr_algo.c
index 936bdd8..4b508cb 100644
--- a/drivers/crypto/chelsio/chcr_algo.c
+++ b/drivers/crypto/chelsio/chcr_algo.c
@@ -1469,11 +1469,8 @@ static int chcr_ahash_update(struct ahash_request *req)
return -ENOMEM;
if (remainder) {
- u8 *temp;
/* Swap buffers */
- temp = req_ctx->reqbfr;
- req_ctx->reqbfr = req_ctx->skbfr;
- req_ctx->skbfr = temp;
+ swap(req_ctx->reqbfr, req_ctx->skbfr);
sg_pcopy_to_buffer(req->src, sg_nents(req->src),
req_ctx->reqbfr, remainder, req->nbytes -
remainder);
--
2.7.4
[PATCH v2] watchdog: pcwd_pci: mark expected switch fall-through
In preparation to enabling -Wimplicit-fallthrough, mark switch cases where we are expecting to fall through. Notice that in this particular case I replaced "Fall" with a proper "fall through" comment, which is what GCC is expecting to find. Signed-off-by: Gustavo A. R. Silva <garsi...@embeddedor.com> --- Changes in v2: Place the "fall through" comment outside the unconditional code block. drivers/watchdog/pcwd_pci.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/watchdog/pcwd_pci.c b/drivers/watchdog/pcwd_pci.c index c0d07ee..1f78f09 100644 --- a/drivers/watchdog/pcwd_pci.c +++ b/drivers/watchdog/pcwd_pci.c @@ -545,8 +545,8 @@ static long pcipcwd_ioctl(struct file *file, unsigned int cmd, return -EINVAL; pcipcwd_keepalive(); - /* Fall */ } + /* fall through */ case WDIOC_GETTIMEOUT: return put_user(heartbeat, p); -- 2.7.4
Re: [PATCH] watchdog: pcwd_pci: mark expected switch fall-through
Quoting Guenter Roeck <li...@roeck-us.net>: On Fri, Nov 03, 2017 at 04:04:23PM +0100, Wim Van Sebroeck wrote: Hi Gustavo, > In preparation to enabling -Wimplicit-fallthrough, mark switch cases > where we are expecting to fall through. > > Notice that in this particular case I replaced "Fall" with a proper > "fall through" comment, which is what GCC is expecting to find. > > Signed-off-by: Gustavo A. R. Silva <garsi...@embeddedor.com> > --- > drivers/watchdog/pcwd_pci.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/watchdog/pcwd_pci.c b/drivers/watchdog/pcwd_pci.c > index c0d07ee..c882252 100644 > --- a/drivers/watchdog/pcwd_pci.c > +++ b/drivers/watchdog/pcwd_pci.c > @@ -545,7 +545,7 @@ static long pcipcwd_ioctl(struct file *file, unsigned int cmd, >return -EINVAL; > >pcipcwd_keepalive(); > - /* Fall */ > + /* fall through */ >} > >case WDIOC_GETTIMEOUT: > -- > 2.7.4 > Shouldn't the /* fall through */ come after the } ? Good question. This is an unconditional code block needed to declare a local variable within the case statement. What is correct in that situation ? I think it is correct to place the comment outside the code block. I'll send a patch shortly. Thanks -- Gustavo A. R. Silva
[PATCH] scsi: lpfc_els: use swap macro in lpfc_plogi_confirm_nport
Make use of the swap macro and remove unnecessary variable keep_nlp_flag. This makes the code easier to read and maintain. This code was detected with the help of Coccinelle. Signed-off-by: Gustavo A. R. Silva <garsi...@embeddedor.com> --- drivers/scsi/lpfc/lpfc_els.c | 6 ++ 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/drivers/scsi/lpfc/lpfc_els.c b/drivers/scsi/lpfc/lpfc_els.c index b14f7c5..1b745fe 100644 --- a/drivers/scsi/lpfc/lpfc_els.c +++ b/drivers/scsi/lpfc/lpfc_els.c @@ -1525,7 +1525,7 @@ lpfc_plogi_confirm_nport(struct lpfc_hba *phba, uint32_t *prsp, struct fc_rport *rport; struct serv_parm *sp; uint8_t name[sizeof(struct lpfc_name)]; - uint32_t rc, keepDID = 0, keep_nlp_flag = 0; + uint32_t rc, keepDID = 0; uint16_t keep_nlp_state; struct lpfc_nvme_rport *keep_nrport = NULL; int put_node; @@ -1616,9 +1616,7 @@ lpfc_plogi_confirm_nport(struct lpfc_hba *phba, uint32_t *prsp, phba->cfg_rrq_xri_bitmap_sz); spin_lock_irq(shost->host_lock); - keep_nlp_flag = new_ndlp->nlp_flag; - new_ndlp->nlp_flag = ndlp->nlp_flag; - ndlp->nlp_flag = keep_nlp_flag; + swap(new_ndlp->nlp_flag, ndlp->nlp_flag); spin_unlock_irq(shost->host_lock); /* Set nlp_states accordingly */ -- 2.7.4
Re: [PATCH] watchdog: pcwd_pci: mark expected switch fall-through
Hi Wim, Quoting Wim Van Sebroeck <w...@iguana.be>: Hi Gustavo, In preparation to enabling -Wimplicit-fallthrough, mark switch cases where we are expecting to fall through. Notice that in this particular case I replaced "Fall" with a proper "fall through" comment, which is what GCC is expecting to find. Signed-off-by: Gustavo A. R. Silva <garsi...@embeddedor.com> --- drivers/watchdog/pcwd_pci.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/watchdog/pcwd_pci.c b/drivers/watchdog/pcwd_pci.c index c0d07ee..c882252 100644 --- a/drivers/watchdog/pcwd_pci.c +++ b/drivers/watchdog/pcwd_pci.c @@ -545,7 +545,7 @@ static long pcipcwd_ioctl(struct file *file, unsigned int cmd, return -EINVAL; pcipcwd_keepalive(); - /* Fall */ + /* fall through */ } case WDIOC_GETTIMEOUT: -- 2.7.4 Shouldn't the /* fall through */ come after the } ? Yep, you are right. I'll fix that. Thank you! -- Gustavo A. R. Silva Kind regards, Wim.
[PATCH] misc: mic: scif_nodeqp: use swap macro in scif_node_connect
Make use of the swap macro and remove unnecessary variable tmppayload. This makes the code easier to read and maintain. This code was detected with the help of Coccinelle. Signed-off-by: Gustavo A. R. Silva <garsi...@embeddedor.com> --- drivers/misc/mic/scif/scif_nodeqp.c | 5 + 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/drivers/misc/mic/scif/scif_nodeqp.c b/drivers/misc/mic/scif/scif_nodeqp.c index c66ca1a..e8ce4ba 100644 --- a/drivers/misc/mic/scif/scif_nodeqp.c +++ b/drivers/misc/mic/scif/scif_nodeqp.c @@ -467,7 +467,6 @@ static void scif_node_connect(struct scif_dev *scifdev, int dst) struct list_head *pos, *tmp; struct scifmsg msg; int err; - u64 tmppayload; if (dst < 1 || dst > scif_info.maxid) return; @@ -524,9 +523,7 @@ static void scif_node_connect(struct scif_dev *scifdev, int dst) msg.src.node = dev_i->node; msg.dst.node = dev_j->node; - tmppayload = msg.payload[0]; - msg.payload[0] = msg.payload[2]; - msg.payload[2] = tmppayload; + swap(msg.payload[0], msg.payload[2]); msg.payload[1] = p2p_ji->ppi_da[SCIF_PPI_MMIO]; msg.payload[3] = p2p_ji->ppi_len[SCIF_PPI_APER] << PAGE_SHIFT; -- 2.7.4
[PATCH] watchdog: wdt_pci: mark expected switch fall-through
In preparation to enabling -Wimplicit-fallthrough, mark switch cases where we are expecting to fall through. Notice that in this particular case I replaced "Fall" with a proper "fall through" comment, which is what GCC is expecting to find. Signed-off-by: Gustavo A. R. Silva <garsi...@embeddedor.com> --- drivers/watchdog/wdt_pci.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/watchdog/wdt_pci.c b/drivers/watchdog/wdt_pci.c index bc7addc..10e2cda0 100644 --- a/drivers/watchdog/wdt_pci.c +++ b/drivers/watchdog/wdt_pci.c @@ -430,7 +430,7 @@ static long wdtpci_ioctl(struct file *file, unsigned int cmd, if (wdtpci_set_heartbeat(new_heartbeat)) return -EINVAL; wdtpci_ping(); - /* Fall */ + /* fall through */ case WDIOC_GETTIMEOUT: return put_user(heartbeat, p); default: -- 2.7.4
[PATCH] watchdog: pcwd_usb: mark expected switch fall-through
In preparation to enabling -Wimplicit-fallthrough, mark switch cases where we are expecting to fall through. Notice that in this particular case I replaced "Fall" with a proper "fall through" comment, which is what GCC is expecting to find. Signed-off-by: Gustavo A. R. Silva <garsi...@embeddedor.com> --- drivers/watchdog/pcwd_usb.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/watchdog/pcwd_usb.c b/drivers/watchdog/pcwd_usb.c index b9e376c..47fe4c5 100644 --- a/drivers/watchdog/pcwd_usb.c +++ b/drivers/watchdog/pcwd_usb.c @@ -456,8 +456,8 @@ static long usb_pcwd_ioctl(struct file *file, unsigned int cmd, return -EINVAL; usb_pcwd_keepalive(usb_pcwd_device); - /* Fall */ } + /* fall through */ case WDIOC_GETTIMEOUT: return put_user(heartbeat, p); -- 2.7.4
[PATCH] watchdog: watchdog_dev: mark expected switch fall-through
In preparation to enabling -Wimplicit-fallthrough, mark switch cases
where we are expecting to fall through.
Notice that in this particular case I replaced "Fall" with a proper
"fall through" comment, which is what GCC is expecting to find.
Signed-off-by: Gustavo A. R. Silva <garsi...@embeddedor.com>
---
drivers/watchdog/watchdog_dev.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/watchdog/watchdog_dev.c b/drivers/watchdog/watchdog_dev.c
index 1e971a5..a79ad5b 100644
--- a/drivers/watchdog/watchdog_dev.c
+++ b/drivers/watchdog/watchdog_dev.c
@@ -720,7 +720,7 @@ static long watchdog_ioctl(struct file *file, unsigned int
cmd,
err = watchdog_ping(wdd);
if (err < 0)
break;
- /* Fall */
+ /* fall through */
case WDIOC_GETTIMEOUT:
/* timeout == 0 means that we don't know the timeout */
if (wdd->timeout == 0) {
--
2.7.4
[PATCH] KVM: VMX: replace move_msr_up with swap macro
Function move_msr_up is used to _manually_ swap MSR entries in MSR array.
This function can be removed and replaced using the swap macro instead.
This code was detected with the help of Coccinelle.
Signed-off-by: Gustavo A. R. Silva <garsi...@embeddedor.com>
---
The new lines are over 80 characters, but I think in this case that is
preferable over splitting them.
arch/x86/kvm/vmx.c | 24 ++--
1 file changed, 6 insertions(+), 18 deletions(-)
diff --git a/arch/x86/kvm/vmx.c b/arch/x86/kvm/vmx.c
index e6c8ffa..210e491 100644
--- a/arch/x86/kvm/vmx.c
+++ b/arch/x86/kvm/vmx.c
@@ -2544,18 +2544,6 @@ static bool vmx_invpcid_supported(void)
return cpu_has_vmx_invpcid() && enable_ept;
}
-/*
- * Swap MSR entry in host/guest MSR entry array.
- */
-static void move_msr_up(struct vcpu_vmx *vmx, int from, int to)
-{
- struct shared_msr_entry tmp;
-
- tmp = vmx->guest_msrs[to];
- vmx->guest_msrs[to] = vmx->guest_msrs[from];
- vmx->guest_msrs[from] = tmp;
-}
-
static void vmx_set_msr_bitmap(struct kvm_vcpu *vcpu)
{
unsigned long *msr_bitmap;
@@ -2600,28 +2588,28 @@ static void setup_msrs(struct vcpu_vmx *vmx)
if (is_long_mode(>vcpu)) {
index = __find_msr_index(vmx, MSR_SYSCALL_MASK);
if (index >= 0)
- move_msr_up(vmx, index, save_nmsrs++);
+ swap(vmx->guest_msrs[index],
vmx->guest_msrs[save_nmsrs++]);
index = __find_msr_index(vmx, MSR_LSTAR);
if (index >= 0)
- move_msr_up(vmx, index, save_nmsrs++);
+ swap(vmx->guest_msrs[index],
vmx->guest_msrs[save_nmsrs++]);
index = __find_msr_index(vmx, MSR_CSTAR);
if (index >= 0)
- move_msr_up(vmx, index, save_nmsrs++);
+ swap(vmx->guest_msrs[index],
vmx->guest_msrs[save_nmsrs++]);
index = __find_msr_index(vmx, MSR_TSC_AUX);
if (index >= 0 && guest_cpuid_has(>vcpu,
X86_FEATURE_RDTSCP))
- move_msr_up(vmx, index, save_nmsrs++);
+ swap(vmx->guest_msrs[index],
vmx->guest_msrs[save_nmsrs++]);
/*
* MSR_STAR is only needed on long mode guests, and only
* if efer.sce is enabled.
*/
index = __find_msr_index(vmx, MSR_STAR);
if ((index >= 0) && (vmx->vcpu.arch.efer & EFER_SCE))
- move_msr_up(vmx, index, save_nmsrs++);
+ swap(vmx->guest_msrs[index],
vmx->guest_msrs[save_nmsrs++]);
}
#endif
index = __find_msr_index(vmx, MSR_EFER);
if (index >= 0 && update_transition_efer(vmx, index))
- move_msr_up(vmx, index, save_nmsrs++);
+ swap(vmx->guest_msrs[index], vmx->guest_msrs[save_nmsrs++]);
vmx->save_nmsrs = save_nmsrs;
--
2.7.4
[PATCH] thunderbolt: tb: fix use after free in tb_activate_pcie_devices
Add a ̣̣continue statement in order to avoid using a previously
free'd pointer tunnel in list_add.
Addresses-Coverity-ID: 1415336
Fixes: 9d3cce0b6136 ("thunderbolt: Introduce thunderbolt bus and connection
manager")
Signed-off-by: Gustavo A. R. Silva <garsi...@embeddedor.com>
---
drivers/thunderbolt/tb.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/drivers/thunderbolt/tb.c b/drivers/thunderbolt/tb.c
index d674e06..1424581 100644
--- a/drivers/thunderbolt/tb.c
+++ b/drivers/thunderbolt/tb.c
@@ -225,6 +225,7 @@ static void tb_activate_pcie_devices(struct tb *tb)
tb_port_info(up_port,
"PCIe tunnel activation failed,
aborting\n");
tb_pci_free(tunnel);
+ continue;
}
list_add(>list, >tunnel_list);
--
2.7.4
Re: [PATCH] net/mlx5e/core/en_fs: fix pointer dereference after free in mlx5e_execute_l2_action
Hi Saeed,
Quoting Saeed Mahameed <sae...@dev.mellanox.co.il>:
On Sat, Nov 4, 2017 at 8:54 PM, Gustavo A. R. Silva
<garsi...@embeddedor.com> wrote:
hn is being kfree'd in mlx5e_del_l2_from_hash and then dereferenced
by accessing hn->ai.addr
Fix this by copying the MAC address into a local variable for its safe use
in all possible execution paths within function mlx5e_execute_l2_action.
Addresses-Coverity-ID: 1417789
Fixes: eeb66cdb6826 ("net/mlx5: Separate between E-Switch and MPFS")
Signed-off-by: Gustavo A. R. Silva <garsi...@embeddedor.com>
Acked-by: Saeed Mahameed <sae...@mellanox.com>
Looks good.
Thank you Gustavo.
Glad to help.
Thanks
--
Gustavo A. R. Silva
Re: watchdog: wdt_pci: mark expected switch fall-through
Quoting Guenter Roeck <li...@roeck-us.net>: On Fri, Nov 03, 2017 at 06:07:23PM -0500, Gustavo A. R. Silva wrote: In preparation to enabling -Wimplicit-fallthrough, mark switch cases where we are expecting to fall through. Notice that in this particular case I replaced "Fall" with a proper "fall through" comment, which is what GCC is expecting to find. Signed-off-by: Gustavo A. R. Silva <garsi...@embeddedor.com> Reviewed-by: Guenter Roeck <li...@roeck-us.net> Thanks! -- Gustavo A. R. Silva
[PATCH] net/mlx5e/core/en_fs: fix pointer dereference after free in mlx5e_execute_l2_action
hn is being kfree'd in mlx5e_del_l2_from_hash and then dereferenced
by accessing hn->ai.addr
Fix this by copying the MAC address into a local variable for its safe use
in all possible execution paths within function mlx5e_execute_l2_action.
Addresses-Coverity-ID: 1417789
Fixes: eeb66cdb6826 ("net/mlx5: Separate between E-Switch and MPFS")
Signed-off-by: Gustavo A. R. Silva <garsi...@embeddedor.com>
---
drivers/net/ethernet/mellanox/mlx5/core/en_fs.c | 13 -
1 file changed, 8 insertions(+), 5 deletions(-)
diff --git a/drivers/net/ethernet/mellanox/mlx5/core/en_fs.c
b/drivers/net/ethernet/mellanox/mlx5/core/en_fs.c
index 850cdc9..4837045 100644
--- a/drivers/net/ethernet/mellanox/mlx5/core/en_fs.c
+++ b/drivers/net/ethernet/mellanox/mlx5/core/en_fs.c
@@ -365,21 +365,24 @@ static void mlx5e_execute_l2_action(struct mlx5e_priv
*priv,
struct mlx5e_l2_hash_node *hn)
{
u8 action = hn->action;
+ u8 mac_addr[ETH_ALEN];
int l2_err = 0;
+ ether_addr_copy(mac_addr, hn->ai.addr);
+
switch (action) {
case MLX5E_ACTION_ADD:
mlx5e_add_l2_flow_rule(priv, >ai, MLX5E_FULLMATCH);
- if (!is_multicast_ether_addr(hn->ai.addr)) {
- l2_err = mlx5_mpfs_add_mac(priv->mdev, hn->ai.addr);
+ if (!is_multicast_ether_addr(mac_addr)) {
+ l2_err = mlx5_mpfs_add_mac(priv->mdev, mac_addr);
hn->mpfs = !l2_err;
}
hn->action = MLX5E_ACTION_NONE;
break;
case MLX5E_ACTION_DEL:
- if (!is_multicast_ether_addr(hn->ai.addr) && hn->mpfs)
- l2_err = mlx5_mpfs_del_mac(priv->mdev, hn->ai.addr);
+ if (!is_multicast_ether_addr(mac_addr) && hn->mpfs)
+ l2_err = mlx5_mpfs_del_mac(priv->mdev, mac_addr);
mlx5e_del_l2_flow_rule(priv, >ai);
mlx5e_del_l2_from_hash(hn);
break;
@@ -387,7 +390,7 @@ static void mlx5e_execute_l2_action(struct mlx5e_priv *priv,
if (l2_err)
netdev_warn(priv->netdev, "MPFS, failed to %s mac %pM,
err(%d)\n",
- action == MLX5E_ACTION_ADD ? "add" : "del",
hn->ai.addr, l2_err);
+ action == MLX5E_ACTION_ADD ? "add" : "del",
mac_addr, l2_err);
}
static void mlx5e_sync_netdev_addr(struct mlx5e_priv *priv)
--
2.7.4
[PATCH] xen: xenbus_probe_frontend: mark expected switch fall-throughs
In preparation to enabling -Wimplicit-fallthrough, mark switch cases where we are expecting to fall through. Addresses-Coverity-ID: 146562 Addresses-Coverity-ID: 146563 Signed-off-by: Gustavo A. R. Silva <garsi...@embeddedor.com> --- drivers/xen/xenbus/xenbus_probe_frontend.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/drivers/xen/xenbus/xenbus_probe_frontend.c b/drivers/xen/xenbus/xenbus_probe_frontend.c index 19e45ce..07896f4 100644 --- a/drivers/xen/xenbus/xenbus_probe_frontend.c +++ b/drivers/xen/xenbus/xenbus_probe_frontend.c @@ -379,10 +379,12 @@ static void xenbus_reset_frontend(char *fe, char *be, int be_state) case XenbusStateConnected: xenbus_printf(XBT_NIL, fe, "state", "%d", XenbusStateClosing); xenbus_reset_wait_for_backend(be, XenbusStateClosing); + /* fall through */ case XenbusStateClosing: xenbus_printf(XBT_NIL, fe, "state", "%d", XenbusStateClosed); xenbus_reset_wait_for_backend(be, XenbusStateClosed); + /* fall through */ case XenbusStateClosed: xenbus_printf(XBT_NIL, fe, "state", "%d", XenbusStateInitialising); -- 2.7.4
[PATCH] xen/pvcalls-front: mark expected switch fall-through
In preparation to enabling -Wimplicit-fallthrough, mark switch cases where we are expecting to fall through. Notice that in this particular case I placed the "fall through" comment on its own line, which is what GCC is expecting to find. Signed-off-by: Gustavo A. R. Silva <garsi...@embeddedor.com> --- drivers/xen/pvcalls-front.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/drivers/xen/pvcalls-front.c b/drivers/xen/pvcalls-front.c index 0c1ec68..ed94ea0 100644 --- a/drivers/xen/pvcalls-front.c +++ b/drivers/xen/pvcalls-front.c @@ -1250,7 +1250,8 @@ static void pvcalls_front_changed(struct xenbus_device *dev, case XenbusStateClosed: if (dev->state == XenbusStateClosed) break; - /* Missed the backend's CLOSING state -- fallthrough */ + /* Missed the backend's CLOSING state */ + /* fall through */ case XenbusStateClosing: xenbus_frontend_closed(dev); break; -- 2.7.4
[PATCH] watchdog: advantechwdt: mark expected switch fall-through
In preparation to enabling -Wimplicit-fallthrough, mark switch cases where we are expecting to fall through. Notice that in this particular case I replaced "Fall" with a proper "fall through" comment, which is what GCC is expecting to find. Signed-off-by: Gustavo A. R. Silva <garsi...@embeddedor.com> --- drivers/watchdog/advantechwdt.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/watchdog/advantechwdt.c b/drivers/watchdog/advantechwdt.c index 7d7db0c..f619443 100644 --- a/drivers/watchdog/advantechwdt.c +++ b/drivers/watchdog/advantechwdt.c @@ -181,7 +181,7 @@ static long advwdt_ioctl(struct file *file, unsigned int cmd, unsigned long arg) if (advwdt_set_heartbeat(new_timeout)) return -EINVAL; advwdt_ping(); - /* Fall */ + /* fall through */ case WDIOC_GETTIMEOUT: return put_user(timeout, p); default: -- 2.7.4
[PATCH] drm/nouveau/bios/timing: mark expected switch fall-throughs
In preparation to enabling -Wimplicit-fallthrough, mark switch cases
where we are expecting to fall through.
Addresses-Coverity-ID: 1260018
Addresses-Coverity-ID: 1260019
Addresses-Coverity-ID: 1260022
Signed-off-by: Gustavo A. R. Silva <garsi...@embeddedor.com>
---
drivers/gpu/drm/nouveau/nvkm/subdev/bios/timing.c | 5 +
1 file changed, 5 insertions(+)
diff --git a/drivers/gpu/drm/nouveau/nvkm/subdev/bios/timing.c
b/drivers/gpu/drm/nouveau/nvkm/subdev/bios/timing.c
index 7e83c39..20ff517 100644
--- a/drivers/gpu/drm/nouveau/nvkm/subdev/bios/timing.c
+++ b/drivers/gpu/drm/nouveau/nvkm/subdev/bios/timing.c
@@ -115,16 +115,21 @@ nvbios_timingEp(struct nvkm_bios *bios, int idx,
switch (min_t(u8, *hdr, 25)) {
case 25:
p->timing_10_24 = nvbios_rd08(bios, data + 0x18);
+ /* fall through */
case 24:
case 23:
case 22:
p->timing_10_21 = nvbios_rd08(bios, data + 0x15);
+ /* fall through */
case 21:
p->timing_10_20 = nvbios_rd08(bios, data + 0x14);
+ /* fall through */
case 20:
p->timing_10_CWL = nvbios_rd08(bios, data + 0x13);
+ /* fall through */
case 19:
p->timing_10_18 = nvbios_rd08(bios, data + 0x12);
+ /* fall through */
case 18:
case 17:
p->timing_10_16 = nvbios_rd08(bios, data + 0x10);
--
2.7.4
