On Tue, Mar 31, 2020 at 09:35:59AM -0700, Maciej Żenczykowski wrote: > From: Maciej Żenczykowski <maze@xxxxxxxxxx> > > Android has long had an extension to IDLETIMER to send netlink > messages to userspace, see: > https://android.googlesource.com/kernel/common/+/refs/heads/android-mainline/include/uapi/linux/netfilter/xt_IDLETIMER.h#42 > Note: this is idletimer target rev 1, there is no rev 0 in > the Android common kernel sources, see registration at: > https://android.googlesource.com/kernel/common/+/refs/heads/android-mainline/net/netfilter/xt_IDLETIMER.c#483 > > When we compare that to upstream's new idletimer target rev 1: > https://git.kernel.org/pub/scm/linux/kernel/git/pablo/nf-next.git/tree/include/uapi/linux/netfilter/xt_IDLETIMER.h#n46 > > We immediately notice that these two rev 1 structs are the > same size and layout, and that while timer_type and send_nl_msg > are differently named and serve a different purpose, they're > at the same offset. > > This makes them impossible to tell apart - and thus one cannot > know in a mixed Android/vanilla environment whether one means > timer_type or send_nl_msg. > > Since this is iptables/netfilter uapi it introduces a problem > between iptables (vanilla vs Android) userspace and kernel > (vanilla vs Android) if the two don't match each other. > > Additionally when at some point in the future Android picks up > 5.7+ it's not at all clear how to resolve the resulting merge > conflict. > > Furthermore, since upgrading the kernel on old Android phones > is pretty much impossible there does not seem to be an easy way > out of this predicament. > > The only thing I've been able to come up with is some super > disgusting kernel version >= 5.7 check in the iptables binary > to flip between different struct layouts. > > By adding a dummy field to the vanilla Linux kernel header file > we can force the two structs to be compatible with each other. > > Long term I think I would like to deprecate send_nl_msg out of > Android entirely, but I haven't quite been able to figure out > exactly how we depend on it. It seems to be very similar to > sysfs notifications but with some extra info. > > Currently it's actually always enabled whenever Android uses > the IDLETIMER target, so we could also probably entirely > remove it from the uapi in favour of just always enabling it, > but again we can't upgrade old kernels already in the field. > > (Also note that this doesn't change the structure's size, > as it is simply fitting into the pre-existing padding, and > that since 5.7 hasn't been released yet, there's still time > to make this uapi visible change) > > Cc: Manoj Basapathi <manojbm@xxxxxxxxxxxxxx> > Cc: Subash Abhinov Kasiviswanathan <subashab@xxxxxxxxxxxxxx> > Cc: Pablo Neira Ayuso <pablo@xxxxxxxxxxxxx> > Signed-off-by: Maciej Żenczykowski <maze@xxxxxxxxxx> > --- > include/uapi/linux/netfilter/xt_IDLETIMER.h | 1 + > 1 file changed, 1 insertion(+) > > diff --git a/include/uapi/linux/netfilter/xt_IDLETIMER.h b/include/uapi/linux/netfilter/xt_IDLETIMER.h > index 434e6506abaa..49ddcdc61c09 100644 > --- a/include/uapi/linux/netfilter/xt_IDLETIMER.h > +++ b/include/uapi/linux/netfilter/xt_IDLETIMER.h > @@ -48,6 +48,7 @@ struct idletimer_tg_info_v1 { > > char label[MAX_IDLETIMER_LABEL_SIZE]; > > + __u8 send_nl_msg; /* unused: for compatibility with Android */ Please, add client code for this send_nl_msg field. Thank you.
