On Thu, 11 Aug 2022 12:48:40 +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@xxxxxxxx> > Reviewed-by: Hemant Kumar <quic_hemantk@xxxxxxxxxxx> > Reviewed-by: Manivannan Sadhasivam <mani@xxxxxxxxxx> You must CC the author of the patch under Fixes, they are usually the best person to review the fix. Adding Loic now. > diff --git a/net/qrtr/mhi.c b/net/qrtr/mhi.c > index 18196e1c8c2f..9ced13c0627a 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,13 @@ 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) { > + qrtr_endpoint_unregister(&qdev->ep); > + return rc; > + } > + > dev_dbg(qdev->dev, "Qualcomm MHI QRTR driver probed\n"); > > return 0;