Re: kern/54150: COMPAT_50 vs NET_RT_IFLIST

[Robert Elz <kre%munnari.OZ.AU@localhost>]

    Date:        Mon, 06 May 2019 06:35:42 +1000
    From:        matthew green <mrg%eterna.com.au@localhost>
    Message-ID:  <29305.1557088542%splode.eterna.com.au@localhost>

Thanks for the push, I had been letting this slide a bit for the past
day or so.

  | this might be the original thing i posted about:

  | i'm pretty sure all the "__align64" in sys/net/*.h are wrong.

You're probably right, but some of those have already gone away (in
the code I am using) 

The problem is, I think (in rt_msg2() in net/rtsock_shared.c):

        if ((len = rt_getlen(type)) == -1) 

where rt_getlen() does:

        switch (type) { 
        case RTM_ODELADDR: 
        case RTM_ONEWADDR:      
        case RTM_OCHGADDR:      
                if (rtsock_iflist_70_hook.hooked)
                        return sizeof(struct ifa_msghdr70);

which happens even when the actual struct that is being placed there
is ifa_msghdr50 (which doesn't have the align, whereas ifa_msggdr70
still does.  That, and the order of the final 2 fields, is the difference
between the two.)

If we could be confident that ifa_msghdr70 doesn't need the __align64
then that might fix things fairly easily.   I haven't really been
concentrating on making NetBSD 7 binaries work, as we have not had
reports that they don't (and NetBSD 7 binaries on a NetBSD8 kernel
would be a much more common thing - particularly soon after the
message formats were changed).

However, the __aligns are in net/if.h 1.174 (the branch point for
netbsd-7) so I suspect that they really are needed for that case.
They were added in 1.150 which is where PF_ROUTE was changed
(to make 64 bit clean versions).

I guess I should try (a variant of) the current patch reconstructed
for -8 - it may be that this alignment problem is only one in HEAD
because of the way the modules are built, and that with the other
issues fixed, -8 would work.

Rather I have been seeing if I can find some way to reasonably change
that sizeof to use the correct struct - the one that is actually going
to be put in the space being left for it.

I think that's hoing to need changing rt_getlen() as just the message
type is not enough info to know - nor can we just look at which compat
fiunctions are loaded into the kernel (the way the current code does)
as whenever we have compat50 we also have compat70, and having compat50
loaded does not mean that the current binary wants it.   The entry
code knows which protocol is in use (PF_ROUTE or PF_OROUTE) and just
needs to pass that down I think - then rt_getlen() could return the
appropriate length, rather than the wrong one.

This is all just a matter of bookkeeping somewhere, I am just (slowly)
looking to see if I can make the right info available in the right place
in a way that doesn't completely break the way the current compat
modules are built & used.

kre

ps: I am also not sure that the conversion of the timeval from 32 bit
time_t to 64 bit is being handled correctly, but I also don't know of
anything that actually uses that data.