[alsa-devel] [PATCH 0/6] slimbus: sparse and CoverityScan fixes
From: Srinivas Kandagatla srinivas.kandagatla@linaro.org
Hi Greg,
Here are few patches which are submitted in the list few days back, these also fix some of the sparse errors reported.
Thanks, Srini
Colin Ian King (3): slimbus: avoid null pointer dereference on msg slimbus: fix retries comparison to correctly identify failed allocation slimbus: make functions slim_ack_txn and slim_alloc_txbuf static
Wei Yongjun (3): slimbus: Use GFP_ATOMIC under spin lock slimbus: Fix missing unlock on error in slim_msg_response() slimbus: qcom: Fix return value check in qcom_slim_probe()
drivers/slimbus/messaging.c | 8 +++++--- drivers/slimbus/qcom-ctrl.c | 13 +++++++------ 2 files changed, 12 insertions(+), 9 deletions(-)
From: Colin Ian King colin.king@canonical.com
The pointer msg is checked to see if it is null at the start of the function and jumps to the error exit label reterr that then dereferences msg when it prints a dev_err error message. Avoid this potential null pointer dereference by only printing the error message if msg is not null.
Detected by CoverityScan, CID#1463141 ("Dereference after null check")
Signed-off-by: Colin Ian King colin.king@canonical.com Signed-off-by: Srinivas Kandagatla srinivas.kandagatla@linaro.org --- drivers/slimbus/messaging.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-)
diff --git a/drivers/slimbus/messaging.c b/drivers/slimbus/messaging.c index 755462a4c75e..8b2c77f516b9 100644 --- a/drivers/slimbus/messaging.c +++ b/drivers/slimbus/messaging.c @@ -170,8 +170,9 @@ static int slim_val_inf_sanity(struct slim_controller *ctrl, break; } reterr: - dev_err(ctrl->dev, "Sanity check failed:msg:offset:0x%x, mc:%d\n", - msg->start_offset, mc); + if (msg) + dev_err(ctrl->dev, "Sanity check failed:msg:offset:0x%x, mc:%d\n", + msg->start_offset, mc); return -EINVAL; }
From: Colin Ian King colin.king@canonical.com
Currently the check for too many retries fails because retries is actually -1 when the retry loop terminates if no pbuf can be allocated because of the post decrement on retries. Fix this by not comparing retries with zero but instead check if it is negative.
Detected by CoverityScan, CID#1463143 ("Logically dead code") and CID#1463144 ("Dereference after null check")
Signed-off-by: Colin Ian King colin.king@canonical.com Signed-off-by: Srinivas Kandagatla srinivas.kandagatla@linaro.org --- drivers/slimbus/qcom-ctrl.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/slimbus/qcom-ctrl.c b/drivers/slimbus/qcom-ctrl.c index fb1a5e0eb8dd..2d67419a3c37 100644 --- a/drivers/slimbus/qcom-ctrl.c +++ b/drivers/slimbus/qcom-ctrl.c @@ -345,7 +345,7 @@ static int qcom_xfer_msg(struct slim_controller *sctrl, } }
- if (!retries && !pbuf) + if (retries < 0 && !pbuf) return -ENOMEM;
puc = (u8 *)pbuf;
From: Colin Ian King colin.king@canonical.com
The functions slim_ack_txn and slim_alloc_txbuf are local to the source and do not need to be in global scope, so make them static.
Cleans up sparse warnings: symbol 'slim_ack_txn' was not declared. Should it be static? symbol 'slim_alloc_txbuf' was not declared. Should it be static?
Signed-off-by: Colin Ian King colin.king@canonical.com Signed-off-by: Srinivas Kandagatla srinivas.kandagatla@linaro.org --- drivers/slimbus/qcom-ctrl.c | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-)
diff --git a/drivers/slimbus/qcom-ctrl.c b/drivers/slimbus/qcom-ctrl.c index 2d67419a3c37..8d12baae4119 100644 --- a/drivers/slimbus/qcom-ctrl.c +++ b/drivers/slimbus/qcom-ctrl.c @@ -146,7 +146,7 @@ static void *slim_alloc_rxbuf(struct qcom_slim_ctrl *ctrl) return ctrl->rx.base + (idx * ctrl->rx.sl_sz); }
-void slim_ack_txn(struct qcom_slim_ctrl *ctrl, int err) +static void slim_ack_txn(struct qcom_slim_ctrl *ctrl, int err) { struct completion *comp; unsigned long flags; @@ -299,8 +299,9 @@ static int qcom_clk_pause_wakeup(struct slim_controller *sctrl) return 0; }
-void *slim_alloc_txbuf(struct qcom_slim_ctrl *ctrl, struct slim_msg_txn *txn, - struct completion *done) +static void *slim_alloc_txbuf(struct qcom_slim_ctrl *ctrl, + struct slim_msg_txn *txn, + struct completion *done) { unsigned long flags; int idx;
From: Wei Yongjun weiyongjun1@huawei.com
A spin lock is taken here so we should use GFP_ATOMIC.
Signed-off-by: Wei Yongjun weiyongjun1@huawei.com Signed-off-by: Srinivas Kandagatla srinivas.kandagatla@linaro.org --- drivers/slimbus/messaging.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/slimbus/messaging.c b/drivers/slimbus/messaging.c index 8b2c77f516b9..a9a6dc4af0da 100644 --- a/drivers/slimbus/messaging.c +++ b/drivers/slimbus/messaging.c @@ -98,7 +98,7 @@ int slim_do_transfer(struct slim_controller *ctrl, struct slim_msg_txn *txn) if (need_tid) { spin_lock_irqsave(&ctrl->txn_lock, flags); tid = idr_alloc(&ctrl->tid_idr, txn, 0, - SLIM_MAX_TIDS, GFP_KERNEL); + SLIM_MAX_TIDS, GFP_ATOMIC); txn->tid = tid;
if (!txn->msg->comp)
From: Wei Yongjun weiyongjun1@huawei.com
Add the missing unlock before return from function slim_msg_response() in the error handling case.
Signed-off-by: Wei Yongjun weiyongjun1@huawei.com Signed-off-by: Srinivas Kandagatla srinivas.kandagatla@linaro.org --- drivers/slimbus/messaging.c | 1 + 1 file changed, 1 insertion(+)
diff --git a/drivers/slimbus/messaging.c b/drivers/slimbus/messaging.c index a9a6dc4af0da..884419c37e84 100644 --- a/drivers/slimbus/messaging.c +++ b/drivers/slimbus/messaging.c @@ -38,6 +38,7 @@ void slim_msg_response(struct slim_controller *ctrl, u8 *reply, u8 tid, u8 len) if (msg == NULL || msg->rbuf == NULL) { dev_err(ctrl->dev, "Got response to invalid TID:%d, len:%d\n", tid, len); + spin_unlock_irqrestore(&ctrl->txn_lock, flags); return; }
From: Wei Yongjun weiyongjun1@huawei.com
In case of error, the function devm_ioremap_resource() returns ERR_PTR() and never returns NULL. The NULL test in the return value check should be replaced with IS_ERR().
Signed-off-by: Wei Yongjun weiyongjun1@huawei.com Signed-off-by: Srinivas Kandagatla srinivas.kandagatla@linaro.org --- drivers/slimbus/qcom-ctrl.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/slimbus/qcom-ctrl.c b/drivers/slimbus/qcom-ctrl.c index 8d12baae4119..ffb46f915334 100644 --- a/drivers/slimbus/qcom-ctrl.c +++ b/drivers/slimbus/qcom-ctrl.c @@ -529,9 +529,9 @@ static int qcom_slim_probe(struct platform_device *pdev)
slim_mem = platform_get_resource_byname(pdev, IORESOURCE_MEM, "ctrl"); ctrl->base = devm_ioremap_resource(ctrl->dev, slim_mem); - if (!ctrl->base) { + if (IS_ERR(ctrl->base)) { dev_err(&pdev->dev, "IOremap failed\n"); - return -ENOMEM; + return PTR_ERR(ctrl->base); }
sctrl->set_laddr = qcom_set_laddr;
participants (1)
-
srinivas.kandagatla＠linaro.org