Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1755581AbdC3D15 (ORCPT ); Wed, 29 Mar 2017 23:27:57 -0400 Received: from mx1.redhat.com ([209.132.183.28]:33344 "EHLO mx1.redhat.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1755282AbdC3D1z (ORCPT ); Wed, 29 Mar 2017 23:27:55 -0400 DMARC-Filter: OpenDMARC Filter v1.3.2 mx1.redhat.com 0BC557F6B2 Authentication-Results: ext-mx01.extmail.prod.ext.phx2.redhat.com; dmarc=none (p=none dis=none) header.from=redhat.com Authentication-Results: ext-mx01.extmail.prod.ext.phx2.redhat.com; spf=pass smtp.mailfrom=jeyu@redhat.com DKIM-Filter: OpenDKIM Filter v2.11.0 mx1.redhat.com 0BC557F6B2 Date: Wed, 29 Mar 2017 20:27:53 -0700 From: Jessica Yu To: Kees Cook , Jakub Kicinski , Catalin Marinas , Heiko Carstens , kbuild-all@01.org, kbuild test robot , Rusty Russell , LKML , "kernel-hardening@lists.openwall.com" Subject: Re: [PATCH v4 2/2] extable: verify address is read-only Message-ID: <20170330032752.kjh2fml4itgrkrnm@jeyu> References: <20170326210825.23255-3-ewk@edkovsky.org> <201703271633.xbYHmB37%fengguang.wu@intel.com> <20170329032825.GA1325@athena> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii; format=flowed Content-Disposition: inline In-Reply-To: <20170329032825.GA1325@athena> X-OS: Linux jeyu 4.8.12-100.fc23.x86_64 x86_64 User-Agent: NeoMutt/20161126 (1.7.1) X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.5.16 (mx1.redhat.com [10.5.110.25]); Thu, 30 Mar 2017 03:27:54 +0000 (UTC) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 4479 Lines: 105 +++ Eddie Kovsky [28/03/17 21:28 -0600]: >On 03/27/17, Kees Cook wrote: >> On Mon, Mar 27, 2017 at 1:43 AM, kbuild test robot wrote: >> > Hi Eddie, >> > >> > [auto build test ERROR on next-20170323] >> > [cannot apply to linus/master linux/master jeyu/modules-next v4.9-rc8 v4.9-rc7 v4.9-rc6 v4.11-rc4] >> > [if your patch is applied to the wrong git tree, please drop us a note to help improve the system] >> > >> > url: https://github.com/0day-ci/linux/commits/Eddie-Kovsky/module-verify-address-is-read-only/20170327-142922 >> > config: blackfin-BF561-EZKIT-SMP_defconfig (attached as .config) >> > compiler: bfin-uclinux-gcc (GCC) 6.2.0 >> > reproduce: >> > wget https://raw.githubusercontent.com/01org/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross >> > chmod +x ~/bin/make.cross >> > # save the attached .config to linux build tree >> > make.cross ARCH=blackfin >> > >> > All errors (new ones prefixed by >>): >> > >> > kernel/built-in.o: In function `core_kernel_rodata': >> >>> kernel/extable.c:169: undefined reference to `__start_data_ro_after_init' >> >>> kernel/extable.c:169: undefined reference to `__start_data_ro_after_init' >> >>> kernel/extable.c:169: undefined reference to `__end_data_ro_after_init' >> >>> kernel/extable.c:169: undefined reference to `__end_data_ro_after_init' >> >>> kernel/extable.c:169: undefined reference to `__start_data_ro_after_init' >> >>> kernel/extable.c:169: undefined reference to `__start_data_ro_after_init' >> >>> kernel/extable.c:169: undefined reference to `__end_data_ro_after_init' >> >>> kernel/extable.c:169: undefined reference to `__end_data_ro_after_init' >> >> Hm, I'm confused about this. blackfin includes >> include/asm-generic-vmlinux.lds.h and uses the RO_DATA macro (which >> resolves to RO_DATA_SECTION to RO_AFTER_INIT_DATA which defines >> __[start|end]_data_ro_after_init. >> >> Also, it seems that commit d7c19b066dcf4bd19c4385e8065558d4e74f9e73 >> ("mm: kmemleak: scan .data.ro_after_init") added a potentially >> redundant section name (s390 already calls this >> __[start|end]_ro_after_init). I'd like to get this cleaned up, since >> having multiple names for the same thing is confusing: >> >> diff --git a/arch/s390/kernel/vmlinux.lds.S b/arch/s390/kernel/vmlinux.lds.S >> index 000e6e91f6a0..3667d20e997f 100644 >> --- a/arch/s390/kernel/vmlinux.lds.S >> +++ b/arch/s390/kernel/vmlinux.lds.S >> @@ -62,9 +62,11 @@ SECTIONS >> >> . = ALIGN(PAGE_SIZE); >> __start_ro_after_init = .; >> + __start_data_ro_after_init = .; >> .data..ro_after_init : { >> *(.data..ro_after_init) >> } >> + __end_data_ro_after_init = .; >> EXCEPTION_TABLE(16) >> . = ALIGN(PAGE_SIZE); >> __end_ro_after_init = .; >> >> And it seems that this hunk is wrong (__end_ro_after_init includes >> s390's exception table, etc). I think we should remove the >> ..._data_... name and use s390's name. >> >> I'll send an adjustment patch, but we'll still need to deal with blackfin. >> >> -Kees >> > >Kees > >I applied your patch (mm: fix section name for .data..ro_after_init) and >changed the new function in extable.c to use __[start|end]_ro_after_init >instead. The new version still builds without errors on x86, which isn't >surprising. > >I've cross compiled this for blackfin and I'm able to reproduce the >build error. I'm still not sure why. As you pointed out, blackfin does >appear to use 'include/asm-generic/vmlinux.lds.h'. This appears to be because blackfin is one of the 2 arches that prepends an underscore '_' to all symbols defined in C. I noticed that __{start,end}_data_ro_after_init in vmlinux.lds.h are not wrapped with VMLINUX_SYMBOL() which adds the necessary prefix for arches that have HAVE_UNDERSCORE_SYMBOL_PREFIX, hence the undefined reference. The below patch fixed the build error for me, if it works for you then I can send a formal patch. diff --git a/include/asm-generic/vmlinux.lds.h b/include/asm-generic/vmlinux.lds.h index 4e09b28..7b262f7 100644 --- a/include/asm-generic/vmlinux.lds.h +++ b/include/asm-generic/vmlinux.lds.h @@ -260,9 +260,9 @@ */ #ifndef RO_AFTER_INIT_DATA #define RO_AFTER_INIT_DATA \ - __start_data_ro_after_init = .; \ + VMLINUX_SYMBOL(__start_data_ro_after_init) = .; \ *(.data..ro_after_init) \ - __end_data_ro_after_init = .; + VMLINUX_SYMBOL(__end_data_ro_after_init) = .; #endif /* Jessica