libgeom: Avoid fixed remappings of the devstat device

[markj]

Summary

libgeom maintains a quasi-private mapping of /dev/devstat, which might grow
over time if new devices appear. When the mapping needs to be expanded, the
old mapping is passed as a hint, but this appears to be unnecessary.

Thus:

• stop passing a hint when remapping
• don't create a mapping in geom_stats_open(), as geom_stats_resync() will create it for us,
• check for errors from munmap().

[jrtc27]

lib/libgeom/geom_stats.c

77

This isn't using MAP_FIXED? It's just hinting to use the old address, though you're still right it's unnecessary.

[markj]

lib/libgeom/geom_stats.c

77

Oh, duh. I confused myself when trying to understand why this fails under CheriABI (a tagged hint pointer without MAP_FIXED results in EPROT). Actually, I find that behaviour somewhat surprising.

But yes, there's no apparent reason to pass a hint at all. Perhaps, in an earlier iteration of this design, geom_stats_snapshot_get() didn't make a copy of the devstat mapping but instead returned a pointer to the mapping directly. Then, geom_stats_resync() would invalidate the pointer. But, without MAP_FIXED, that can happen anyway..

[asomers]

This LGTM. And BTW, I successfully tested it with an out-of-tree libgeom consumer.

[imp]

Looks good to me, modulo the err() in a library thing... but it's already a thing for too many errors in libgeom and those are kinda hard to flush out.

[brooks]

lib/libgeom/geom_stats.c

77

Oh, duh. I confused myself when trying to understand why this fails under CheriABI (a tagged hint pointer without MAP_FIXED results in EPROT). Actually, I find that behaviour somewhat surprising.

FWIW, the reason this is an error in CheriBSD is that we currently reject non-NULL derived, untagged capabilities and I don't want success to be racy in the face of revocation. It's possible we could be more relaxed here, but I mostly feel like it's better to be a bit too rigid to push people away from bogus use of hints.