From patchwork Mon Jun 19 18:44:31 2017 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Joe Stringer X-Patchwork-Id: 777937 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Received: from mail.linuxfoundation.org (mail.linuxfoundation.org [140.211.169.12]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by ozlabs.org (Postfix) with ESMTPS id 3ws0Jt1XzFz9ryT for ; Tue, 20 Jun 2017 04:45:02 +1000 (AEST) Received: from mail.linux-foundation.org (localhost [127.0.0.1]) by mail.linuxfoundation.org (Postfix) with ESMTP id 980F5B76; Mon, 19 Jun 2017 18:44:58 +0000 (UTC) X-Original-To: dev@openvswitch.org Delivered-To: ovs-dev@mail.linuxfoundation.org Received: from smtp1.linuxfoundation.org (smtp1.linux-foundation.org [172.17.192.35]) by mail.linuxfoundation.org (Postfix) with ESMTPS id 59C9CB59 for ; Mon, 19 Jun 2017 18:44:57 +0000 (UTC) X-Greylist: domain auto-whitelisted by SQLgrey-1.7.6 Received: from relay3-d.mail.gandi.net (relay3-d.mail.gandi.net [217.70.183.195]) by smtp1.linuxfoundation.org (Postfix) with ESMTPS id CDF84124 for ; Mon, 19 Jun 2017 18:44:56 +0000 (UTC) Received: from mfilter22-d.gandi.net (mfilter22-d.gandi.net [217.70.178.150]) by relay3-d.mail.gandi.net (Postfix) with ESMTP id 74771A80C0 for ; Mon, 19 Jun 2017 20:44:55 +0200 (CEST) X-Virus-Scanned: Debian amavisd-new at mfilter22-d.gandi.net Received: from relay3-d.mail.gandi.net ([IPv6:::ffff:217.70.183.195]) by mfilter22-d.gandi.net (mfilter22-d.gandi.net [::ffff:10.0.15.180]) (amavisd-new, port 10024) with ESMTP id HJxKsW_917MV for ; Mon, 19 Jun 2017 20:44:54 +0200 (CEST) X-Originating-IP: 209.85.218.48 Received: from mail-oi0-f48.google.com (mail-oi0-f48.google.com [209.85.218.48]) (Authenticated sender: joe@ovn.org) by relay3-d.mail.gandi.net (Postfix) with ESMTPSA id D3BFDA80D0 for ; Mon, 19 Jun 2017 20:44:53 +0200 (CEST) Received: by mail-oi0-f48.google.com with SMTP id p187so1578694oif.3 for ; Mon, 19 Jun 2017 11:44:53 -0700 (PDT) X-Gm-Message-State: AKS2vOzfBTec6icdMupzWPNJsEzxTjd01KF1gh2iKRySTfWrJpjLNP0U gQtRTXcU1tdCFH704NCa8yKhRQ1NWA== X-Received: by 10.202.175.19 with SMTP id y19mr13357464oie.132.1497897892295; Mon, 19 Jun 2017 11:44:52 -0700 (PDT) MIME-Version: 1.0 Received: by 10.157.43.140 with HTTP; Mon, 19 Jun 2017 11:44:31 -0700 (PDT) In-Reply-To: <1497656229-14712-1-git-send-email-gvrose8192@gmail.com> References: <1497656229-14712-1-git-send-email-gvrose8192@gmail.com> From: Joe Stringer Date: Mon, 19 Jun 2017 11:44:31 -0700 X-Gmail-Original-Message-ID: Message-ID: To: Greg Rose X-Spam-Status: No, score=-2.6 required=5.0 tests=BAYES_00, RCVD_IN_DNSWL_LOW autolearn=ham version=3.3.1 X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on smtp1.linux-foundation.org Cc: ovs dev Subject: Re: [ovs-dev] [PATCH V3] compat: Restrict __ro_after_init usage X-BeenThere: ovs-dev@openvswitch.org X-Mailman-Version: 2.1.12 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Sender: ovs-dev-bounces@openvswitch.org Errors-To: ovs-dev-bounces@openvswitch.org On 16 June 2017 at 16:37, Greg Rose 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 > Signed-off-by: Greg Rose > --- 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 +/* 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