netdev - Re: [PATCH] net: qrtr: mhi: make it work again

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Date:   Sun, 29 Aug 2021 19:19:01 +0530
From:   Manivannan Sadhasivam <manivannan.sadhasivam@...aro.org>
To:     Dmitry Baryshkov <dmitry.baryshkov@...aro.org>
Cc:     "David S. Miller" <davem@...emloft.net>,
        linux-arm-msm@...r.kernel.org, netdev@...r.kernel.org
Subject: Re: [PATCH] net: qrtr: mhi: make it work again

Hi Dmitry,

On Sun, Aug 29, 2021 at 03:45:28PM +0300, Dmitry Baryshkov wrote:
> The commit ce78ffa3ef16 ("net: really fix the build...") introduced two
> issues into the mhi.c driver:
>  - use of initialized completion
>  - calling mhi_prepare_for_transfer twice
> 

ce78ffa3ef16 got reverted in net-next:
https://git.kernel.org/pub/scm/linux/kernel/git/netdev/net-next.git/commit/net/qrtr?id=9ebc2758d0bbed951511d1709be0717178ec2660

And you also need the below commit:
https://git.kernel.org/pub/scm/linux/kernel/git/gregkh/char-misc.git/commit/?h=char-misc-next&id=0dc3ad3f859d3a65b335c861ec342d31d91e8bc8

Both commits are in linux-next so the issue should be fixed there and also in
the mainline soon.

Thanks,
Mani

> While the first one is pretty obvious, the second one makes all devices
> using mhi.c to return -EINVAL during probe. Fist
> mhi_prepare_for_transfer() would change both channels state to ENABLED.
> Then when second mhi_prepare_for_transfer() would be called it would
> also try switching them to ENABLED again, which is forbidden by the
> state machine in the mhi_update_channel_state() function (see
> drivers/bus/mhi/core/main.c).
> These two issues make all drivers using qcom_mhi_qrtr (e.g. ath11k) to
> fail with -EINVAL.
> 
> Fix them by removing first mhi_prepare_for_transfer() call and by adding
> the init_completion() call.
> 
> Fixes: ce78ffa3ef16 ("net: really fix the build...")
> Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@...aro.org>
> ---
>  net/qrtr/mhi.c | 7 ++-----
>  1 file changed, 2 insertions(+), 5 deletions(-)
> 
> diff --git a/net/qrtr/mhi.c b/net/qrtr/mhi.c
> index 1dc955ca57d3..f3f4a5fdeaf3 100644
> --- a/net/qrtr/mhi.c
> +++ b/net/qrtr/mhi.c
> @@ -83,15 +83,12 @@ 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(mhi_dev, 0);
> -	if (rc)
> -		return rc;
> -
>  	qdev = devm_kzalloc(&mhi_dev->dev, sizeof(*qdev), GFP_KERNEL);
>  	if (!qdev)
>  		return -ENOMEM;
>  
> +	init_completion(&qdev->ready);
> +
>  	qdev->mhi_dev = mhi_dev;
>  	qdev->dev = &mhi_dev->dev;
>  	qdev->ep.xmit = qcom_mhi_qrtr_send;
> -- 
> 2.33.0
>