On Wed, 22 Oct 2025 at 10:41, Mathieu Poirier <[email protected]> wrote: > > On Wed, 22 Oct 2025 at 09:54, Dawei Li <[email protected]> wrote: > > > > Hi Dan, > > > > Thanks for the report. > > > > On Wed, Oct 22, 2025 at 02:05:36PM +0300, Dan Carpenter wrote: > > > Hello Dawei Li, > > > > > > Commit 2410558f5f11 ("rpmsg: char: Implement eptdev based on > > > anonymous inode") from Oct 15, 2025 (linux-next), leads to the > > > following Smatch static checker warning: > > > > > > drivers/rpmsg/rpmsg_char.c:548 rpmsg_anonymous_eptdev_create() > > > error: dereferencing freed memory 'eptdev' (line 546) > > > > > > drivers/rpmsg/rpmsg_char.c > > > 538 /* Anonymous inode only supports these file flags */ > > > 539 if (flags & ~(O_ACCMODE | O_NONBLOCK | O_CLOEXEC)) > > > 540 return -EINVAL; > > > 541 > > > 542 eptdev = rpmsg_eptdev_alloc(rpdev, parent, false); > > > 543 if (IS_ERR(eptdev)) > > > 544 return PTR_ERR(eptdev); > > > 545 > > > 546 ret = rpmsg_eptdev_add(eptdev, chinfo, false); > > > 547 if (ret) { > > > --> 548 dev_err(&eptdev->dev, "failed to add %s\n", > > > eptdev->chinfo.name); > > > ^^^^^^ ^^^^^^ > > > The rpmsg_eptdev_add() function frees "eptdev" on error. > > > > > > 549 return ret; > > > 550 } > > > 551 > > > 552 fd = anon_inode_getfd("rpmsg-eptdev", > > > &rpmsg_anonymous_eptdev_fops, eptdev, flags); > > > 553 if (fd < 0) { > > > 554 put_device(&eptdev->dev); > > > 555 return fd; > > > 556 } > > > 557 > > > 558 mutex_lock(&eptdev->ept_lock); > > > 559 ret = __rpmsg_eptdev_open(eptdev); > > > > > > Should we free eptdev if __rpmsg_eptdev_open() fails? > > > > > > 560 mutex_unlock(&eptdev->ept_lock); > > > 561 > > > 562 if (!ret) > > > 563 *pfd = fd; > > > 564 > > > 565 return ret; > > > 566 } > > > > > > regards, > > > dan carpenter > > > > Diff below should do the trick. > > > > diff --git a/drivers/rpmsg/rpmsg_char.c b/drivers/rpmsg/rpmsg_char.c > > index 34b35ea74aab..c322df56394f 100644 > > --- a/drivers/rpmsg/rpmsg_char.c > > +++ b/drivers/rpmsg/rpmsg_char.c > > @@ -494,6 +494,7 @@ static int rpmsg_eptdev_add(struct rpmsg_eptdev *eptdev, > > if (cdev) > > ida_free(&rpmsg_minor_ida, MINOR(dev->devt)); > > free_eptdev: > > + dev_err(&eptdev->dev, "failed to add %s\n", eptdev->chinfo.name); > > put_device(dev); > > kfree(eptdev); > > > > @@ -545,7 +546,6 @@ int rpmsg_anonymous_eptdev_create(struct rpmsg_device > > *rpdev, struct device *par > > > > ret = rpmsg_eptdev_add(eptdev, chinfo, false); > > if (ret) { > > - dev_err(&eptdev->dev, "failed to add %s\n", > > eptdev->chinfo.name); > > return ret; > > } > > > > @@ -561,6 +561,8 @@ int rpmsg_anonymous_eptdev_create(struct rpmsg_device > > *rpdev, struct device *par > > > > if (!ret) > > *pfd = fd; > > + else > > + put_device(&eptdev->dev); > > > > return ret; > > } > > > > Mathieu, Bjorn, > > > > What do you expect me to do about it? > > 1. Send an independent fix patch. > > 2. Squash the fix patch into previous ones and resend series again. > > 3. Wait for other (if any) bug reports and fix them in a whole. > > > > I am fine with all of them. > > > > Please send another patch I can apply on top.
I haven't received a fix for this yet. After looking into this bug report with more scrutiny I am of the opinion that @eptdev should be free'd in rpmsg_anonymous_eptdev_create() where it was allocated. Furthermore, if function anon_inode_getfd() in rpmsg_anonymous_eptdev_create() fails, function rpmsg_eptdev_release_device() will be called but I don't see a call to cdev_device_del() in there. Am I missing something? > > > Thanks, > > > > Dawei > >
