| Message ID | 20230109223801.1706213-1-quic_bjorande@quicinc.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | rpmsg: glink: Avoid infinite loop on intent for missing channel | expand |
On 1/9/2023 2:38 PM, Bjorn Andersson wrote: > In the event that an intent advertisement arrives on an unknown channel > the fifo is not advanced, resulting in the same message being handled > over and over. > > Fixes: dacbb35e930f ("rpmsg: glink: Receive and store the remote intent buffers") > Signed-off-by: Bjorn Andersson <quic_bjorande@quicinc.com> > --- > drivers/rpmsg/qcom_glink_native.c | 5 +++-- > 1 file changed, 3 insertions(+), 2 deletions(-) > > diff --git a/drivers/rpmsg/qcom_glink_native.c b/drivers/rpmsg/qcom_glink_native.c > index f36740cb6866..7b1320b1579e 100644 > --- a/drivers/rpmsg/qcom_glink_native.c > +++ b/drivers/rpmsg/qcom_glink_native.c > @@ -946,12 +946,12 @@ static void qcom_glink_handle_intent(struct qcom_glink *glink, > spin_unlock_irqrestore(&glink->idr_lock, flags); > if (!channel) { > dev_err(glink->dev, "intents for non-existing channel\n"); > - return; > + goto advance_rx; > } > > msg = kmalloc(msglen, GFP_ATOMIC); > if (!msg) > - return; > + goto advance_rx; Should we be dropping the packet for this case? If we try again later more memory might be available to handle the command. > > qcom_glink_rx_peak(glink, msg, 0, msglen); > > @@ -973,6 +973,7 @@ static void qcom_glink_handle_intent(struct qcom_glink *glink, > } > > kfree(msg); > +advance_rx: > qcom_glink_rx_advance(glink, ALIGN(msglen, 8)); > } > >
On Tue, Feb 14, 2023 at 02:55:02PM -0800, Chris Lew wrote: > On 1/9/2023 2:38 PM, Bjorn Andersson wrote: > > In the event that an intent advertisement arrives on an unknown channel > > the fifo is not advanced, resulting in the same message being handled > > over and over. > > > > Fixes: dacbb35e930f ("rpmsg: glink: Receive and store the remote intent buffers") > > Signed-off-by: Bjorn Andersson <quic_bjorande@quicinc.com> > > --- > > drivers/rpmsg/qcom_glink_native.c | 5 +++-- > > 1 file changed, 3 insertions(+), 2 deletions(-) > > > > diff --git a/drivers/rpmsg/qcom_glink_native.c b/drivers/rpmsg/qcom_glink_native.c > > index f36740cb6866..7b1320b1579e 100644 > > --- a/drivers/rpmsg/qcom_glink_native.c > > +++ b/drivers/rpmsg/qcom_glink_native.c > > @@ -946,12 +946,12 @@ static void qcom_glink_handle_intent(struct qcom_glink *glink, > > spin_unlock_irqrestore(&glink->idr_lock, flags); > > if (!channel) { > > dev_err(glink->dev, "intents for non-existing channel\n"); > > - return; > > + goto advance_rx; > > } > > msg = kmalloc(msglen, GFP_ATOMIC); > > if (!msg) > > - return; > > + goto advance_rx; > > > Should we be dropping the packet for this case? If we try again later more > memory might be available to handle the command. > You're right, we found a channel above, but we don't have enough memory to handle the message right now. That seems like a message worth not throwing away. Thanks, Bjorn > > qcom_glink_rx_peak(glink, msg, 0, msglen); > > @@ -973,6 +973,7 @@ static void qcom_glink_handle_intent(struct qcom_glink *glink, > > } > > kfree(msg); > > +advance_rx: > > qcom_glink_rx_advance(glink, ALIGN(msglen, 8)); > > } > >
diff --git a/drivers/rpmsg/qcom_glink_native.c b/drivers/rpmsg/qcom_glink_native.c index f36740cb6866..7b1320b1579e 100644 --- a/drivers/rpmsg/qcom_glink_native.c +++ b/drivers/rpmsg/qcom_glink_native.c @@ -946,12 +946,12 @@ static void qcom_glink_handle_intent(struct qcom_glink *glink, spin_unlock_irqrestore(&glink->idr_lock, flags); if (!channel) { dev_err(glink->dev, "intents for non-existing channel\n"); - return; + goto advance_rx; } msg = kmalloc(msglen, GFP_ATOMIC); if (!msg) - return; + goto advance_rx; qcom_glink_rx_peak(glink, msg, 0, msglen); @@ -973,6 +973,7 @@ static void qcom_glink_handle_intent(struct qcom_glink *glink, } kfree(msg); +advance_rx: qcom_glink_rx_advance(glink, ALIGN(msglen, 8)); }
In the event that an intent advertisement arrives on an unknown channel the fifo is not advanced, resulting in the same message being handled over and over. Fixes: dacbb35e930f ("rpmsg: glink: Receive and store the remote intent buffers") Signed-off-by: Bjorn Andersson <quic_bjorande@quicinc.com> --- drivers/rpmsg/qcom_glink_native.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-)