| Message ID | 20181206083257.9596-1-ard.biesheuvel@linaro.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [v3] ARM: smp: add support for per-task stack canaries | expand |
Hi Ard,
I love your patch! Perhaps something to improve:
[auto build test WARNING on arm/for-next]
[also build test WARNING on v4.20-rc5]
[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/Ard-Biesheuvel/ARM-smp-add-support-for-per-task-stack-canaries/20181209-033321
base: git://git.armlinux.org.uk/~rmk/linux-arm.git for-next
config: arm-allmodconfig (attached as .config)
compiler: arm-linux-gnueabi-gcc (Debian 7.2.0-11) 7.2.0
reproduce:
wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# save the attached .config to linux build tree
GCC_VERSION=7.2.0 make.cross ARCH=arm
All warnings (new ones prefixed by >>):
lib/test_ubsan.c: In function 'test_ubsan_vla_bound_not_positive':
>> lib/test_ubsan.c:48:2: warning: ISO C90 forbids variable length array 'buf' [-Wvla]
char buf[size];
^~~~
lib/test_ubsan.c: In function 'test_ubsan_out_of_bounds':
>> lib/test_ubsan.c:64:2: warning: ISO C90 forbids variable length array 'arr' [-Wvla]
volatile int arr[i];
^~~~~~~~
vim +/buf +48 lib/test_ubsan.c
854686f4 Jinbum Park 2018-04-10 44
854686f4 Jinbum Park 2018-04-10 45 static void test_ubsan_vla_bound_not_positive(void)
854686f4 Jinbum Park 2018-04-10 46 {
854686f4 Jinbum Park 2018-04-10 47 volatile int size = -1;
854686f4 Jinbum Park 2018-04-10 @48 char buf[size];
854686f4 Jinbum Park 2018-04-10 49
854686f4 Jinbum Park 2018-04-10 50 (void)buf;
854686f4 Jinbum Park 2018-04-10 51 }
854686f4 Jinbum Park 2018-04-10 52
854686f4 Jinbum Park 2018-04-10 53 static void test_ubsan_shift_out_of_bounds(void)
854686f4 Jinbum Park 2018-04-10 54 {
854686f4 Jinbum Park 2018-04-10 55 volatile int val = -1;
854686f4 Jinbum Park 2018-04-10 56 int val2 = 10;
854686f4 Jinbum Park 2018-04-10 57
854686f4 Jinbum Park 2018-04-10 58 val2 <<= val;
854686f4 Jinbum Park 2018-04-10 59 }
854686f4 Jinbum Park 2018-04-10 60
854686f4 Jinbum Park 2018-04-10 61 static void test_ubsan_out_of_bounds(void)
854686f4 Jinbum Park 2018-04-10 62 {
854686f4 Jinbum Park 2018-04-10 63 volatile int i = 4, j = 5;
854686f4 Jinbum Park 2018-04-10 @64 volatile int arr[i];
854686f4 Jinbum Park 2018-04-10 65
854686f4 Jinbum Park 2018-04-10 66 arr[j] = i;
854686f4 Jinbum Park 2018-04-10 67 }
854686f4 Jinbum Park 2018-04-10 68
:::::: The code at line 48 was first introduced by commit
:::::: 854686f4edf483db1e0d26d972bdb8fb65c8bfaa lib: add testing module for UBSAN
:::::: TO: Jinbum Park <jinb.park7@gmail.com>
:::::: CC: Linus Torvalds <torvalds@linux-foundation.org>
---
0-DAY kernel test infrastructure Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all Intel Corporation
On Sun, Dec 09, 2018 at 06:28:11PM +0800, kbuild test robot wrote: > Hi Ard, > > I love your patch! Perhaps something to improve: Hi, This looks to me like a false warning - how can a patch touching arch code affect the result of lib/test_ubsan.c? Please can you double- check this result? Thanks. > > [auto build test WARNING on arm/for-next] > [also build test WARNING on v4.20-rc5] > [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/Ard-Biesheuvel/ARM-smp-add-support-for-per-task-stack-canaries/20181209-033321 > base: git://git.armlinux.org.uk/~rmk/linux-arm.git for-next > config: arm-allmodconfig (attached as .config) > compiler: arm-linux-gnueabi-gcc (Debian 7.2.0-11) 7.2.0 > reproduce: > wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross > chmod +x ~/bin/make.cross > # save the attached .config to linux build tree > GCC_VERSION=7.2.0 make.cross ARCH=arm > > All warnings (new ones prefixed by >>): > > lib/test_ubsan.c: In function 'test_ubsan_vla_bound_not_positive': > >> lib/test_ubsan.c:48:2: warning: ISO C90 forbids variable length array 'buf' [-Wvla] > char buf[size]; > ^~~~ > lib/test_ubsan.c: In function 'test_ubsan_out_of_bounds': > >> lib/test_ubsan.c:64:2: warning: ISO C90 forbids variable length array 'arr' [-Wvla] > volatile int arr[i]; > ^~~~~~~~ > > vim +/buf +48 lib/test_ubsan.c > > 854686f4 Jinbum Park 2018-04-10 44 > 854686f4 Jinbum Park 2018-04-10 45 static void test_ubsan_vla_bound_not_positive(void) > 854686f4 Jinbum Park 2018-04-10 46 { > 854686f4 Jinbum Park 2018-04-10 47 volatile int size = -1; > 854686f4 Jinbum Park 2018-04-10 @48 char buf[size]; > 854686f4 Jinbum Park 2018-04-10 49 > 854686f4 Jinbum Park 2018-04-10 50 (void)buf; > 854686f4 Jinbum Park 2018-04-10 51 } > 854686f4 Jinbum Park 2018-04-10 52 > 854686f4 Jinbum Park 2018-04-10 53 static void test_ubsan_shift_out_of_bounds(void) > 854686f4 Jinbum Park 2018-04-10 54 { > 854686f4 Jinbum Park 2018-04-10 55 volatile int val = -1; > 854686f4 Jinbum Park 2018-04-10 56 int val2 = 10; > 854686f4 Jinbum Park 2018-04-10 57 > 854686f4 Jinbum Park 2018-04-10 58 val2 <<= val; > 854686f4 Jinbum Park 2018-04-10 59 } > 854686f4 Jinbum Park 2018-04-10 60 > 854686f4 Jinbum Park 2018-04-10 61 static void test_ubsan_out_of_bounds(void) > 854686f4 Jinbum Park 2018-04-10 62 { > 854686f4 Jinbum Park 2018-04-10 63 volatile int i = 4, j = 5; > 854686f4 Jinbum Park 2018-04-10 @64 volatile int arr[i]; > 854686f4 Jinbum Park 2018-04-10 65 > 854686f4 Jinbum Park 2018-04-10 66 arr[j] = i; > 854686f4 Jinbum Park 2018-04-10 67 } > 854686f4 Jinbum Park 2018-04-10 68 > > :::::: The code at line 48 was first introduced by commit > :::::: 854686f4edf483db1e0d26d972bdb8fb65c8bfaa lib: add testing module for UBSAN > > :::::: TO: Jinbum Park <jinb.park7@gmail.com> > :::::: CC: Linus Torvalds <torvalds@linux-foundation.org> > > --- > 0-DAY kernel test infrastructure Open Source Technology Center > https://lists.01.org/pipermail/kbuild-all Intel Corporation
On Sun, Dec 9, 2018 at 2:38 AM Russell King - ARM Linux <linux@armlinux.org.uk> wrote: > > On Sun, Dec 09, 2018 at 06:28:11PM +0800, kbuild test robot wrote: > > Hi Ard, > > > > I love your patch! Perhaps something to improve: > > Hi, > > This looks to me like a false warning - how can a patch touching arch > code affect the result of lib/test_ubsan.c? Please can you double- > check this result? Yeah, no idea. Do you want me to toss this into the ARM patch queue? If not, I can this through the gcc-plugin tree. -Kees
On Mon, Dec 10, 2018 at 2:34 PM Kees Cook <keescook@chromium.org> wrote: > > On Sun, Dec 9, 2018 at 2:38 AM Russell King - ARM Linux > <linux@armlinux.org.uk> wrote: > > > > On Sun, Dec 09, 2018 at 06:28:11PM +0800, kbuild test robot wrote: > > > Hi Ard, > > > > > > I love your patch! Perhaps something to improve: > > > > Hi, > > > > This looks to me like a false warning - how can a patch touching arch > > code affect the result of lib/test_ubsan.c? Please can you double- > > check this result? > > Yeah, no idea. Do you want me to toss this into the ARM patch queue? > If not, I can this through the gcc-plugin tree. Since the ARM changes are relatively small, and it adds a new plugin, I'll take this via the gcc-plugins tree for -next now. Please yell if this should be done differently. Thanks!
On 12/09/2018 06:37 PM, Russell King - ARM Linux wrote: > On Sun, Dec 09, 2018 at 06:28:11PM +0800, kbuild test robot wrote: >> Hi Ard, >> >> I love your patch! Perhaps something to improve: > Hi, > > This looks to me like a false warning - how can a patch touching arch > code affect the result of lib/test_ubsan.c? Please can you double- > check this result? Hi, I'm sorry for the trouble, It's a false warning, we will look into it. Best Regards, Rong Chen > Thanks. > >> [auto build test WARNING on arm/for-next] >> [also build test WARNING on v4.20-rc5] >> [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/Ard-Biesheuvel/ARM-smp-add-support-for-per-task-stack-canaries/20181209-033321 >> base: git://git.armlinux.org.uk/~rmk/linux-arm.git for-next >> config: arm-allmodconfig (attached as .config) >> compiler: arm-linux-gnueabi-gcc (Debian 7.2.0-11) 7.2.0 >> reproduce: >> wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross >> chmod +x ~/bin/make.cross >> # save the attached .config to linux build tree >> GCC_VERSION=7.2.0 make.cross ARCH=arm >> >> All warnings (new ones prefixed by >>): >> >> lib/test_ubsan.c: In function 'test_ubsan_vla_bound_not_positive': >>>> lib/test_ubsan.c:48:2: warning: ISO C90 forbids variable length array 'buf' [-Wvla] >> char buf[size]; >> ^~~~ >> lib/test_ubsan.c: In function 'test_ubsan_out_of_bounds': >>>> lib/test_ubsan.c:64:2: warning: ISO C90 forbids variable length array 'arr' [-Wvla] >> volatile int arr[i]; >> ^~~~~~~~ >> >> vim +/buf +48 lib/test_ubsan.c >> >> 854686f4 Jinbum Park 2018-04-10 44 >> 854686f4 Jinbum Park 2018-04-10 45 static void test_ubsan_vla_bound_not_positive(void) >> 854686f4 Jinbum Park 2018-04-10 46 { >> 854686f4 Jinbum Park 2018-04-10 47 volatile int size = -1; >> 854686f4 Jinbum Park 2018-04-10 @48 char buf[size]; >> 854686f4 Jinbum Park 2018-04-10 49 >> 854686f4 Jinbum Park 2018-04-10 50 (void)buf; >> 854686f4 Jinbum Park 2018-04-10 51 } >> 854686f4 Jinbum Park 2018-04-10 52 >> 854686f4 Jinbum Park 2018-04-10 53 static void test_ubsan_shift_out_of_bounds(void) >> 854686f4 Jinbum Park 2018-04-10 54 { >> 854686f4 Jinbum Park 2018-04-10 55 volatile int val = -1; >> 854686f4 Jinbum Park 2018-04-10 56 int val2 = 10; >> 854686f4 Jinbum Park 2018-04-10 57 >> 854686f4 Jinbum Park 2018-04-10 58 val2 <<= val; >> 854686f4 Jinbum Park 2018-04-10 59 } >> 854686f4 Jinbum Park 2018-04-10 60 >> 854686f4 Jinbum Park 2018-04-10 61 static void test_ubsan_out_of_bounds(void) >> 854686f4 Jinbum Park 2018-04-10 62 { >> 854686f4 Jinbum Park 2018-04-10 63 volatile int i = 4, j = 5; >> 854686f4 Jinbum Park 2018-04-10 @64 volatile int arr[i]; >> 854686f4 Jinbum Park 2018-04-10 65 >> 854686f4 Jinbum Park 2018-04-10 66 arr[j] = i; >> 854686f4 Jinbum Park 2018-04-10 67 } >> 854686f4 Jinbum Park 2018-04-10 68 >> >> :::::: The code at line 48 was first introduced by commit >> :::::: 854686f4edf483db1e0d26d972bdb8fb65c8bfaa lib: add testing module for UBSAN >> >> :::::: TO: Jinbum Park <jinb.park7@gmail.com> >> :::::: CC: Linus Torvalds <torvalds@linux-foundation.org> >> >> --- >> 0-DAY kernel test infrastructure Open Source Technology Center >> https://lists.01.org/pipermail/kbuild-all Intel Corporation > >
On 12/13/2018 10:53 AM, Rong Chen wrote: > > > On 12/09/2018 06:37 PM, Russell King - ARM Linux wrote: >> On Sun, Dec 09, 2018 at 06:28:11PM +0800, kbuild test robot wrote: >>> Hi Ard, >>> >>> I love your patch! Perhaps something to improve: >> Hi, >> >> This looks to me like a false warning - how can a patch touching arch >> code affect the result of lib/test_ubsan.c? Please can you double- >> check this result? > > Hi, > > I'm sorry for the trouble, It's a false warning, we will look into it. > > Best Regards, > Rong Chen > Hi all, I tested the commit on another machine and the warnings are exist. I found "-Wno-vla" was lost after the commit, so I think it's a real problem. -Werror=designated-init -fplugin-arg-arm_ssp_per_task_plugin-tso=1 -fplugin-arg-arm_ssp_per_task_plugin-offset=24 -fsanitize=shift-fsanitize=integer-divide-by-zero -fsanitize=unreachable -fsanitize=vla-bound -fsanitize=signed-integer-overflow -fsanitize=bounds -fsanitize=object-size -fsanitize=bool -fsanitize=enum -fsanitize=alignment -Wno-maybe-uninitialized -fsanitize-coverage=trace-pc -DMODULE -DKBUILD_BASENAME='"test_ubsan"' -DKBUILD_MODNAME='"test_ubsan"' -c -o lib/.tmp_test_ubsan.o lib/test_ubsan.c -Werror=designated-init -Wno-vla -fsanitize=shift-fsanitize=integer-divide-by-zero -fsanitize=unreachable -fsanitize=vla-bound -fsanitize=signed-integer-overflow -fsanitize=bounds -fsanitize=object-size -fsanitize=bool -fsanitize=enum -fsanitize=alignment -Wno-maybe-uninitialized -fsanitize-coverage=trace-pc -DMODULE -DKBUILD_BASENAME='"test_ubsan"' -DKBUILD_MODNAME='"test_ubsan"' -c -o lib/.tmp_test_ubsan.o lib/test_ubsan.c Best Regards, Rong Chen >> Thanks. >> >>> [auto build test WARNING on arm/for-next] >>> [also build test WARNING on v4.20-rc5] >>> [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/Ard-Biesheuvel/ARM-smp-add-support-for-per-task-stack-canaries/20181209-033321 >>> base: git://git.armlinux.org.uk/~rmk/linux-arm.git for-next >>> config: arm-allmodconfig (attached as .config) >>> compiler: arm-linux-gnueabi-gcc (Debian 7.2.0-11) 7.2.0 >>> reproduce: >>> wget >>> https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross >>> -O ~/bin/make.cross >>> chmod +x ~/bin/make.cross >>> # save the attached .config to linux build tree >>> GCC_VERSION=7.2.0 make.cross ARCH=arm >>> >>> All warnings (new ones prefixed by >>): >>> >>> lib/test_ubsan.c: In function 'test_ubsan_vla_bound_not_positive': >>>>> lib/test_ubsan.c:48:2: warning: ISO C90 forbids variable length >>>>> array 'buf' [-Wvla] >>> char buf[size]; >>> ^~~~ >>> lib/test_ubsan.c: In function 'test_ubsan_out_of_bounds': >>>>> lib/test_ubsan.c:64:2: warning: ISO C90 forbids variable length >>>>> array 'arr' [-Wvla] >>> volatile int arr[i]; >>> ^~~~~~~~ >>> >>> vim +/buf +48 lib/test_ubsan.c >>> >>> 854686f4 Jinbum Park 2018-04-10 44 >>> 854686f4 Jinbum Park 2018-04-10 45 static void >>> test_ubsan_vla_bound_not_positive(void) >>> 854686f4 Jinbum Park 2018-04-10 46 { >>> 854686f4 Jinbum Park 2018-04-10 47 volatile int size = -1; >>> 854686f4 Jinbum Park 2018-04-10 @48 char buf[size]; >>> 854686f4 Jinbum Park 2018-04-10 49 >>> 854686f4 Jinbum Park 2018-04-10 50 (void)buf; >>> 854686f4 Jinbum Park 2018-04-10 51 } >>> 854686f4 Jinbum Park 2018-04-10 52 >>> 854686f4 Jinbum Park 2018-04-10 53 static void >>> test_ubsan_shift_out_of_bounds(void) >>> 854686f4 Jinbum Park 2018-04-10 54 { >>> 854686f4 Jinbum Park 2018-04-10 55 volatile int val = -1; >>> 854686f4 Jinbum Park 2018-04-10 56 int val2 = 10; >>> 854686f4 Jinbum Park 2018-04-10 57 >>> 854686f4 Jinbum Park 2018-04-10 58 val2 <<= val; >>> 854686f4 Jinbum Park 2018-04-10 59 } >>> 854686f4 Jinbum Park 2018-04-10 60 >>> 854686f4 Jinbum Park 2018-04-10 61 static void >>> test_ubsan_out_of_bounds(void) >>> 854686f4 Jinbum Park 2018-04-10 62 { >>> 854686f4 Jinbum Park 2018-04-10 63 volatile int i = 4, j = 5; >>> 854686f4 Jinbum Park 2018-04-10 @64 volatile int arr[i]; >>> 854686f4 Jinbum Park 2018-04-10 65 >>> 854686f4 Jinbum Park 2018-04-10 66 arr[j] = i; >>> 854686f4 Jinbum Park 2018-04-10 67 } >>> 854686f4 Jinbum Park 2018-04-10 68 >>> >>> :::::: The code at line 48 was first introduced by commit >>> :::::: 854686f4edf483db1e0d26d972bdb8fb65c8bfaa lib: add testing >>> module for UBSAN >>> >>> :::::: TO: Jinbum Park <jinb.park7@gmail.com> >>> :::::: CC: Linus Torvalds <torvalds@linux-foundation.org> >>> >>> --- >>> 0-DAY kernel test infrastructure Open Source >>> Technology Center >>> https://lists.01.org/pipermail/kbuild-all Intel Corporation >> >> > > _______________________________________________ > kbuild-all mailing list > kbuild-all@lists.01.org > https://lists.01.org/mailman/listinfo/kbuild-all <html> <head> <meta http-equiv="Content-Type" content="text/html; charset=utf-8"> </head> <body text="#000000" bgcolor="#FFFFFF"> <p><br> </p> <br> <div class="moz-cite-prefix">On 12/13/2018 10:53 AM, Rong Chen wrote:<br> </div> <blockquote type="cite" cite="mid:47a91e5a-0e97-334c-2f36-a0bd966a0050@intel.com"> <br> <br> On 12/09/2018 06:37 PM, Russell King - ARM Linux wrote: <br> <blockquote type="cite">On Sun, Dec 09, 2018 at 06:28:11PM +0800, kbuild test robot wrote: <br> <blockquote type="cite">Hi Ard, <br> <br> I love your patch! Perhaps something to improve: <br> </blockquote> Hi, <br> <br> This looks to me like a false warning - how can a patch touching arch <br> code affect the result of lib/test_ubsan.c? Please can you double- <br> check this result? <br> </blockquote> <br> Hi, <br> <br> I'm sorry for the trouble, It's a false warning, we will look into it. <br> <br> Best Regards, <br> Rong Chen <br> <br> </blockquote> <br> Hi all,<br> <br> I tested the commit on another machine and the warnings are exist.<br> I found "<span style="color: rgb(255, 0, 0); font-family: -apple-system, BlinkMacSystemFont, "Segoe UI", Roboto, "Noto Sans", Ubuntu, "Droid Sans", "Helvetica Neue", sans-serif; font-size: 14px; font-style: normal; font-variant-ligatures: normal; font-variant-caps: normal; font-weight: 400; letter-spacing: normal; orphans: 2; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: 2; word-spacing: 0px; -webkit-text-stroke-width: 0px; background-color: rgb(245, 245, 245); text-decoration-style: initial; text-decoration-color: initial; display: inline !important; float: none;"><span> </span>-Wno-vla</span>" was lost after the commit, so I think it's a real problem.<br> <br> <font style="outline: none !important; font-family: -apple-system, BlinkMacSystemFont, "Segoe UI", Roboto, "Noto Sans", Ubuntu, "Droid Sans", "Helvetica Neue", sans-serif; font-size: 14px; font-style: normal; font-variant-ligatures: normal; font-variant-caps: normal; font-weight: 400; letter-spacing: normal; orphans: 2; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: 2; word-spacing: 0px; -webkit-text-stroke-width: 0px; background-color: rgb(245, 245, 245); text-decoration-style: initial; text-decoration-color: initial;" color="#FF0000">-Werror=designated-init -fplugin-arg-arm_ssp_per_task_plugin-tso=1 -fplugin-arg-arm_ssp_per_task_plugin-offset=24 -fsanitize=shift<span> </span></font><span style="color: rgb(51, 51, 51); font-family: -apple-system, BlinkMacSystemFont, "Segoe UI", Roboto, "Noto Sans", Ubuntu, "Droid Sans", "Helvetica Neue", sans-serif; font-size: 14px; font-style: normal; font-variant-ligatures: normal; font-variant-caps: normal; font-weight: 400; letter-spacing: normal; orphans: 2; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: 2; word-spacing: 0px; -webkit-text-stroke-width: 0px; background-color: rgb(245, 245, 245); text-decoration-style: initial; text-decoration-color: initial; display: inline !important; float: none;">-fsanitize=integer-divide-by-zero -fsanitize=unreachable -fsanitize=vla-bound -fsanitize=signed-integer-overflow -fsanitize=bounds -fsanitize=object-size -fsanitize=bool -fsanitize=enum -fsanitize=alignment -Wno-maybe-uninitialized -fsanitize-coverage=trace-pc -DMODULE -DKBUILD_BASENAME='"test_ubsan"' -DKBUILD_MODNAME='"test_ubsan"' -c -o lib/.tmp_test_ubsan.o lib/test_ubsan.c</span><br> <br> <font style="outline: none !important; font-family: -apple-system, BlinkMacSystemFont, "Segoe UI", Roboto, "Noto Sans", Ubuntu, "Droid Sans", "Helvetica Neue", sans-serif; font-size: 14px; font-style: normal; font-variant-ligatures: normal; font-variant-caps: normal; font-weight: 400; letter-spacing: normal; orphans: 2; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: 2; word-spacing: 0px; -webkit-text-stroke-width: 0px; background-color: rgb(245, 245, 245); text-decoration-style: initial; text-decoration-color: initial;" color="#FF0000">-Werror=designated-init -Wno-vla -fsanitize=shift</font><span style="color: rgb(51, 51, 51); font-family: -apple-system, BlinkMacSystemFont, "Segoe UI", Roboto, "Noto Sans", Ubuntu, "Droid Sans", "Helvetica Neue", sans-serif; font-size: 14px; font-style: normal; font-variant-ligatures: normal; font-variant-caps: normal; font-weight: 400; letter-spacing: normal; orphans: 2; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: 2; word-spacing: 0px; -webkit-text-stroke-width: 0px; background-color: rgb(245, 245, 245); text-decoration-style: initial; text-decoration-color: initial; display: inline !important; float: none;"><span> </span>-fsanitize=integer-divide-by-zero -fsanitize=unreachable -fsanitize=vla-bound -fsanitize=signed-integer-overflow -fsanitize=bounds -fsanitize=object-size -fsanitize=bool -fsanitize=enum -fsanitize=alignment -Wno-maybe-uninitialized -fsanitize-coverage=trace-pc -DMODULE -DKBUILD_BASENAME='"test_ubsan"' -DKBUILD_MODNAME='"test_ubsan"' -c -o lib/.tmp_test_ubsan.o lib/test_ubsan.c</span><br> <br> Best Regards,<br> Rong Chen<br> <br> <blockquote type="cite" cite="mid:47a91e5a-0e97-334c-2f36-a0bd966a0050@intel.com"> <blockquote type="cite">Thanks. <br> <br> <blockquote type="cite">[auto build test WARNING on arm/for-next] <br> [also build test WARNING on v4.20-rc5] <br> [if your patch is applied to the wrong git tree, please drop us a note to help improve the system] <br> <br> url: <a class="moz-txt-link-freetext" href="https://github.com/0day-ci/linux/commits/Ard-Biesheuvel/ARM-smp-add-support-for-per-task-stack-canaries/20181209-033321">https://github.com/0day-ci/linux/commits/Ard-Biesheuvel/ARM-smp-add-support-for-per-task-stack-canaries/20181209-033321</a><br> base: git://git.armlinux.org.uk/~rmk/linux-arm.git for-next <br> config: arm-allmodconfig (attached as .config) <br> compiler: arm-linux-gnueabi-gcc (Debian 7.2.0-11) 7.2.0 <br> reproduce: <br> wget <a class="moz-txt-link-freetext" href="https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross">https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross</a> -O ~/bin/make.cross <br> chmod +x ~/bin/make.cross <br> # save the attached .config to linux build tree <br> GCC_VERSION=7.2.0 make.cross ARCH=arm <br> <br> All warnings (new ones prefixed by >>): <br> <br> lib/test_ubsan.c: In function 'test_ubsan_vla_bound_not_positive': <br> <blockquote type="cite"> <blockquote type="cite">lib/test_ubsan.c:48:2: warning: ISO C90 forbids variable length array 'buf' [-Wvla] <br> </blockquote> </blockquote> char buf[size]; <br> ^~~~ <br> lib/test_ubsan.c: In function 'test_ubsan_out_of_bounds': <br> <blockquote type="cite"> <blockquote type="cite">lib/test_ubsan.c:64:2: warning: ISO C90 forbids variable length array 'arr' [-Wvla] <br> </blockquote> </blockquote> volatile int arr[i]; <br> ^~~~~~~~ <br> <br> vim +/buf +48 lib/test_ubsan.c <br> <br> 854686f4 Jinbum Park 2018-04-10 44 <br> 854686f4 Jinbum Park 2018-04-10 45 static void test_ubsan_vla_bound_not_positive(void) <br> 854686f4 Jinbum Park 2018-04-10 46 { <br> 854686f4 Jinbum Park 2018-04-10 47 volatile int size = -1; <br> 854686f4 Jinbum Park 2018-04-10 @48 char buf[size]; <br> 854686f4 Jinbum Park 2018-04-10 49 <br> 854686f4 Jinbum Park 2018-04-10 50 (void)buf; <br> 854686f4 Jinbum Park 2018-04-10 51 } <br> 854686f4 Jinbum Park 2018-04-10 52 <br> 854686f4 Jinbum Park 2018-04-10 53 static void test_ubsan_shift_out_of_bounds(void) <br> 854686f4 Jinbum Park 2018-04-10 54 { <br> 854686f4 Jinbum Park 2018-04-10 55 volatile int val = -1; <br> 854686f4 Jinbum Park 2018-04-10 56 int val2 = 10; <br> 854686f4 Jinbum Park 2018-04-10 57 <br> 854686f4 Jinbum Park 2018-04-10 58 val2 <<= val; <br> 854686f4 Jinbum Park 2018-04-10 59 } <br> 854686f4 Jinbum Park 2018-04-10 60 <br> 854686f4 Jinbum Park 2018-04-10 61 static void test_ubsan_out_of_bounds(void) <br> 854686f4 Jinbum Park 2018-04-10 62 { <br> 854686f4 Jinbum Park 2018-04-10 63 volatile int i = 4, j = 5; <br> 854686f4 Jinbum Park 2018-04-10 @64 volatile int arr[i]; <br> 854686f4 Jinbum Park 2018-04-10 65 <br> 854686f4 Jinbum Park 2018-04-10 66 arr[j] = i; <br> 854686f4 Jinbum Park 2018-04-10 67 } <br> 854686f4 Jinbum Park 2018-04-10 68 <br> <br> :::::: The code at line 48 was first introduced by commit <br> :::::: 854686f4edf483db1e0d26d972bdb8fb65c8bfaa lib: add testing module for UBSAN <br> <br> :::::: TO: Jinbum Park <a class="moz-txt-link-rfc2396E" href="mailto:jinb.park7@gmail.com"><jinb.park7@gmail.com></a> <br> :::::: CC: Linus Torvalds <a class="moz-txt-link-rfc2396E" href="mailto:torvalds@linux-foundation.org"><torvalds@linux-foundation.org></a> <br> <br> --- <br> 0-DAY kernel test infrastructure Open Source Technology Center <br> <a class="moz-txt-link-freetext" href="https://lists.01.org/pipermail/kbuild-all">https://lists.01.org/pipermail/kbuild-all</a> Intel Corporation <br> </blockquote> <br> <br> </blockquote> <br> _______________________________________________ <br> kbuild-all mailing list <br> <a class="moz-txt-link-abbreviated" href="mailto:kbuild-all@lists.01.org">kbuild-all@lists.01.org</a> <br> <a class="moz-txt-link-freetext" href="https://lists.01.org/mailman/listinfo/kbuild-all">https://lists.01.org/mailman/listinfo/kbuild-all</a> <br> </blockquote> <br> </body> </html>
On Tue, Jan 8, 2019 at 10:30 PM Rong Chen <rong.a.chen@intel.com> wrote: > I tested the commit on another machine and the warnings are exist. > I found " -Wno-vla" was lost after the commit, so I think it's a real problem. > > -Werror=designated-init -fplugin-arg-arm_ssp_per_task_plugin-tso=1 -fplugin-arg-arm_ssp_per_task_plugin-offset=24 -fsanitize=shift -fsanitize=integer-divide-by-zero -fsanitize=unreachable -fsanitize=vla-bound -fsanitize=signed-integer-overflow -fsanitize=bounds -fsanitize=object-size -fsanitize=bool -fsanitize=enum -fsanitize=alignment -Wno-maybe-uninitialized -fsanitize-coverage=trace-pc -DMODULE -DKBUILD_BASENAME='"test_ubsan"' -DKBUILD_MODNAME='"test_ubsan"' -c -o lib/.tmp_test_ubsan.o lib/test_ubsan.c > > -Werror=designated-init -Wno-vla -fsanitize=shift -fsanitize=integer-divide-by-zero -fsanitize=unreachable -fsanitize=vla-bound -fsanitize=signed-integer-overflow -fsanitize=bounds -fsanitize=object-size -fsanitize=bool -fsanitize=enum -fsanitize=alignment -Wno-maybe-uninitialized -fsanitize-coverage=trace-pc -DMODULE -DKBUILD_BASENAME='"test_ubsan"' -DKBUILD_MODNAME='"test_ubsan"' -c -o lib/.tmp_test_ubsan.o lib/test_ubsan.c Hm, so the -Wno-vla is getting clobbered. in lib/Makefile, I see "+=", so I wouldn't expect a clobber: CFLAGS_test_kasan.o += $(call cc-disable-warning, vla) ... CFLAGS_test_ubsan.o += $(call cc-disable-warning, vla) and scripts/Makefile.gcc-plugins is using += against KBUILD_CFLAGS: KBUILD_CFLAGS += $(GCC_PLUGINS_CFLAGS) I'm not sure where the per-target CFLAGS are built, but it seems like something has gone wrong there?
On Thu, Dec 06, 2018 at 09:32:57AM +0100, Ard Biesheuvel wrote: > On ARM, we currently only change the value of the stack canary when > switching tasks if the kernel was built for UP. On SMP kernels, this > is impossible since the stack canary value is obtained via a global > symbol reference, which means > a) all running tasks on all CPUs must use the same value > b) we can only modify the value when no kernel stack frames are live > on any CPU, which is effectively never. > > So instead, use a GCC plugin to add a RTL pass that replaces each > reference to the address of the __stack_chk_guard symbol with an > expression that produces the address of the 'stack_canary' field > that is added to struct thread_info. This way, each task will use > its own randomized value. > > Cc: Russell King <linux@armlinux.org.uk> > Cc: Kees Cook <keescook@chromium.org> > Cc: Emese Revfy <re.emese@gmail.com> > Cc: Arnd Bergmann <arnd@arndb.de> > Cc: Laura Abbott <labbott@redhat.com> > Cc: kernel-hardening@lists.openwall.com > Acked-by: Nicolas Pitre <nico@linaro.org> > Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org> Since this patch is in the tree, cc-option no longer works on the arm architecture if CONFIG_STACKPROTECTOR_PER_TASK is enabled. Any idea how to fix that ? Thanks, Guenter
On Mon, Mar 09, 2020 at 09:49:31AM -0700, Guenter Roeck wrote: > On Thu, Dec 06, 2018 at 09:32:57AM +0100, Ard Biesheuvel wrote: > > On ARM, we currently only change the value of the stack canary when > > switching tasks if the kernel was built for UP. On SMP kernels, this > > is impossible since the stack canary value is obtained via a global > > symbol reference, which means > > a) all running tasks on all CPUs must use the same value > > b) we can only modify the value when no kernel stack frames are live > > on any CPU, which is effectively never. > > > > So instead, use a GCC plugin to add a RTL pass that replaces each > > reference to the address of the __stack_chk_guard symbol with an > > expression that produces the address of the 'stack_canary' field > > that is added to struct thread_info. This way, each task will use > > its own randomized value. > > > > Cc: Russell King <linux@armlinux.org.uk> > > Cc: Kees Cook <keescook@chromium.org> > > Cc: Emese Revfy <re.emese@gmail.com> > > Cc: Arnd Bergmann <arnd@arndb.de> > > Cc: Laura Abbott <labbott@redhat.com> > > Cc: kernel-hardening@lists.openwall.com > > Acked-by: Nicolas Pitre <nico@linaro.org> > > Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org> > > Since this patch is in the tree, cc-option no longer works on > the arm architecture if CONFIG_STACKPROTECTOR_PER_TASK is enabled. > > Any idea how to fix that ? I thought Arnd sent a patch to fix it and it got picked up?
On 3/11/20 10:21 AM, Kees Cook wrote: > On Mon, Mar 09, 2020 at 09:49:31AM -0700, Guenter Roeck wrote: >> On Thu, Dec 06, 2018 at 09:32:57AM +0100, Ard Biesheuvel wrote: >>> On ARM, we currently only change the value of the stack canary when >>> switching tasks if the kernel was built for UP. On SMP kernels, this >>> is impossible since the stack canary value is obtained via a global >>> symbol reference, which means >>> a) all running tasks on all CPUs must use the same value >>> b) we can only modify the value when no kernel stack frames are live >>> on any CPU, which is effectively never. >>> >>> So instead, use a GCC plugin to add a RTL pass that replaces each >>> reference to the address of the __stack_chk_guard symbol with an >>> expression that produces the address of the 'stack_canary' field >>> that is added to struct thread_info. This way, each task will use >>> its own randomized value. >>> >>> Cc: Russell King <linux@armlinux.org.uk> >>> Cc: Kees Cook <keescook@chromium.org> >>> Cc: Emese Revfy <re.emese@gmail.com> >>> Cc: Arnd Bergmann <arnd@arndb.de> >>> Cc: Laura Abbott <labbott@redhat.com> >>> Cc: kernel-hardening@lists.openwall.com >>> Acked-by: Nicolas Pitre <nico@linaro.org> >>> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org> >> >> Since this patch is in the tree, cc-option no longer works on >> the arm architecture if CONFIG_STACKPROTECTOR_PER_TASK is enabled. >> >> Any idea how to fix that ? > > I thought Arnd sent a patch to fix it and it got picked up? > Yes, but the fix is not upstream (it is only in -next), and I missed it. Guenter
On Wed, Mar 11, 2020 at 11:31:13AM -0700, Guenter Roeck wrote: > On 3/11/20 10:21 AM, Kees Cook wrote: > > On Mon, Mar 09, 2020 at 09:49:31AM -0700, Guenter Roeck wrote: > >> On Thu, Dec 06, 2018 at 09:32:57AM +0100, Ard Biesheuvel wrote: > >>> On ARM, we currently only change the value of the stack canary when > >>> switching tasks if the kernel was built for UP. On SMP kernels, this > >>> is impossible since the stack canary value is obtained via a global > >>> symbol reference, which means > >>> a) all running tasks on all CPUs must use the same value > >>> b) we can only modify the value when no kernel stack frames are live > >>> on any CPU, which is effectively never. > >>> > >>> So instead, use a GCC plugin to add a RTL pass that replaces each > >>> reference to the address of the __stack_chk_guard symbol with an > >>> expression that produces the address of the 'stack_canary' field > >>> that is added to struct thread_info. This way, each task will use > >>> its own randomized value. > >>> > >>> Cc: Russell King <linux@armlinux.org.uk> > >>> Cc: Kees Cook <keescook@chromium.org> > >>> Cc: Emese Revfy <re.emese@gmail.com> > >>> Cc: Arnd Bergmann <arnd@arndb.de> > >>> Cc: Laura Abbott <labbott@redhat.com> > >>> Cc: kernel-hardening@lists.openwall.com > >>> Acked-by: Nicolas Pitre <nico@linaro.org> > >>> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org> > >> > >> Since this patch is in the tree, cc-option no longer works on > >> the arm architecture if CONFIG_STACKPROTECTOR_PER_TASK is enabled. > >> > >> Any idea how to fix that ? > > > > I thought Arnd sent a patch to fix it and it got picked up? > > > > Yes, but the fix is not upstream (it is only in -next), and I missed it. Ah, yes, I found it again now too; it went through rmk's tree. For thread posterity: ARM: 8961/2: Fix Kbuild issue caused by per-task stack protector GCC plugin https://www.arm.linux.org.uk/developer/patches/viewpatch.php?id=8961/2
On Wed, Mar 11, 2020 at 11:47:20AM -0700, Kees Cook wrote: > On Wed, Mar 11, 2020 at 11:31:13AM -0700, Guenter Roeck wrote: > > On 3/11/20 10:21 AM, Kees Cook wrote: > > > On Mon, Mar 09, 2020 at 09:49:31AM -0700, Guenter Roeck wrote: > > >> On Thu, Dec 06, 2018 at 09:32:57AM +0100, Ard Biesheuvel wrote: > > >>> On ARM, we currently only change the value of the stack canary when > > >>> switching tasks if the kernel was built for UP. On SMP kernels, this > > >>> is impossible since the stack canary value is obtained via a global > > >>> symbol reference, which means > > >>> a) all running tasks on all CPUs must use the same value > > >>> b) we can only modify the value when no kernel stack frames are live > > >>> on any CPU, which is effectively never. > > >>> > > >>> So instead, use a GCC plugin to add a RTL pass that replaces each > > >>> reference to the address of the __stack_chk_guard symbol with an > > >>> expression that produces the address of the 'stack_canary' field > > >>> that is added to struct thread_info. This way, each task will use > > >>> its own randomized value. > > >>> > > >>> Cc: Russell King <linux@armlinux.org.uk> > > >>> Cc: Kees Cook <keescook@chromium.org> > > >>> Cc: Emese Revfy <re.emese@gmail.com> > > >>> Cc: Arnd Bergmann <arnd@arndb.de> > > >>> Cc: Laura Abbott <labbott@redhat.com> > > >>> Cc: kernel-hardening@lists.openwall.com > > >>> Acked-by: Nicolas Pitre <nico@linaro.org> > > >>> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org> > > >> > > >> Since this patch is in the tree, cc-option no longer works on > > >> the arm architecture if CONFIG_STACKPROTECTOR_PER_TASK is enabled. > > >> > > >> Any idea how to fix that ? > > > > > > I thought Arnd sent a patch to fix it and it got picked up? > > > > > > > Yes, but the fix is not upstream (it is only in -next), and I missed it. > > Ah, yes, I found it again now too; it went through rmk's tree. > > For thread posterity: > > ARM: 8961/2: Fix Kbuild issue caused by per-task stack protector GCC plugin > https://www.arm.linux.org.uk/developer/patches/viewpatch.php?id=8961/2 It's in my fixes branch, waiting for me to do my (now usual) push of fixes to Linus. I'm not sure whether the above discussion is suggesting that there's a problem with this patch, or whether it's trying to encourage me to send it up to Linus. I _think_ there's the suggestion that it causes a regression, hmm?
On 3/11/20 1:45 PM, Russell King - ARM Linux admin wrote: > On Wed, Mar 11, 2020 at 11:47:20AM -0700, Kees Cook wrote: >> On Wed, Mar 11, 2020 at 11:31:13AM -0700, Guenter Roeck wrote: >>> On 3/11/20 10:21 AM, Kees Cook wrote: >>>> On Mon, Mar 09, 2020 at 09:49:31AM -0700, Guenter Roeck wrote: >>>>> On Thu, Dec 06, 2018 at 09:32:57AM +0100, Ard Biesheuvel wrote: >>>>>> On ARM, we currently only change the value of the stack canary when >>>>>> switching tasks if the kernel was built for UP. On SMP kernels, this >>>>>> is impossible since the stack canary value is obtained via a global >>>>>> symbol reference, which means >>>>>> a) all running tasks on all CPUs must use the same value >>>>>> b) we can only modify the value when no kernel stack frames are live >>>>>> on any CPU, which is effectively never. >>>>>> >>>>>> So instead, use a GCC plugin to add a RTL pass that replaces each >>>>>> reference to the address of the __stack_chk_guard symbol with an >>>>>> expression that produces the address of the 'stack_canary' field >>>>>> that is added to struct thread_info. This way, each task will use >>>>>> its own randomized value. >>>>>> >>>>>> Cc: Russell King <linux@armlinux.org.uk> >>>>>> Cc: Kees Cook <keescook@chromium.org> >>>>>> Cc: Emese Revfy <re.emese@gmail.com> >>>>>> Cc: Arnd Bergmann <arnd@arndb.de> >>>>>> Cc: Laura Abbott <labbott@redhat.com> >>>>>> Cc: kernel-hardening@lists.openwall.com >>>>>> Acked-by: Nicolas Pitre <nico@linaro.org> >>>>>> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org> >>>>> >>>>> Since this patch is in the tree, cc-option no longer works on >>>>> the arm architecture if CONFIG_STACKPROTECTOR_PER_TASK is enabled. >>>>> >>>>> Any idea how to fix that ? >>>> >>>> I thought Arnd sent a patch to fix it and it got picked up? >>>> >>> >>> Yes, but the fix is not upstream (it is only in -next), and I missed it. >> >> Ah, yes, I found it again now too; it went through rmk's tree. >> >> For thread posterity: >> >> ARM: 8961/2: Fix Kbuild issue caused by per-task stack protector GCC plugin >> https://www.arm.linux.org.uk/developer/patches/viewpatch.php?id=8961/2 > > It's in my fixes branch, waiting for me to do my (now usual) push > of fixes to Linus. > > I'm not sure whether the above discussion is suggesting that there's > a problem with this patch, or whether it's trying to encourage me > to send it up to Linus. I _think_ there's the suggestion that it > causes a regression, hmm? > ARM: 8961/2: Fix Kbuild issue caused by per-task stack protector GCC plugin Commit 189af4657186da08 ("ARM: smp: add support for per-task stack canaries") caused a regression. "ARM: 8961/2: Fix Kbuild issue caused by per-task stack protector GCC plugin" fixes it. Guenter
diff --git a/arch/arm/Kconfig b/arch/arm/Kconfig index 91be74d8df65..5c0305585a0a 100644 --- a/arch/arm/Kconfig +++ b/arch/arm/Kconfig @@ -1810,6 +1810,21 @@ config XEN help Say Y if you want to run Linux in a Virtual Machine on Xen on ARM. +config STACKPROTECTOR_PER_TASK + bool "Use a unique stack canary value for each task" + depends on GCC_PLUGINS && STACKPROTECTOR && SMP && !XIP_DEFLATED_DATA + select GCC_PLUGIN_ARM_SSP_PER_TASK + default y + help + Due to the fact that GCC uses an ordinary symbol reference from + which to load the value of the stack canary, this value can only + change at reboot time on SMP systems, and all tasks running in the + kernel's address space are forced to use the same canary value for + the entire duration that the system is up. + + Enable this option to switch to a different method that uses a + different canary value for each task. + endmenu menu "Boot options" diff --git a/arch/arm/Makefile b/arch/arm/Makefile index 05a91d8b89f3..0436002d5091 100644 --- a/arch/arm/Makefile +++ b/arch/arm/Makefile @@ -303,6 +303,18 @@ else KBUILD_IMAGE := $(boot)/zImage endif +ifeq ($(CONFIG_STACKPROTECTOR_PER_TASK),y) +prepare: stack_protector_prepare +stack_protector_prepare: prepare0 + $(eval KBUILD_CFLAGS += \ + -fplugin-arg-arm_ssp_per_task_plugin-tso=$(shell \ + awk '{if ($$2 == "THREAD_SZ_ORDER") print $$3;}'\ + include/generated/asm-offsets.h) \ + -fplugin-arg-arm_ssp_per_task_plugin-offset=$(shell \ + awk '{if ($$2 == "TI_STACK_CANARY") print $$3;}'\ + include/generated/asm-offsets.h)) +endif + all: $(notdir $(KBUILD_IMAGE)) diff --git a/arch/arm/boot/compressed/Makefile b/arch/arm/boot/compressed/Makefile index 1f5a5ffe7fcf..01bf2585a0fa 100644 --- a/arch/arm/boot/compressed/Makefile +++ b/arch/arm/boot/compressed/Makefile @@ -101,6 +101,7 @@ clean-files += piggy_data lib1funcs.S ashldi3.S bswapsdi2.S \ $(libfdt) $(libfdt_hdrs) hyp-stub.S KBUILD_CFLAGS += -DDISABLE_BRANCH_PROFILING +KBUILD_CFLAGS += $(DISABLE_ARM_SSP_PER_TASK_PLUGIN) ifeq ($(CONFIG_FUNCTION_TRACER),y) ORIG_CFLAGS := $(KBUILD_CFLAGS) diff --git a/arch/arm/include/asm/stackprotector.h b/arch/arm/include/asm/stackprotector.h index ef5f7b69443e..72a20c3a0a90 100644 --- a/arch/arm/include/asm/stackprotector.h +++ b/arch/arm/include/asm/stackprotector.h @@ -6,8 +6,10 @@ * the stack frame and verifying that it hasn't been overwritten when * returning from the function. The pattern is called stack canary * and gcc expects it to be defined by a global variable called - * "__stack_chk_guard" on ARM. This unfortunately means that on SMP - * we cannot have a different canary value per task. + * "__stack_chk_guard" on ARM. This prevents SMP systems from using a + * different value for each task unless we enable a GCC plugin that + * replaces these symbol references with references to each task's own + * value. */ #ifndef _ASM_STACKPROTECTOR_H @@ -16,6 +18,8 @@ #include <linux/random.h> #include <linux/version.h> +#include <asm/thread_info.h> + extern unsigned long __stack_chk_guard; /* @@ -33,7 +37,11 @@ static __always_inline void boot_init_stack_canary(void) canary ^= LINUX_VERSION_CODE; current->stack_canary = canary; +#ifndef CONFIG_STACKPROTECTOR_PER_TASK __stack_chk_guard = current->stack_canary; +#else + current_thread_info()->stack_canary = current->stack_canary; +#endif } #endif /* _ASM_STACKPROTECTOR_H */ diff --git a/arch/arm/include/asm/thread_info.h b/arch/arm/include/asm/thread_info.h index 8f55dc520a3e..286eb61c632b 100644 --- a/arch/arm/include/asm/thread_info.h +++ b/arch/arm/include/asm/thread_info.h @@ -53,6 +53,9 @@ struct thread_info { struct task_struct *task; /* main task structure */ __u32 cpu; /* cpu */ __u32 cpu_domain; /* cpu domain */ +#ifdef CONFIG_STACKPROTECTOR_PER_TASK + unsigned long stack_canary; +#endif struct cpu_context_save cpu_context; /* cpu context */ __u32 syscall; /* syscall number */ __u8 used_cp[16]; /* thread used copro */ diff --git a/arch/arm/kernel/asm-offsets.c b/arch/arm/kernel/asm-offsets.c index 3968d6c22455..28b27104ac0c 100644 --- a/arch/arm/kernel/asm-offsets.c +++ b/arch/arm/kernel/asm-offsets.c @@ -79,6 +79,10 @@ int main(void) #ifdef CONFIG_CRUNCH DEFINE(TI_CRUNCH_STATE, offsetof(struct thread_info, crunchstate)); #endif +#ifdef CONFIG_STACKPROTECTOR_PER_TASK + DEFINE(TI_STACK_CANARY, offsetof(struct thread_info, stack_canary)); +#endif + DEFINE(THREAD_SZ_ORDER, THREAD_SIZE_ORDER); BLANK(); DEFINE(S_R0, offsetof(struct pt_regs, ARM_r0)); DEFINE(S_R1, offsetof(struct pt_regs, ARM_r1)); diff --git a/arch/arm/kernel/process.c b/arch/arm/kernel/process.c index 82ab015bf42b..16601d1442d1 100644 --- a/arch/arm/kernel/process.c +++ b/arch/arm/kernel/process.c @@ -39,7 +39,7 @@ #include <asm/tls.h> #include <asm/vdso.h> -#ifdef CONFIG_STACKPROTECTOR +#if defined(CONFIG_STACKPROTECTOR) && !defined(CONFIG_STACKPROTECTOR_PER_TASK) #include <linux/stackprotector.h> unsigned long __stack_chk_guard __read_mostly; EXPORT_SYMBOL(__stack_chk_guard); @@ -267,6 +267,10 @@ copy_thread(unsigned long clone_flags, unsigned long stack_start, thread_notify(THREAD_NOTIFY_COPY, thread); +#ifdef CONFIG_STACKPROTECTOR_PER_TASK + thread->stack_canary = p->stack_canary; +#endif + return 0; } diff --git a/scripts/Makefile.gcc-plugins b/scripts/Makefile.gcc-plugins index 46c5c6809806..048179d8c07f 100644 --- a/scripts/Makefile.gcc-plugins +++ b/scripts/Makefile.gcc-plugins @@ -36,6 +36,12 @@ ifdef CONFIG_GCC_PLUGIN_STACKLEAK endif export DISABLE_STACKLEAK_PLUGIN +gcc-plugin-$(CONFIG_GCC_PLUGIN_ARM_SSP_PER_TASK) += arm_ssp_per_task_plugin.so +ifdef CONFIG_GCC_PLUGIN_ARM_SSP_PER_TASK + DISABLE_ARM_SSP_PER_TASK_PLUGIN += -fplugin-arg-arm_ssp_per_task_plugin-disable +endif +export DISABLE_ARM_SSP_PER_TASK_PLUGIN + # All the plugin CFLAGS are collected here in case a build target needs to # filter them out of the KBUILD_CFLAGS. GCC_PLUGINS_CFLAGS := $(strip $(addprefix -fplugin=$(objtree)/scripts/gcc-plugins/, $(gcc-plugin-y)) $(gcc-plugin-cflags-y)) diff --git a/scripts/gcc-plugins/Kconfig b/scripts/gcc-plugins/Kconfig index 0d5c799688f0..d45f7f36b859 100644 --- a/scripts/gcc-plugins/Kconfig +++ b/scripts/gcc-plugins/Kconfig @@ -190,4 +190,8 @@ config STACKLEAK_RUNTIME_DISABLE runtime to control kernel stack erasing for kernels built with CONFIG_GCC_PLUGIN_STACKLEAK. +config GCC_PLUGIN_ARM_SSP_PER_TASK + bool + depends on GCC_PLUGINS && ARM + endif diff --git a/scripts/gcc-plugins/arm_ssp_per_task_plugin.c b/scripts/gcc-plugins/arm_ssp_per_task_plugin.c new file mode 100644 index 000000000000..de70b8470971 --- /dev/null +++ b/scripts/gcc-plugins/arm_ssp_per_task_plugin.c @@ -0,0 +1,103 @@ +// SPDX-License-Identifier: GPL-2.0 + +#include "gcc-common.h" + +__visible int plugin_is_GPL_compatible; + +static unsigned int sp_mask, canary_offset; + +static unsigned int arm_pertask_ssp_rtl_execute(void) +{ + rtx_insn *insn; + + for (insn = get_insns(); insn; insn = NEXT_INSN(insn)) { + const char *sym; + rtx body; + rtx masked_sp; + + /* + * Find a SET insn involving a SYMBOL_REF to __stack_chk_guard + */ + if (!INSN_P(insn)) + continue; + body = PATTERN(insn); + if (GET_CODE(body) != SET || + GET_CODE(SET_SRC(body)) != SYMBOL_REF) + continue; + sym = XSTR(SET_SRC(body), 0); + if (strcmp(sym, "__stack_chk_guard")) + continue; + + /* + * Replace the source of the SET insn with an expression that + * produces the address of the copy of the stack canary value + * stored in struct thread_info + */ + masked_sp = gen_reg_rtx(Pmode); + + emit_insn_before(gen_rtx_SET(masked_sp, + gen_rtx_AND(Pmode, + stack_pointer_rtx, + GEN_INT(sp_mask))), + insn); + + SET_SRC(body) = gen_rtx_PLUS(Pmode, masked_sp, + GEN_INT(canary_offset)); + } + return 0; +} + +#define PASS_NAME arm_pertask_ssp_rtl + +#define NO_GATE +#include "gcc-generate-rtl-pass.h" + +__visible int plugin_init(struct plugin_name_args *plugin_info, + struct plugin_gcc_version *version) +{ + const char * const plugin_name = plugin_info->base_name; + const int argc = plugin_info->argc; + const struct plugin_argument *argv = plugin_info->argv; + int tso = 0; + int i; + + if (!plugin_default_version_check(version, &gcc_version)) { + error(G_("incompatible gcc/plugin versions")); + return 1; + } + + for (i = 0; i < argc; ++i) { + if (!strcmp(argv[i].key, "disable")) + return 0; + + /* all remaining options require a value */ + if (!argv[i].value) { + error(G_("no value supplied for option '-fplugin-arg-%s-%s'"), + plugin_name, argv[i].key); + return 1; + } + + if (!strcmp(argv[i].key, "tso")) { + tso = atoi(argv[i].value); + continue; + } + + if (!strcmp(argv[i].key, "offset")) { + canary_offset = atoi(argv[i].value); + continue; + } + error(G_("unknown option '-fplugin-arg-%s-%s'"), + plugin_name, argv[i].key); + return 1; + } + + /* create the mask that produces the base of the stack */ + sp_mask = ~((1U << (12 + tso)) - 1); + + PASS_INFO(arm_pertask_ssp_rtl, "expand", 1, PASS_POS_INSERT_AFTER); + + register_callback(plugin_info->base_name, PLUGIN_PASS_MANAGER_SETUP, + NULL, &arm_pertask_ssp_rtl_pass_info); + + return 0; +}