netlink: make netlink work correctly on CHERI.

[melifaro]

.

[jrtc27]

sys/netlink/netlink_message_writer.h

54

Is the only case that relied on the type punning the copy in _nlmsg_refill_buffer? If so, why not leave the union as is (or decrease arg_uint's size if it doesn't actually need to store 64-bit integers, or even split it up into protocol+group so you don't have to combine+split it all the time) and then just give the union itself a name so _nlmsg_refill_buffer can copy the whole thing instead of copying a field and punning? Something like:

union {
    void *ptr;
    struct {
        uint16_t protocol; /* or whatever is the right type */
        uint16_t id;
    } group;
} arg;

Using uintptr_t for the field is better than type punning like you were before, but it's still lazy programming practice that can make it unclear what's going on.

[jhb]

This needs more detail on the "why" in the log message. I suspect the real issue is that the copy of ns_new.arg_uint = nw->arg_unit was insufficient on CHERI since it wouldn't copy the ptr member properly (and if so, that needs to be explicitly explained), but I can't see why this would affect i386. i386 is little endian so pointers in the low bits should have been copied across ok by the existing arg_uint copy.

I think it would actually be cleaner to keep the union as:

union {
    void *ptr;
    uint64_t uint;
} arg;

Then in _nlmsg_refill_buffer you would copy arg as you do now which would always copy the entire union. Existing assignments could remain, just having to replace arg_ptr with arg.ptr, etc. This keeps the current behavior of making it clearer when you are using a scalar vs a pointer.

[melifaro]

Address comments.

[jrtc27]

Looks much cleaner now thanks, just one print I spotted to fix

sys/netlink/netlink_message_writer.c

248–249

proto + id?

[melifaro]

In D39557#900445, @jhb wrote:

This needs more detail on the "why" in the log message. I suspect the real issue is that the copy of ns_new.arg_uint = nw->arg_unit was insufficient on CHERI since it wouldn't copy the ptr member properly (and if so, that needs to be explicitly explained), but I can't see why this would affect i386. i386 is little endian so pointers in the low bits should have been copied across ok by the existing arg_uint copy.

Yep, thanks for mentioning it! I'll update the details upon the commit.

I think it would actually be cleaner to keep the union as:

union {
    void *ptr;
    uint64_t uint;
} arg;

Then in _nlmsg_refill_buffer you would copy arg as you do now which would always copy the entire union. Existing assignments could remain, just having to replace arg_ptr with arg.ptr, etc. This keeps the current behavior of making it clearer when you are using a scalar vs a pointer.

[melifaro]

Address comments.

[melifaro]

sys/netlink/netlink_message_writer.c

248–249

Yep, thanks for pointing that out, I missed that.

[jrtc27]

Diff looks good now, just needs a proper commit message