On 19 June 2017 at 11:44, Joe Stringer <[email protected]> wrote: > On 16 June 2017 at 16:37, Greg Rose <[email protected]> wrote: >> The attribute __ro_after_init was introduced in Linux kernel 4.5. If >> a data structure is given this attribute then after the driver module >> loads the memory page where the data resides will be marked read only. >> >> The compat code in cache.h always defines __ro_after_init if it is not >> already defined so that it can be used as an attribute for the datapath >> genl_family structure definitions. If __ro_after_init is defined then >> it is used "as-is" where it will apply the read only attribute after >> driver initialization. >> >> This is incorrect usage for the Generic Netlink genl_family structure >> definitions prior to Linux kernel 4.10. The genl_family structure >> in those kernels includes a list header member that will be written >> to when the generic netlink family is unregistered. This will cause >> a subsequent page fault and kernel panic because at this time the >> genl_family structure data has been marked read only in the page >> descriptor. >> >> A new compat macro is introduced in acinclude.m4 to detect when the >> genl_family structure has the family_list list header as a member. >> In this case HAVE_GENL_FAMILY_LIST is defined and if __ro_after_init >> is also defined then it is undefined and redefined as empty. This >> will prevent the genl_family data structure from being marked read >> only in kernels 4.5 through 4.9 and thus prevent the page fault when >> the generic netlink families in datapath.c are unregistered. >> >> Fixes: ba63fe260bd5 ("datapath: Allow compile against current net-next.") >> CC: Jarno Rajahalme <[email protected]> >> Signed-off-by: Greg Rose <[email protected]> >> --- > > Nice find, thanks for the patch! > > Sometimes it's nice to have some of this context from the commit > message above the macro define. I'm considering to roll this > incremental into the patch before applying to master shortly: > > diff --git a/datapath/linux/compat/include/linux/cache.h > b/datapath/linux/compat/include/linux/cache.h > index 35da4e70ce5c..87f543722b62 100644 > --- a/datapath/linux/compat/include/linux/cache.h > +++ b/datapath/linux/compat/include/linux/cache.h > @@ -3,6 +3,12 @@ > > #include_next <linux/cache.h> > > +/* Commit c74ba8b3480d ("arch: Introduce post-init read-only memory") > + * introduced the __ro_after_init attribute, however it wasn't applied to > + * generic netlink sockets until commit 34158151d2aa ("netfilter: cttimeout: > + * use nf_ct_iterate_cleanup_net to unlink timeout objs"). Using it on > + * genetlink before the latter commit leads to crash on module unload. > + * For kernels < 4.10, define it as empty. */ > #ifdef HAVE_GENL_FAMILY_LIST > #ifdef __ro_after_init > #undef __ro_after_init
Applied to master and branch-2.7. _______________________________________________ dev mailing list [email protected] https://mail.openvswitch.org/mailman/listinfo/ovs-dev
