| Message ID | 20170512190044.17541-2-digetx@gmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On 05/12/2017 10:00 PM, Dmitry Osipenko wrote: > The start = 0 is invalid and causes weird CDMA channel timeouts, presumably > some memory misuse/corruption is going on. What makes you think start = 0 is invalid? I can't see anything pointing to that in the idr code and there are many users in the kernel passing 0 as start. > > Fixes: bdd2f9cd10eb ("drm/tegra: Don't leak kernel pointer to userspace") > Signed-off-by: Dmitry Osipenko <digetx@gmail.com> > --- > drivers/gpu/drm/tegra/drm.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/gpu/drm/tegra/drm.c b/drivers/gpu/drm/tegra/drm.c > index 768750226452..732c8d98044f 100644 > --- a/drivers/gpu/drm/tegra/drm.c > +++ b/drivers/gpu/drm/tegra/drm.c > @@ -518,7 +518,7 @@ static int tegra_client_open(struct tegra_drm_file *fpriv, > if (err < 0) > return err; > > - err = idr_alloc(&fpriv->contexts, context, 0, 0, GFP_KERNEL); > + err = idr_alloc(&fpriv->contexts, context, 1, 0, GFP_KERNEL); > if (err < 0) { > client->ops->close_channel(context); > return err; >
On 14.05.2017 14:53, Mikko Perttunen wrote: > On 05/12/2017 10:00 PM, Dmitry Osipenko wrote: >> The start = 0 is invalid and causes weird CDMA channel timeouts, presumably >> some memory misuse/corruption is going on. > > What makes you think start = 0 is invalid? I can't see anything pointing to that > in the idr code and there are many users in the kernel passing 0 as start. > Well, I can't see either. You are right that there are quite many others with 0 as a start, the 1 probably just masks the bug.
On 14.05.2017 16:02, Dmitry Osipenko wrote: > On 14.05.2017 14:53, Mikko Perttunen wrote: >> On 05/12/2017 10:00 PM, Dmitry Osipenko wrote: >>> The start = 0 is invalid and causes weird CDMA channel timeouts, presumably >>> some memory misuse/corruption is going on. >> >> What makes you think start = 0 is invalid? I can't see anything pointing to that >> in the idr code and there are many users in the kernel passing 0 as start. >> > > Well, I can't see either. You are right that there are quite many others with 0 > as a start, the 1 probably just masks the bug. > Finally, I found the root of the issue. The job->client is set to the context ID in the tegra_drm_submit() and the host1x_cdma sets client ID to 0 to mark CDMA job timeout timer as already armed. I'll send V2 with a new commit description.
diff --git a/drivers/gpu/drm/tegra/drm.c b/drivers/gpu/drm/tegra/drm.c index 768750226452..732c8d98044f 100644 --- a/drivers/gpu/drm/tegra/drm.c +++ b/drivers/gpu/drm/tegra/drm.c @@ -518,7 +518,7 @@ static int tegra_client_open(struct tegra_drm_file *fpriv, if (err < 0) return err; - err = idr_alloc(&fpriv->contexts, context, 0, 0, GFP_KERNEL); + err = idr_alloc(&fpriv->contexts, context, 1, 0, GFP_KERNEL); if (err < 0) { client->ops->close_channel(context); return err;
The start = 0 is invalid and causes weird CDMA channel timeouts, presumably some memory misuse/corruption is going on. Fixes: bdd2f9cd10eb ("drm/tegra: Don't leak kernel pointer to userspace") Signed-off-by: Dmitry Osipenko <digetx@gmail.com> --- drivers/gpu/drm/tegra/drm.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)