devstat: Provide 32-bit compatibility

[des]

Summary

If a 32-bit process running on a 64-bit kernel requests kern.devstat.all,
translate each struct devstat to its 32-bit equivalent before copying it
out.

Also fix a bug where an early error would be ignored if there were no
devices to report.

[kib]

sys/kern/subr_devstat.c

405

406

There should be a blank line after this one.

429

433

The comment is rather obvious, I do not see much sense in adding it.

435

A blank line is needed after this one.

446

This (and all other 64bit types) must account for BE/LE.

[des]

sys/kern/subr_devstat.c

446

Why? We don't support running LE binaries on a BE kernel or vice versa.

[des]

rf

[kib]

sys/kern/subr_devstat.c

446

I was too terse/assumed something.

Problem is that it is not quite possible to use uint64_t for definitions of the 32bit ABI structures. The type has different alignment on 32bit vs 64bit ABI e.g. on x86, 4 bytes on i386 vs 8 bytes on amd64. Our man page states that this is true for all other arches except arm, but I just looked at the linux ABI for ppc, where it is 8 bytes for both, at least for Linux ABI.

So, we have to use pair of uint32_t's when passing 64bit int to 32bit ABI process. Look at all places in e.g. freebsd32_misc.c. And then, we have to handle the word order BE vs LE.

[des]

sys/kern/subr_devstat.c

446

We don't have to handle the word order. BT_CP() just used a cast to treat the pair of uint32_t as a single uint64_t, assuming that alignment doesn't matter. I changed struct bintime32 to __packed which removes the need for using a pair of uint32_t, and the need for a cast. Likewise, struct devstat32 is __packed and works fine. Of course, this would _not_ have worked if armv7 or i386 versions of struct devstat had included padding, but they don't; all their members have sizes divisible by four.

[kib]

sys/kern/subr_devstat.c

446

Using __packed is not correct, since some arches align uint64_t to 8 bytes, for instance ppc.

And, accessing unaligned uint64_t is UB.

Please look how this stuff is handled for the whole freebsd32_misc.c.

[des]

sys/kern/subr_devstat.c

446

The original code was already doing unaligned accesses for struct bintime32.

[kib]

sys/kern/subr_devstat.c

446

Where?

bintime32 by itself is fine, it is using two int32's.

I looked at freebsd32.h, and there is a problematic handling of ffclock_estimate32, but even this is not related to bintime32 as is. I will fix this eventually.

[des]

sys/kern/subr_devstat.c

446

Look at the BT_CP() macro in <sys/abi_compat.h>.

[kib]

sys/kern/subr_devstat.c

446

So it is UB. D54663