From: Manivannan Sadhasivam <mani@kernel.org>
To: Maxim Kochetkov <fido_max@inbox.ru>
Cc: netdev@vger.kernel.org, davem@davemloft.net, edumazet@google.com,
kuba@kernel.org, pabeni@redhat.com,
linux-arm-msm@vger.kernel.org,
Hemant Kumar <quic_hemantk@quicinc.com>
Subject: Re: [PATCH 1/1] net: qrtr: start MHI channel after endpoit creation
Date: Thu, 11 Aug 2022 15:07:21 +0530 [thread overview]
Message-ID: <20220811093721.GA29799@workstation> (raw)
In-Reply-To: <20220811092145.1648008-1-fido_max@inbox.ru>
On Thu, Aug 11, 2022 at 12:21:45PM +0300, Maxim Kochetkov wrote:
> MHI channel may generates event/interrupt right after enabling.
> It may leads to 2 race conditions issues.
>
> 1)
> Such event may be dropped by qcom_mhi_qrtr_dl_callback() at check:
>
> if (!qdev || mhi_res->transaction_status)
> return;
>
> Because dev_set_drvdata(&mhi_dev->dev, qdev) may be not performed at
> this moment. In this situation qrtr-ns will be unable to enumerate
> services in device.
> ---------------------------------------------------------------
>
> 2)
> Such event may come at the moment after dev_set_drvdata() and
> before qrtr_endpoint_register(). In this case kernel will panic with
> accessing wrong pointer at qcom_mhi_qrtr_dl_callback():
>
> rc = qrtr_endpoint_post(&qdev->ep, mhi_res->buf_addr,
> mhi_res->bytes_xferd);
>
> Because endpoint is not created yet.
> --------------------------------------------------------------
> So move mhi_prepare_for_transfer_autoqueue after endpoint creation
> to fix it.
>
> Fixes: a2e2cc0dbb11 ("net: qrtr: Start MHI channels during init")
> Signed-off-by: Maxim Kochetkov <fido_max@inbox.ru>
> Reviewed-by: Hemant Kumar <quic_hemantk@quicinc.com>
> Reviewed-by: Manivannan Sadhasivam <mani@kernel.org>
> ---
> net/qrtr/mhi.c | 10 +++++-----
> 1 file changed, 5 insertions(+), 5 deletions(-)
>
> diff --git a/net/qrtr/mhi.c b/net/qrtr/mhi.c
> index 18196e1c8c2f..17520d9e7a51 100644
> --- a/net/qrtr/mhi.c
> +++ b/net/qrtr/mhi.c
> @@ -78,11 +78,6 @@ static int qcom_mhi_qrtr_probe(struct mhi_device *mhi_dev,
> struct qrtr_mhi_dev *qdev;
> int rc;
>
> - /* start channels */
> - rc = mhi_prepare_for_transfer_autoqueue(mhi_dev);
> - if (rc)
> - return rc;
> -
> qdev = devm_kzalloc(&mhi_dev->dev, sizeof(*qdev), GFP_KERNEL);
> if (!qdev)
> return -ENOMEM;
> @@ -96,6 +91,11 @@ static int qcom_mhi_qrtr_probe(struct mhi_device *mhi_dev,
> if (rc)
> return rc;
>
> + /* start channels */
> + rc = mhi_prepare_for_transfer_autoqueue(mhi_dev);
> + if (rc)
> + return rc;
I missed the fact that you need to call qrtr_endpoint_unregister() in
the error path.
Please fix it.
Thanks,
Mani
> +
> dev_dbg(qdev->dev, "Qualcomm MHI QRTR driver probed\n");
>
> return 0;
> --
> 2.34.1
>
prev parent reply other threads:[~2022-08-11 9:37 UTC|newest]
Thread overview: 2+ messages / expand[flat|nested] mbox.gz Atom feed top
2022-08-11 9:21 [PATCH 1/1] net: qrtr: start MHI channel after endpoit creation Maxim Kochetkov
2022-08-11 9:37 ` Manivannan Sadhasivam [this message]
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20220811093721.GA29799@workstation \
--to=mani@kernel.org \
--cc=davem@davemloft.net \
--cc=edumazet@google.com \
--cc=fido_max@inbox.ru \
--cc=kuba@kernel.org \
--cc=linux-arm-msm@vger.kernel.org \
--cc=netdev@vger.kernel.org \
--cc=pabeni@redhat.com \
--cc=quic_hemantk@quicinc.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.