[LKP] [PATCH] drm/radeon: Try to init amdkfd only if 64 bit kernel
Oded Gabbay writes:
> I didn't say it doesn't always work.
> The actual thing that doesn't work is the define symbol_get and only in a
> specific case of 32bit kernel AND CONFIG_MODULES is unset AND
> CONFIG_RANDOMIZE_BASE is set.
> The define in that case is:
> #define symbol_get(x) ({ extern typeof(x) x __attribute__((weak)); &(x); })
>
> Why it doesn't work (doesn't return NULL when symbol doesn't exists) ?
Hmm, I'd guess CONFIG_RANDOMIZE_BASE is relocating NULL symbols...
No, I can't reproduce this. Please send your .config privately.
Here's my test case:
diff --git a/init/main.c b/init/main.c
index 61b993767db5..a3ee1ec97ec3 100644
--- a/init/main.c
+++ b/init/main.c
@@ -683,6 +683,12 @@ asmlinkage __visible void __init start_kernel(void)
ftrace_init();
+ {
+ extern void nonexistent_fn(void);
+ printk("symbol_get(nonexistent_fn) = %p\n",
+ symbol_get(nonexistent_fn));
+ }
+
/* Do the rest non-__init'ed, we're now alive */
rest_init();
}
Thanks,
Rusty.
[LKP] [PATCH] drm/radeon: Try to init amdkfd only if 64 bit kernel
Kees Cook writes: > On Sun, Jan 4, 2015 at 8:28 PM, Rusty Russell > wrote: >> Kees, as far as I can tell you need another 0-terminated vmlinux.relocs >> section for weak symbols. These should not be relocated if already 0. > > A few questions: > > Why doesn't this break on 32-bit without kASLR? 32-bit does relocation > by default, even without CONFIG_RANDOMIZE_BASE. Well, the offset was 0 until I removed CONFIG_HIBERNATE. > Are there any symbols that are NULL that aren't weak? I'd expect all > strong symbols to have non-zero offsets, but I must be > misunderstanding something here. I don't think there would be. Anyway, you might be able to filter them out in x86/tools/relocs itself. Cheers, Rusty.
[LKP] [PATCH] drm/radeon: Try to init amdkfd only if 64 bit kernel
Kees Cook writes:
> On Sun, Jan 4, 2015 at 8:28 PM, Rusty Russell
> wrote:
>> Oded Gabbay writes:
>>> On 12/24/2014 01:01 AM, Rusty Russell wrote:
>>>> Oded Gabbay writes:
>>>>> I didn't say it doesn't always work.
>>>>> The actual thing that doesn't work is the define symbol_get and only in a
>>>>> specific case of 32bit kernel AND CONFIG_MODULES is unset AND
>>>>> CONFIG_RANDOMIZE_BASE is set.
>>>>> The define in that case is:
>>>>> #define symbol_get(x) ({ extern typeof(x) x __attribute__((weak)); &(x);
>>>>> })
>>>>>
>>>>> Why it doesn't work (doesn't return NULL when symbol doesn't exists) ?
>>>>
>>>> Hmm, I'd guess CONFIG_RANDOMIZE_BASE is relocating NULL symbols...
>>>>
>>>> No, I can't reproduce this. Please send your .config privately.
>>>>
>>>> Here's my test case:
>>>>
>>>> diff --git a/init/main.c b/init/main.c
>>>> index 61b993767db5..a3ee1ec97ec3 100644
>>>> --- a/init/main.c
>>>> +++ b/init/main.c
>>>> @@ -683,6 +683,12 @@ asmlinkage __visible void __init start_kernel(void)
>>>>
>>>> ftrace_init();
>>>>
>>>> +{
>>>> +extern void nonexistent_fn(void);
>>>> +printk("symbol_get(nonexistent_fn) = %p\n",
>>>> + symbol_get(nonexistent_fn));
>>>> +}
>>>> +
>>>> /* Do the rest non-__init'ed, we're now alive */
>>>> rest_init();
>>>> }
>>>>
>>>> Thanks,
>>>> Rusty.
>>>>
>>> Hi Rusty,
>>>
>>> Attached is the bad config file. (config-bad)
>>> I have narrowed the changes you need to do to the config file in order to
>>> reproduce this bug.
>>> The base assumption is a 32-bit kernel and without modules support. Rest of
>>> the
>>> config file is pretty standard, IMO.
>>> Then, its not enough to enable CONFIG_RANDOMIZE_BASE like I wrote in my
>>> original
>>> post. You need also to unset CONFIG_HIBERNATION.
>>
>> Indeed, thanks; your config breaks as reported. With CONFIG_HIBERNATION
>> the kernel offset is 0, so we don't see this.
>>
>> Kees, as far as I can tell you need another 0-terminated vmlinux.relocs
>> section for weak symbols. These should not be relocated if already 0.
>>
>> Put this somewhere to test. It fails for x86_64, too:
>>
>> diff --git a/init/main.c b/init/main.c
>> index 61b993767db5..c9e0195c792a 100644
>> --- a/init/main.c
>> +++ b/init/main.c
>> @@ -683,6 +683,12 @@ asmlinkage __visible void __init start_kernel(void)
>>
>> ftrace_init();
>>
>> + {
>> + extern void __attribute__((weak)) nonexistent_fn(void);
>> + printk("nonexistent_fn = %p\n", nonexistent_fn);
>> + BUG_ON(nonexistent_fn != NULL);
>> + }
>> +
>> /* Do the rest non-__init'ed, we're now alive */
>> rest_init();
>> }
>
> Hm, I can't reproduce this on v3.19-rc4. My nonexistent_fn comes back
> NULL regardless of CONFIG and kaslr on/off states I've tried. Could
> this be a (yet another) linker bug? What was the toolchain used? I
> built with gcc 4.8.2 and binutils 2.24.
$ ld --version
GNU ld (GNU Binutils for Ubuntu) 2.24.90.20141014
...
$ gcc --version
gcc (Ubuntu 4.9.1-16ubuntu6) 4.9.1
I tested with gcc 4.8.3 as well, same fail.
I'll send you the config I used separately.
Cheers,
Rusty.
[LKP] [PATCH] drm/radeon: Try to init amdkfd only if 64 bit kernel
Oded Gabbay writes:
> On 12/24/2014 01:01 AM, Rusty Russell wrote:
>> Oded Gabbay writes:
>>> I didn't say it doesn't always work.
>>> The actual thing that doesn't work is the define symbol_get and only in a
>>> specific case of 32bit kernel AND CONFIG_MODULES is unset AND
>>> CONFIG_RANDOMIZE_BASE is set.
>>> The define in that case is:
>>> #define symbol_get(x) ({ extern typeof(x) x __attribute__((weak)); &(x); })
>>>
>>> Why it doesn't work (doesn't return NULL when symbol doesn't exists) ?
>>
>> Hmm, I'd guess CONFIG_RANDOMIZE_BASE is relocating NULL symbols...
>>
>> No, I can't reproduce this. Please send your .config privately.
>>
>> Here's my test case:
>>
>> diff --git a/init/main.c b/init/main.c
>> index 61b993767db5..a3ee1ec97ec3 100644
>> --- a/init/main.c
>> +++ b/init/main.c
>> @@ -683,6 +683,12 @@ asmlinkage __visible void __init start_kernel(void)
>>
>> ftrace_init();
>>
>> +{
>> +extern void nonexistent_fn(void);
>> +printk("symbol_get(nonexistent_fn) = %p\n",
>> + symbol_get(nonexistent_fn));
>> +}
>> +
>> /* Do the rest non-__init'ed, we're now alive */
>> rest_init();
>> }
>>
>> Thanks,
>> Rusty.
>>
> Hi Rusty,
>
> Attached is the bad config file. (config-bad)
> I have narrowed the changes you need to do to the config file in order to
> reproduce this bug.
> The base assumption is a 32-bit kernel and without modules support. Rest of
> the
> config file is pretty standard, IMO.
> Then, its not enough to enable CONFIG_RANDOMIZE_BASE like I wrote in my
> original
> post. You need also to unset CONFIG_HIBERNATION.
Indeed, thanks; your config breaks as reported. With CONFIG_HIBERNATION
the kernel offset is 0, so we don't see this.
Kees, as far as I can tell you need another 0-terminated vmlinux.relocs
section for weak symbols. These should not be relocated if already 0.
Put this somewhere to test. It fails for x86_64, too:
diff --git a/init/main.c b/init/main.c
index 61b993767db5..c9e0195c792a 100644
--- a/init/main.c
+++ b/init/main.c
@@ -683,6 +683,12 @@ asmlinkage __visible void __init start_kernel(void)
ftrace_init();
+ {
+ extern void __attribute__((weak)) nonexistent_fn(void);
+ printk("nonexistent_fn = %p\n", nonexistent_fn);
+ BUG_ON(nonexistent_fn != NULL);
+ }
+
/* Do the rest non-__init'ed, we're now alive */
rest_init();
}
Cheers,
Rusty.
Build regressions/improvements in v4.1-rc1
Geert Uytterhoeven writes: > On Mon, Apr 27, 2015 at 11:51 AM, Geert Uytterhoeven > wrote: >> Below is the list of build error/warning regressions/improvements in >> v4.1-rc1[1] compared to v4.0[2]. >> >> Summarized: >> - build errors: +34/-11 >> - build warnings: +135/-163 >> >> As I haven't mastered kup yet, there's no verbose summary at >> http://www.kernel.org/pub/linux/kernel/people/geert/linux-log/v4.1-rc1.summary.gz >> >> Happy fixing! ;-) >> >> Thanks to the linux-next team for providing the build service. >> >> [1] http://kisskb.ellerman.id.au/kisskb/head/8779/ (254 out of 257 configs) >> [2] http://kisskb.ellerman.id.au/kisskb/head/8710/ (254 out of 257 configs) >> >> >> *** ERRORS *** >> >> 34 regressions: > > The quiet days are over... > >> + /home/kisskb/slave/src/arch/mips/cavium-octeon/smp.c: error: passing >> argument 2 of 'cpumask_clear_cpu' discards 'volatile' qualifier from pointer >> target type [-Werror]: => 242:2 >> + /home/kisskb/slave/src/arch/mips/kernel/process.c: error: passing >> argument 2 of 'cpumask_test_cpu' discards 'volatile' qualifier from pointer >> target type [-Werror]: => 52:2 >> + /home/kisskb/slave/src/arch/mips/kernel/smp.c: error: passing argument 2 >> of 'cpumask_set_cpu' discards 'volatile' qualifier from pointer target type >> [-Werror]: => 149:2, 211:2 >> + /home/kisskb/slave/src/arch/mips/kernel/smp.c: error: passing argument 2 >> of 'cpumask_test_cpu' discards 'volatile' qualifier from pointer target type >> [-Werror]: => 221:2 > > mips/bigsur_defconfig > mips/malta_defconfig > mips/cavium_octeon_defconfig > mips/ip27_defconfig Already fixed in other thread... > and related warnings due to lack of -Werror on > ia64-defconfig That fix is fairly obvious, I'll post separately. > tilegx_defconfig Can't see that one with a simple grep: can you post warning? > m32r/m32700ut.smp_defconfig Will post fix for this too. > cpumask also gives fishy warnings: > > lib/cpumask.c:167:25: warning: the address of 'cpu_all_bits' will > always evaluate as 'true' [-Waddress] > > on sparc (e.g. sparc64/sparc64-allmodconfig) and powerpc (e.g. > powerpc/ppc64_defconfig), which seem to have been reported 6 months > ago... Hmm, this is cpumask_of_node? That's... Oh my, that requires a separate post. > Can we throw some bitcoins at the cpumasks? ;-) I think I should be throwing bitcoins at you, instead! Thanks, Rusty.
Build regressions/improvements in v4.1-rc1
Geert Uytterhoeven writes: >> Can't see that one with a simple grep: can you post warning? > > /home/kisskb/slave/src/arch/tile/kernel/setup.c: In function > 'zone_sizes_init': > /home/kisskb/slave/src/arch/tile/kernel/setup.c:777:3: warning: > passing argument 2 of 'cpumask_test_cpu' from incompatible pointer > type [enabled by default] > /home/kisskb/slave/src/include/linux/cpumask.h:294:19: note: expected > 'const struct cpumask *' but argument is of type 'struct nodemask_t *' Um, I turned the cpu_isset() into cpumask_test_cpu(), but that just showed this bug up. The tile maintainers need to fix this one. Thanks, Rusty.
