linux-kernel - Re: [PATCH] drm: do not use device name as a format string

Open Source and information security mailing list archives

Message-ID: <20151206093559.GT10243@phenom.ffwll.local>
Date:	Sun, 6 Dec 2015 10:35:59 +0100
From:	Daniel Vetter <daniel@...ll.ch>
To:	Nicolas Iooss <nicolas.iooss_linux@....org>
Cc:	Boris Brezillon <boris.brezillon@...e-electrons.com>,
	David Airlie <airlied@...ux.ie>,
	Jianwei Wang <jianwei.wang.chn@...il.com>,
	Alison Wang <alison.wang@...escale.com>,
	Thierry Reding <thierry.reding@...il.com>,
	Terje Bergström <tbergstrom@...dia.com>,
	dri-devel@...ts.freedesktop.org, linux-tegra@...r.kernel.org,
	linux-kernel@...r.kernel.org
Subject: Re: [PATCH] drm: do not use device name as a format string

On Sat, Dec 05, 2015 at 10:45:50AM +0100, Nicolas Iooss wrote:
> Hello,
> I sent the path below a few weeks ago and did not have any feedback.
> Is there any issue in it that I need to fix before submitting it again?

Sorry, must have missed this.

> 
> Thanks,
> Nicolas Iooss
> 
> On 11/18/2015 06:58 PM, Nicolas Iooss wrote:
> > drm_dev_set_unique() formats its parameter using kvasprintf() but many
> > of its callers directly pass dev_name(dev) as printf format string,
> > without any format parameter.  This can cause some issues when the
> > device name contains '%' characters.
> > 
> > To avoid any potential issue, always use "%s" when using
> > drm_dev_set_unique() with dev_name().

Not sure this is worth it really, normally people don't place % characters
into their device names, ever. And if they do it'll blow up. There's also
no security issue here since userspace can't set this name.

If the maintainers of the affected drivers don't want this I won't merge
this patch.

Thanks, Daniel

> > 
> > Signed-off-by: Nicolas Iooss <nicolas.iooss_linux@....org>
> > ---
> >  drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.c | 2 +-
> >  drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_drv.c    | 2 +-
> >  drivers/gpu/drm/tegra/drm.c                  | 2 +-
> >  drivers/gpu/drm/vc4/vc4_drv.c                | 2 +-
> >  include/drm/drmP.h                           | 1 +
> >  5 files changed, 5 insertions(+), 4 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.c b/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.c
> > index 244df0a440b7..0d720d3a7ee0 100644
> > --- a/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.c
> > +++ b/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.c
> > @@ -733,7 +733,7 @@ static int atmel_hlcdc_dc_drm_probe(struct platform_device *pdev)
> >  	if (!ddev)
> >  		return -ENOMEM;
> >  
> > -	ret = drm_dev_set_unique(ddev, dev_name(ddev->dev));
> > +	ret = drm_dev_set_unique(ddev, "%s", dev_name(ddev->dev));
> >  	if (ret)
> >  		goto err_unref;
> >  
> > diff --git a/drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_drv.c b/drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_drv.c
> > index 1930234ba5f1..947d75f59881 100644
> > --- a/drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_drv.c
> > +++ b/drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_drv.c
> > @@ -363,7 +363,7 @@ static int fsl_dcu_drm_probe(struct platform_device *pdev)
> >  	fsl_dev->np = dev->of_node;
> >  	drm->dev_private = fsl_dev;
> >  	dev_set_drvdata(dev, fsl_dev);
> > -	drm_dev_set_unique(drm, dev_name(dev));
> > +	drm_dev_set_unique(drm, "%s", dev_name(dev));
> >  
> >  	ret = drm_dev_register(drm, 0);
> >  	if (ret < 0)
> > diff --git a/drivers/gpu/drm/tegra/drm.c b/drivers/gpu/drm/tegra/drm.c
> > index 159ef515cab1..b278f60f4376 100644
> > --- a/drivers/gpu/drm/tegra/drm.c
> > +++ b/drivers/gpu/drm/tegra/drm.c
> > @@ -991,7 +991,7 @@ static int host1x_drm_probe(struct host1x_device *dev)
> >  	if (!drm)
> >  		return -ENOMEM;
> >  
> > -	drm_dev_set_unique(drm, dev_name(&dev->dev));
> > +	drm_dev_set_unique(drm, "%s", dev_name(&dev->dev));
> >  	dev_set_drvdata(&dev->dev, drm);
> >  
> >  	err = drm_dev_register(drm, 0);
> > diff --git a/drivers/gpu/drm/vc4/vc4_drv.c b/drivers/gpu/drm/vc4/vc4_drv.c
> > index 6e730605edcc..c90a451aaa79 100644
> > --- a/drivers/gpu/drm/vc4/vc4_drv.c
> > +++ b/drivers/gpu/drm/vc4/vc4_drv.c
> > @@ -168,7 +168,7 @@ static int vc4_drm_bind(struct device *dev)
> >  	vc4->dev = drm;
> >  	drm->dev_private = vc4;
> >  
> > -	drm_dev_set_unique(drm, dev_name(dev));
> > +	drm_dev_set_unique(drm, "%s", dev_name(dev));
> >  
> >  	drm_mode_config_init(drm);
> >  	if (ret)
> > diff --git a/include/drm/drmP.h b/include/drm/drmP.h
> > index 0b921ae06cd8..995fb96cb740 100644
> > --- a/include/drm/drmP.h
> > +++ b/include/drm/drmP.h
> > @@ -1049,6 +1049,7 @@ void drm_dev_ref(struct drm_device *dev);
> >  void drm_dev_unref(struct drm_device *dev);
> >  int drm_dev_register(struct drm_device *dev, unsigned long flags);
> >  void drm_dev_unregister(struct drm_device *dev);
> > +__printf(2, 3)
> >  int drm_dev_set_unique(struct drm_device *dev, const char *fmt, ...);
> >  
> >  struct drm_minor *drm_minor_acquire(unsigned int minor_id);
> > 
> 
> _______________________________________________
> dri-devel mailing list
> dri-devel@...ts.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/dri-devel

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives