cdev: dev_copyname copies a character device's name to a buffer

[jhb]

Summary

This function can be called while not in a cdevsw method unlike
devtoname which requires the caller to hold a reference on the cdev.

[kib]

The reference on cdev still must be held, otherwise dev_refthread() might end up accessing freed memory. For instance, for the csw methods, the safety comes from the fact that devfs VCHR vnodes keep such references on cdev.
Then, cdev->si_name memory is part of the struct cdev, and we do not modify it until free, so dev_copyname() alone is somewhat excessive. Note that vm_object_list_handler() use of dev_refthread() is a) to avoid reporting names for dead VCHR nodes b) to provide some extra safety for obj->cdev memory walk. b) might be too cautious.

[jhb]

In D49334#1125116, @kib wrote:

The reference on cdev still must be held, otherwise dev_refthread() might end up accessing freed memory. For instance, for the csw methods, the safety comes from the fact that devfs VCHR vnodes keep such references on cdev.
Then, cdev->si_name memory is part of the struct cdev, and we do not modify it until free, so dev_copyname() alone is somewhat excessive. Note that vm_object_list_handler() use of dev_refthread() is a) to avoid reporting names for dead VCHR nodes b) to provide some extra safety for obj->cdev memory walk. b) might be too cautious.

Hmmm, I was worried that other device pagers might want to also use this if they store a pointer to their cdev in the handle they use for the pager (like the netmap case) vs just inlining the logic into old_cdev_pager_path. Do you think the dead VCHR case is just as relevant for other device pager objects?

[kib]

In D49334#1125306, @jhb wrote:

In D49334#1125116, @kib wrote:

The reference on cdev still must be held, otherwise dev_refthread() might end up accessing freed memory. For instance, for the csw methods, the safety comes from the fact that devfs VCHR vnodes keep such references on cdev.
Then, cdev->si_name memory is part of the struct cdev, and we do not modify it until free, so dev_copyname() alone is somewhat excessive. Note that vm_object_list_handler() use of dev_refthread() is a) to avoid reporting names for dead VCHR nodes b) to provide some extra safety for obj->cdev memory walk. b) might be too cautious.

Hmmm, I was worried that other device pagers might want to also use this if they store a pointer to their cdev in the handle they use for the pager (like the netmap case) vs just inlining the logic into old_cdev_pager_path. Do you think the dead VCHR case is just as relevant for other device pager objects?

It could be, it really depends on what d_close() does.

[jhb]

In D49334#1125310, @kib wrote:

In D49334#1125306, @jhb wrote:

In D49334#1125116, @kib wrote:

The reference on cdev still must be held, otherwise dev_refthread() might end up accessing freed memory. For instance, for the csw methods, the safety comes from the fact that devfs VCHR vnodes keep such references on cdev.
Then, cdev->si_name memory is part of the struct cdev, and we do not modify it until free, so dev_copyname() alone is somewhat excessive. Note that vm_object_list_handler() use of dev_refthread() is a) to avoid reporting names for dead VCHR nodes b) to provide some extra safety for obj->cdev memory walk. b) might be too cautious.

Hmmm, I was worried that other device pagers might want to also use this if they store a pointer to their cdev in the handle they use for the pager (like the netmap case) vs just inlining the logic into old_cdev_pager_path. Do you think the dead VCHR case is just as relevant for other device pager objects?

It could be, it really depends on what d_close() does.

Hmm, to be clear, by a dead VCHR node, you mean that destroy_dev() has been called, but the VM object is still around? My assumption is a character device using the default device pager can't be safely destroyed unless it knows for certain that the VM object doesn't exist (which is hard to do without races).

[kib]

In D49334#1125648, @jhb wrote:

Hmm, to be clear, by a dead VCHR node, you mean that destroy_dev() has been called, but the VM object is still around? My assumption is a character device using the default device pager can't be safely destroyed unless it knows for certain that the VM object doesn't exist (which is hard to do without races).

Yes and yes.