From: Dan Carpenter <dan.carpenter@oracle.com>
To: Jing Xiangfeng <jingxiangfeng@huawei.com>
Cc: mporter@kernel.crashing.org, alex.bou9@gmail.com,
akpm@linux-foundation.org, gustavoars@kernel.org,
jhubbard@nvidia.com, keescook@chromium.org,
madhuparnabhowmik10@gmail.com, linux-kernel@vger.kernel.org
Subject: Re: [PATCH] rapidio: fix the missed put_device() for rio_mport_add_riodev
Date: Tue, 22 Sep 2020 12:52:55 +0300 [thread overview]
Message-ID: <20200922095255.GC18329@kadam> (raw)
In-Reply-To: <5F69C18A.2070800@huawei.com>
On Tue, Sep 22, 2020 at 05:19:06PM +0800, Jing Xiangfeng wrote:
>
>
> On 2020/9/22 16:04, Dan Carpenter wrote:
> > On Tue, Sep 22, 2020 at 03:25:25PM +0800, Jing Xiangfeng wrote:
> > > rio_mport_add_riodev() misses to call put_device() when the device
> > > already exists. Add the missed function call to fix it.
> > >
> > Looks good.
> >
> > Reviewed-by: Dan Carpenter <dan.carpenter@oracle.com>
> >
> > I notice that rio_mport_del_riodev() has a related bug.
> >
> > 1802 err = rio_add_device(rdev);
> > 1803 if (err)
> > 1804 goto cleanup;
> > 1805 rio_dev_get(rdev);
> > ^^^^^^^^^^^^^^^^^
> > This calls get_device(&rdev->dev);
> >
> > 1806
> > 1807 return 0;
> > 1808 cleanup:
> > 1809 kfree(rdev);
> > 1810 return err;
> > 1811 }
> > 1812
> > 1813 static int rio_mport_del_riodev(struct mport_cdev_priv *priv, void __user *arg)
> > 1814 {
> > 1815 struct rio_rdev_info dev_info;
> > 1816 struct rio_dev *rdev = NULL;
> > 1817 struct device *dev;
> > 1818 struct rio_mport *mport;
> > 1819 struct rio_net *net;
> > 1820
> > 1821 if (copy_from_user(&dev_info, arg, sizeof(dev_info)))
> > 1822 return -EFAULT;
> > 1823 dev_info.name[sizeof(dev_info.name) - 1] = '\0';
> > 1824
> > 1825 mport = priv->md->mport;
> > 1826
> > 1827 /* If device name is specified, removal by name has priority */
> > 1828 if (strlen(dev_info.name)) {
> > 1829 dev = bus_find_device_by_name(&rio_bus_type, NULL,
> > 1830 dev_info.name);
> > 1831 if (dev)
> > 1832 rdev = to_rio_dev(dev);
> >
> > This path takes a second get_device(&rdev->dev);
> >
> > 1833 } else {
> > 1834 do {
> > 1835 rdev = rio_get_comptag(dev_info.comptag, rdev);
> > 1836 if (rdev && rdev->dev.parent == &mport->net->dev &&
> > 1837 rdev->destid == dev_info.destid &&
> > 1838 rdev->hopcount == dev_info.hopcount)
> > 1839 break;
> >
> > This path does not call get_device().
> Add calling rio_dev_get() in this path? like the following changes:
>
> static int rio_mport_del_riodev(struct mport_cdev_priv *priv, void __user
> *arg)
> rdev = rio_get_comptag(dev_info.comptag, rdev);
> if (rdev && rdev->dev.parent == &mport->net->dev &&
> rdev->destid == dev_info.destid &&
> - rdev->hopcount == dev_info.hopcount)
> + rdev->hopcount == dev_info.hopcount) {
> + rio_dev_get(rdev);
> break;
> + }
To be honest, I'm not sure how the rio_get_comptag() stuff is supposed
to work... It probably requires some thought and testing.
Anyway, your patch is straight forward enough so let's just merge that
and hope someone with the hardware can take a look at this.
regards,
dan carpenter
prev parent reply other threads:[~2020-09-22 9:53 UTC|newest]
Thread overview: 4+ messages / expand[flat|nested] mbox.gz Atom feed top
2020-09-22 7:25 [PATCH] rapidio: fix the missed put_device() for rio_mport_add_riodev Jing Xiangfeng
2020-09-22 8:04 ` Dan Carpenter
2020-09-22 9:19 ` Jing Xiangfeng
2020-09-22 9:52 ` Dan Carpenter [this message]
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=20200922095255.GC18329@kadam \
--to=dan.carpenter@oracle.com \
--cc=akpm@linux-foundation.org \
--cc=alex.bou9@gmail.com \
--cc=gustavoars@kernel.org \
--cc=jhubbard@nvidia.com \
--cc=jingxiangfeng@huawei.com \
--cc=keescook@chromium.org \
--cc=linux-kernel@vger.kernel.org \
--cc=madhuparnabhowmik10@gmail.com \
--cc=mporter@kernel.crashing.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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox