From: Yassine Oudjana <y.oudjana@protonmail.com>
To: dan.carpenter@oracle.com
Cc: bjorn.andersson@linaro.org, butterflyhuangxx@gmail.com,
davem@davemloft.net, kuba@kernel.org,
linux-arm-msm@vger.kernel.org, loic.poulain@linaro.org,
mani@kernel.org, netdev@vger.kernel.org
Subject: Re: [PATCH v2 net] net: qrtr: make checks in qrtr_endpoint_post() stricter
Date: Fri, 03 Sep 2021 19:29:28 +0000 [thread overview]
Message-ID: <S4IVYQ.R543O8OZ1IFR3@protonmail.com> (raw)
On Mon, 30 Aug 2021 11:37:17 +0300 Dan Carpenter wrote:
> These checks are still not strict enough. The main problem is that if
> "cb->type == QRTR_TYPE_NEW_SERVER" is true then "len - hdrlen" is
> guaranteed to be 4 but we need to be at least 16 bytes. In fact, we
> can reject everything smaller than sizeof(*pkt) which is 20 bytes.
>
> Also I don't like the ALIGN(size, 4). It's better to just insist that
> data is needs to be aligned at the start.
>
> Fixes: 0baa99ee353c ("net: qrtr: Allow non-immediate node routing")
> Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
> ---
> v2: Fix a % vs & bug. Thanks, butt3rflyh4ck!
>
> net/qrtr/qrtr.c | 8 ++++++--
> 1 file changed, 6 insertions(+), 2 deletions(-)
>
> diff --git a/net/qrtr/qrtr.c b/net/qrtr/qrtr.c
> index b8508e35d20e..dbb647f5481b 100644
> --- a/net/qrtr/qrtr.c
> +++ b/net/qrtr/qrtr.c
> @@ -493,7 +493,7 @@ int qrtr_endpoint_post(struct qrtr_endpoint *ep,
const void *data, size_t len)
> goto err;
> }
>
> - if (!size || len != ALIGN(size, 4) + hdrlen)
> + if (!size || size & 3 || len != size + hdrlen)
> goto err;
>
> if (cb->dst_port != QRTR_PORT_CTRL && cb->type != QRTR_TYPE_DATA &&
> @@ -506,8 +506,12 @@ int qrtr_endpoint_post(struct qrtr_endpoint
*ep, const void *data, size_t len)
>
> if (cb->type == QRTR_TYPE_NEW_SERVER) {
> /* Remote node endpoint can bridge other distant nodes */
> - const struct qrtr_ctrl_pkt *pkt = data + hdrlen;
> + const struct qrtr_ctrl_pkt *pkt;
>
> + if (size < sizeof(*pkt))
> + goto err;
> +
> + pkt = data + hdrlen;
> qrtr_node_assign(node, le32_to_cpu(pkt->server.node));
> }
>
> --
> 2.20.1
>
This is crashing MSM8996. I get these messages (dmesg | grep
remoteproc):
[ 11.677216] qcom-q6v5-mss 2080000.remoteproc: supply pll not found,
using dummy regulator
[ 11.701423] qcom_q6v5_pas 1c00000.remoteproc: supply cx not found,
using dummy regulator
[ 11.716475] qcom_q6v5_pas 1c00000.remoteproc: supply px not found,
using dummy regulator
[ 11.724481] remoteproc remoteproc0: 2080000.remoteproc is available
[ 11.747772] remoteproc remoteproc1: 1c00000.remoteproc is available
[ 11.762163] qcom_q6v5_pas 9300000.remoteproc: supply cx not found,
using dummy regulator
[ 11.778599] qcom_q6v5_pas 9300000.remoteproc: supply px not found,
using dummy regulator
[ 11.785288] remoteproc remoteproc2: 9300000.remoteproc is available
[ 11.786574] remoteproc remoteproc1: powering up 1c00000.remoteproc
[ 11.791908] remoteproc remoteproc1: Booting fw image
qcom/msm8996/scorpio/slpi.mbn, size 3921212
[ 11.870859] remoteproc remoteproc2: powering up 9300000.remoteproc
[ 11.873980] remoteproc remoteproc2: Booting fw image
qcom/msm8996/scorpio/adsp.mbn, size 12264177
[ 11.922394] remoteproc remoteproc1: remote processor
1c00000.remoteproc is now up
[ 12.036379] qcom_smd_qrtr remoteproc1:smd-edge.IPCRTR.-1.-1: invalid
ipcrouter packet
[ 12.039457] qcom_smd_qrtr remoteproc1:smd-edge.IPCRTR.-1.-1: invalid
ipcrouter packet
[ 12.112448] qcom_smd_qrtr remoteproc2:smd-edge.IPCRTR.-1.-1: invalid
ipcrouter packet
[ 13.015132] qcom,apr remoteproc2:smd-edge.apr_audio_svc.-1.-1: Adding
APR dev: aprsvc:q6core:4:3
[ 13.019159] qcom,apr remoteproc2:smd-edge.apr_audio_svc.-1.-1: Adding
APR dev: aprsvc:q6afe:4:4
[ 13.028870] qcom,apr remoteproc2:smd-edge.apr_audio_svc.-1.-1: Adding
APR dev: aprsvc:q6asm:4:7
[ 13.031606] qcom,apr remoteproc2:smd-edge.apr_audio_svc.-1.-1: Adding
APR dev: aprsvc:q6adm:4:8
[ 13.214501] q6asm-dai 9300000.remoteproc:smd-edge:apr:q6asm:dais:
Adding to iommu group 3
[ 13.994777] remoteproc remoteproc0: powering up 2080000.remoteproc
[ 13.999669] remoteproc remoteproc0: Booting fw image
qcom/msm8996/scorpio/mba.mbn, size 213888
[ 14.247034] qcom_smd_qrtr remoteproc1:smd-edge.IPCRTR.-1.-1: invalid
ipcrouter packet
[ 14.247298] qcom_smd_qrtr remoteproc1:smd-edge.IPCRTR.-1.-1: invalid
ipcrouter packet
[ 17.118806] qcom_q6v5_pas 1c00000.remoteproc: timeout waiting for
subsystem event response
[ 17.119496] qcom_smd_qrtr remoteproc2:smd-edge.IPCRTR.-1.-1: invalid
ipcrouter packet
[ 17.119556] qcom_smd_qrtr remoteproc2:smd-edge.IPCRTR.-1.-1: invalid
ipcrouter packet
[ 19.422732] qcom_q6v5_pas 1c00000.remoteproc: timeout waiting for
subsystem event response
[ 19.423388] qcom_smd_qrtr remoteproc2:smd-edge.IPCRTR.-1.-1: invalid
ipcrouter packet
[ 19.423453] qcom_smd_qrtr remoteproc2:smd-edge.IPCRTR.-1.-1: invalid
ipcrouter packet
[ 22.238725] qcom_q6v5_pas 9300000.remoteproc: timeout waiting for
subsystem event response
[ 24.542706] qcom_q6v5_pas 9300000.remoteproc: timeout waiting for
subsystem event response
[ 24.543468] qcom_smd_qrtr remoteproc2:smd-edge.IPCRTR.-1.-1: invalid
ipcrouter packet
[ 24.543524] qcom_smd_qrtr remoteproc2:smd-edge.IPCRTR.-1.-1: invalid
ipcrouter packet
[ 24.658698] qcom-q6v5-mss 2080000.remoteproc: MBA booted without debug
policy, loading mpss
[ 25.994603] qcom_smd_qrtr remoteproc0:smd-edge.IPCRTR.-1.-1: invalid
ipcrouter packet
[ 29.662816] qcom_q6v5_pas 9300000.remoteproc: timeout waiting for
subsystem event response
[ 29.662922] remoteproc remoteproc2: remote processor
9300000.remoteproc is now up
[ 29.665429] qcom_smd_qrtr remoteproc1:smd-edge.IPCRTR.-1.-1: invalid
ipcrouter packet
[ 29.665645] qcom_smd_qrtr remoteproc1:smd-edge.IPCRTR.-1.-1: invalid
ipcrouter packet
[ 34.782737] qcom_q6v5_pas 1c00000.remoteproc: timeout waiting for
subsystem event response
[ 34.783369] qcom_smd_qrtr remoteproc2:smd-edge.IPCRTR.-1.-1: invalid
ipcrouter packet
[ 34.783526] qcom_smd_qrtr remoteproc2:smd-edge.IPCRTR.-1.-1: invalid
ipcrouter packet
[ 39.902789] qcom_q6v5_pas 9300000.remoteproc: timeout waiting for
subsystem event response
[ 39.903057] qcom_smd_qrtr remoteproc0:smd-edge.IPCRTR.-1.-1: invalid
ipcrouter packet
[ 39.903131] qcom_smd_qrtr remoteproc0:smd-edge.IPCRTR.-1.-1: invalid
ipcrouter packet
[ 45.022691] qcom-q6v5-mss 2080000.remoteproc: timeout waiting for
subsystem event response
[ 45.022824] qcom_smd_qrtr remoteproc0:smd-edge.IPCRTR.-1.-1: invalid
ipcrouter packet
[ 45.022863] qcom_smd_qrtr remoteproc0:smd-edge.IPCRTR.-1.-1: invalid
ipcrouter packet
[ 50.146792] qcom-q6v5-mss 2080000.remoteproc: timeout waiting for
subsystem event response
[ 50.146888] remoteproc remoteproc0: remote processor
2080000.remoteproc is now up
[ 66.001288] qcom-q6v5-mss 2080000.remoteproc: fatal error without
message
[ 66.001311] remoteproc remoteproc0: crash detected in
2080000.remoteproc: type fatal error
[ 66.001328] remoteproc remoteproc0: handling crash #1 in
2080000.remoteproc
[ 66.001334] remoteproc remoteproc0: recovering 2080000.remoteproc
[ 66.003850] qcom_smd_qrtr remoteproc1:smd-edge.IPCRTR.-1.-1: invalid
ipcrouter packet
[ 66.004073] qcom_smd_qrtr remoteproc1:smd-edge.IPCRTR.-1.-1: invalid
ipcrouter packet
[ 71.134780] qcom_q6v5_pas 1c00000.remoteproc: timeout waiting for
subsystem event response
[ 71.135455] qcom_smd_qrtr remoteproc2:smd-edge.IPCRTR.-1.-1: invalid
ipcrouter packet
[ 71.135505] qcom_smd_qrtr remoteproc2:smd-edge.IPCRTR.-1.-1: invalid
ipcrouter packet
[ 76.258685] qcom_q6v5_pas 9300000.remoteproc: timeout waiting for
subsystem event response
[ 76.261799] qcom_smd_qrtr remoteproc1:smd-edge.IPCRTR.-1.-1: invalid
ipcrouter packet
[ 76.262029] qcom_smd_qrtr remoteproc1:smd-edge.IPCRTR.-1.-1: invalid
ipcrouter packet
[ 81.374728] qcom_q6v5_pas 1c00000.remoteproc: timeout waiting for
subsystem event response
[ 86.494754] qcom_q6v5_pas 9300000.remoteproc: timeout waiting for
subsystem event response
[ 86.494812] remoteproc remoteproc0: stopped remote processor
2080000.remoteproc
Then I get a last message which I wasn't able to capture above but was
able to see:
qcom-q6v5-mss 2080000.remoteproc: MBA booted without debug policy,
loading mpss
and it crashes and reboots right after this message.
next reply other threads:[~2021-09-03 19:29 UTC|newest]
Thread overview: 7+ messages / expand[flat|nested] mbox.gz Atom feed top
2021-09-03 19:29 Yassine Oudjana [this message]
2021-09-06 6:53 ` [PATCH v2 net] net: qrtr: make checks in qrtr_endpoint_post() stricter Dan Carpenter
2021-09-15 17:30 ` Steev Klimaszewski
2021-09-15 18:40 ` Yassine Oudjana
2021-09-15 21:58 ` Steev Klimaszewski
-- strict thread matches above, loose matches on Subject: below --
2021-08-27 15:35 [PATCH net-next] " butt3rflyh4ck
2021-08-30 8:37 ` [PATCH v2 net] " Dan Carpenter
2021-08-30 11:30 ` patchwork-bot+netdevbpf
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=S4IVYQ.R543O8OZ1IFR3@protonmail.com \
--to=y.oudjana@protonmail.com \
--cc=bjorn.andersson@linaro.org \
--cc=butterflyhuangxx@gmail.com \
--cc=dan.carpenter@oracle.com \
--cc=davem@davemloft.net \
--cc=kuba@kernel.org \
--cc=linux-arm-msm@vger.kernel.org \
--cc=loic.poulain@linaro.org \
--cc=mani@kernel.org \
--cc=netdev@vger.kernel.org \
/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.