Re: [PATCH v1] rpmsg: char: fix use-after-free on probe error path

[최유호]

Dear Mathieu

Thanks for the review.

I will fix the Fixes tag and remove the header in v2.
The check will be added in the next patch.

Thanks,
Yuho

On Wed, 22 Apr 2026 at 10:51, Mathieu Poirier
<mathieu.poirier@xxxxxxxxxx> wrote:
>
> Good morning,
>
> On Thu, Apr 16, 2026 at 12:23:38AM -0400, Yuho Choi wrote:
> > rpmsg_chrdev_probe() stores the newly allocated eptdev in the default
> > endpoint's priv pointer before calling rpmsg_chrdev_eptdev_add(). If
> > rpmsg_chrdev_eptdev_add() then fails, its error path frees eptdev while
> > the default endpoint may still dispatch inbound messages through
> > rpmsg_ept_cb(), leaving the callback with a dangling priv pointer.
> >
> > Avoid publishing eptdev through the default endpoint until
> > rpmsg_chrdev_eptdev_add() succeeds. Any message that arrives before the
> > priv is published should be ignored, so make rpmsg_ept_cb() return
> > success when priv is NULL.
> >
> > Fixes: bc69d10665690 ("rpmsg: char: Introduce the "rpmsg-raw" channel")
>
> The SHA should be 12 characters.
>
> > Signed-off-by: Yuho Choi <dbgh9129@xxxxxxxxx>
> > ---
> >  drivers/rpmsg/rpmsg_char.c | 13 +++++++++++--
> >  1 file changed, 11 insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/rpmsg/rpmsg_char.c b/drivers/rpmsg/rpmsg_char.c
> > index ca9cf8858a5ef..549a24b70884d 100644
> > --- a/drivers/rpmsg/rpmsg_char.c
> > +++ b/drivers/rpmsg/rpmsg_char.c
> > @@ -11,6 +11,7 @@
> >   * was based on TI & Google OMX rpmsg driver.
> >   */
> >
> > +#include "asm-generic/rwonce.h"
>
> Why is this needed?
>
> >  #define pr_fmt(fmt)  KBUILD_MODNAME ": " fmt
> >
> >  #include <linux/cdev.h>
> > @@ -104,6 +105,9 @@ static int rpmsg_ept_cb(struct rpmsg_device *rpdev, void *buf, int len,
> >       struct rpmsg_eptdev *eptdev = priv;
> >       struct sk_buff *skb;
> >
> > +     if (!eptdev)
> > +             return 0;
> > +
>
> If this is needed, the same check is probably needed in rpmsg_ept_flow_cb().
>
> Thanks,
> Mathieu
>
> >       skb = alloc_skb(len, GFP_ATOMIC);
> >       if (!skb)
> >               return -ENOMEM;
> > @@ -490,6 +494,7 @@ static int rpmsg_chrdev_probe(struct rpmsg_device *rpdev)
> >       struct rpmsg_channel_info chinfo;
> >       struct rpmsg_eptdev *eptdev;
> >       struct device *dev = &rpdev->dev;
> > +     int ret;
> >
> >       memcpy(chinfo.name, rpdev->id.name, RPMSG_NAME_SIZE);
> >       chinfo.src = rpdev->src;
> > @@ -502,13 +507,17 @@ static int rpmsg_chrdev_probe(struct rpmsg_device *rpdev)
> >       /* Set the default_ept to the rpmsg device endpoint */
> >       eptdev->default_ept = rpdev->ept;
> >
> > +     ret = rpmsg_chrdev_eptdev_add(eptdev, chinfo);
> > +
> > +     if (ret)
> > +             return ret;
> >       /*
> >        * The rpmsg_ept_cb uses *priv parameter to get its rpmsg_eptdev context.
> > -      * Storedit in default_ept *priv field.
> > +      * Stored it in default_ept *priv field.
> >        */
> >       eptdev->default_ept->priv = eptdev;
> >
> > -     return rpmsg_chrdev_eptdev_add(eptdev, chinfo);
> > +     return 0;
> >  }
> >
> >  static void rpmsg_chrdev_remove(struct rpmsg_device *rpdev)
> > --
> > 2.50.1 (Apple Git-155)
> >