Re: [PATCH 3/8] bdi: add a ->dev_name field to struct backing_dev_info

[Jan Kara]

On Thu 16-04-20 18:54:48, Christoph Hellwig wrote:
> Cache a copy of the name for the life time of the backing_dev_info
> structure so that we can reference it even after unregistering.
> 
> Fixes: 68f23b89067f ("memcg: fix a crash in wb_workfn when a device disappears")
> Reported-by: Yufen Yu <yuyufen@xxxxxxxxxx>
> Signed-off-by: Christoph Hellwig <hch@xxxxxx>

...

> @@ -938,7 +938,8 @@ int bdi_register_va(struct backing_dev_info *bdi, const char *fmt, va_list args)
>  	if (bdi->dev)	/* The driver needs to use separate queues per device */
>  		return 0;
>  
> -	dev = device_create_vargs(bdi_class, NULL, MKDEV(0, 0), bdi, fmt, args);
> +	vsnprintf(bdi->dev_name, sizeof(bdi->dev_name), fmt, args);
> +	dev = device_create(bdi_class, NULL, MKDEV(0, 0), bdi, bdi->dev_name);
>  	if (IS_ERR(dev))
>  		return PTR_ERR(dev);
>  

This can have a sideeffect not only bdi->dev_name will be truncated to 64
chars (which generally doesn't matter) but possibly also kobject name will
be truncated in the same way.  Which may have user visible effects. E.g.
for fs/vboxsf 64 chars need not be enough. So shouldn't we rather do it the
other way around - i.e., let device_create_vargs() create the device name
and then copy to bdi->dev_name whatever fits?

								Honza
-- 
Jan Kara <jack@xxxxxxxx>
SUSE Labs, CR