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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 4/19/20 12:58 AM, Christoph Hellwig wrote:
On Sat, Apr 18, 2020 at 08:40:20AM -0700, Bart Van Assche wrote:
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?

How about using kvasprintf() instead of vsnprintf()?

That is what v1 did, see the thread in response to that on why it isn't
a good idea.

Are you perhaps referring to patch "[PATCH 3/8] bdi: add a ->dev_name field to struct backing_dev_info" (https://lore.kernel.org/linux-block/20200416071519.807660-4-hch@xxxxxx/) and also to the replies to that patch? This is what I found in the replies: "When driver try to to re-register bdi but without release_bdi(), the old dev_name will be cover directly by the newer in bdi_register_va(). So, I am not sure whether it can cause memory leak for bdi->dev_name."

Has it been considered to avoid that leak by freeing bdi->dev_name from unregister_bdi(), e.g. as follows?

void bdi_unregister(struct backing_dev_info *bdi)
{
+	char *dev_name;

	/* make sure nobody finds us on the bdi_list anymore */
	bdi_remove_from_list(bdi);
	wb_shutdown(&bdi->wb);
	cgwb_bdi_unregister(bdi);

	if (bdi->dev) {
		bdi_debug_unregister(bdi);
		device_unregister(bdi->dev);
		bdi->dev = NULL;
+		dev_name = bdi->dev_name;
+		bdi->dev_name = "(unregistered)";
+		kfree(dev_name);
	}

	if (bdi->owner) {
		put_device(bdi->owner);
		bdi->owner = NULL;
	}
}

Thanks,

Bart.



[Index of Archives]     [Linux RAID]     [Linux SCSI]     [Linux ATA RAID]     [IDE]     [Linux Wireless]     [Linux Kernel]     [ATH6KL]     [Linux Bluetooth]     [Linux Netdev]     [Kernel Newbies]     [Security]     [Git]     [Netfilter]     [Bugtraq]     [Yosemite News]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Device Mapper]

  Powered by Linux