[ovs-dev] datapath: compat: Fix compilation issue with grsecurity patch

Grsecurity patch enables GCC's constify plugin so that it will
automatically constify a class of type that contains only function
pointers.  However, if the type is also specified by __read_mostly, it
will put the constify object into the read_mostly section that results
in compilation error.  This patch works around the compilation issue by
disabling __ready_mostly when grsecurity patch is applied.

Tested with 4.14.33 kernel with grsecurity patch.

Signed-off-by: Yi-Hung Wei <yihung.wei@gmail.com>
---
 datapath/compat.h | 7 +++++++
 1 file changed, 7 insertions(+)

On 10/10/2018 4:21 PM, Yi-Hung Wei wrote:
> Grsecurity patch enables GCC's constify plugin so that it will
> automatically constify a class of type that contains only function
> pointers.  However, if the type is also specified by __read_mostly, it
> will put the constify object into the read_mostly section that results
> in compilation error.  This patch works around the compilation issue by
> disabling __ready_mostly when grsecurity patch is applied.
>
> Tested with 4.14.33 kernel with grsecurity patch.
>
> Signed-off-by: Yi-Hung Wei <yihung.wei@gmail.com>
> ---
>   datapath/compat.h | 7 +++++++
>   1 file changed, 7 insertions(+)
>
> diff --git a/datapath/compat.h b/datapath/compat.h
> index 98b68640a372..b820251a4767 100644
> --- a/datapath/compat.h
> +++ b/datapath/compat.h
> @@ -28,6 +28,13 @@
>   #include <net/netfilter/ipv6/nf_defrag_ipv6.h>
>   #include <net/netfilter/nf_conntrack_count.h>
>   
> +/* Fix grsecurity patch compilation issue. */
> +#ifdef CONSTIFY_PLUGIN
> +#include <linux/cache.h>
> +#undef __read_mostly
> +#define __read_mostly
> +#endif
> +
>   /* Even though vanilla 3.10 kernel has grp->id, RHEL 7 kernel is missing
>    * this field. */
>   #ifdef HAVE_GENL_MULTICAST_GROUP_WITH_ID

The patch looks good - I did see a few errors on the Travis build but 
they did not look related to this
patch since they showed up on the master build as well.

I'm also now getting a compile error at the end of the build which looks 
a bit strange:
building 'ovs._json' extension
x86_64-linux-gnu-gcc -pthread -DNDEBUG -g -fwrapv -O2 -Wall 
-Wstrict-prototypes -g -fstack-protector-strong -Wformat 
-Werror=format-security -Wdate-time -D_FORTIFY_SOURCE=2 -fPIC 
-I/home/gvrose/prj/ovs-experimental/_build/../include 
-I/home/gvrose/prj/ovs-experimental/_build/include 
-I/usr/include/python3.5m -c ovs/_json.c -o 
build/temp.linux-x86_64-3.5/ovs/_json.o
x86_64-linux-gnu-gcc -pthread -shared -Wl,-O1 -Wl,-Bsymbolic-functions 
-Wl,-Bsymbolic-functions -Wl,-z,relro -Wl,-Bsymbolic-functions 
-Wl,-z,relro -g -fstack-protector-strong -Wformat 
-Werror=format-security -Wdate-time -D_FORTIFY_SOURCE=2 
build/temp.linux-x86_64-3.5/ovs/_json.o 
-L/home/gvrose/prj/ovs-experimental/_build/lib/.libs -lopenvswitch -o 
/home/gvrose/prj/ovs-experimental/_build/python/ovs/_json.cpython-35m-x86_64-linux-gnu.so
/usr/bin/ld: 
/home/gvrose/prj/ovs-experimental/_build/lib/.libs/libopenvswitch.a(json.o): 
relocation R_X86_64_32 against `.rodata.str1.1' can not be used when 
making a shared object; recompile with -fPIC
/home/gvrose/prj/ovs-experimental/_build/lib/.libs/libopenvswitch.a: 
error adding symbols: Bad value
collect2: error: ld returned 1 exit status

I tried 'make EXTRA_CFLAGS=-fPIC' as suggested but the gcc 7.3 compiler 
doesn't like it and then I get this message:
   CC [M] /home/gvrose/prj/ovs-experimental/_build/datapath/linux/actions.o
/home/gvrose/prj/ovs-experimental/_build/datapath/linux/actions.c:1:0: 
error: code model kernel does not support PIC mode

But this happens on the master branch as well, so again it does not seem 
related to your patch.

So I think the patch is fine.  Passes check-kmod and checkpatch.

Tested-by: Greg Rose <gvrose8192@gmail.com>
Reviewed-by: Greg Rose <gvrose8192@gmail.com>

On Mon, Oct 15, 2018 at 11:01:45AM -0700, Gregory Rose wrote:
> I'm also now getting a compile error at the end of the build which looks a
> bit strange:
> building 'ovs._json' extension

Should be fixed with a recent revert.

> So I think the patch is fine.  Passes check-kmod and checkpatch.
> 
> Tested-by: Greg Rose <gvrose8192@gmail.com>
> Reviewed-by: Greg Rose <gvrose8192@gmail.com>

Thanks, applied to master, let me know if you want backports.

On Mon, Oct 15, 2018 at 11:24 AM Ben Pfaff <blp@ovn.org> wrote:
>
> On Mon, Oct 15, 2018 at 11:01:45AM -0700, Gregory Rose wrote:
> > I'm also now getting a compile error at the end of the build which looks a
> > bit strange:
> > building 'ovs._json' extension
>
> Should be fixed with a recent revert.
>
> > So I think the patch is fine.  Passes check-kmod and checkpatch.
> >
> > Tested-by: Greg Rose <gvrose8192@gmail.com>
> > Reviewed-by: Greg Rose <gvrose8192@gmail.com>
>
> Thanks, applied to master, let me know if you want backports.

Hi Ben,

It would be great if we can backport it to 2.10.

Thanks,

-Yi-Hung

On Mon, Oct 15, 2018 at 11:40:59AM -0700, Yi-Hung Wei wrote:
> On Mon, Oct 15, 2018 at 11:24 AM Ben Pfaff <blp@ovn.org> wrote:
> >
> > On Mon, Oct 15, 2018 at 11:01:45AM -0700, Gregory Rose wrote:
> > > I'm also now getting a compile error at the end of the build which looks a
> > > bit strange:
> > > building 'ovs._json' extension
> >
> > Should be fixed with a recent revert.
> >
> > > So I think the patch is fine.  Passes check-kmod and checkpatch.
> > >
> > > Tested-by: Greg Rose <gvrose8192@gmail.com>
> > > Reviewed-by: Greg Rose <gvrose8192@gmail.com>
> >
> > Thanks, applied to master, let me know if you want backports.
> 
> Hi Ben,
> 
> It would be great if we can backport it to 2.10.

Done!