There are some cases where the code generated for BUG() results
into an infinite while loop without causing a null dereference,
this ends on a kernel being stuck on a loop and the user without
a clue of what happened.
E.g.: lib/scatterlist.c : __sg_alloc_table
BUG_ON(nents > max_ents);
438: 9a000000 bls 440 <__sg_alloc_table+0x20>
43c: eafffffe b 43c <__sg_alloc_table+0x1c>
Adding volatile makes the compiler to avoid optimizations on this
code, which makes the panic to occur:
BUG_ON(nents > max_ents);
438: 9a000002 bls 448 <__sg_alloc_table+0x28>
43c: e3a03000 mov r3, #0
440: e5833000 str r3, [r3]
444: eafffffc b 43c <__sg_alloc_table+0x1c>
Seen with gnu/linux cs arm-2010q1-202 and arm2010.09-50.
Signed-off-by: Omar Ramirez Luna <[email protected]>
---
If needed I can change:
./arch/arm/mach-clps711x/include/mach/io.h:
#define __raw_readsb(p,d,l) do { *(int *)0 = 0; } while (0)
./arch/arm/mach-clps711x/include/mach/io.h:
#define __raw_readsl(p,d,l) do { *(int *)0 = 0; } while (0)
./arch/arm/mach-clps711x/include/mach/io.h:
#define __raw_writesb(p,d,l) do { *(int *)0 = 0; } while (0)
./arch/arm/mach-clps711x/include/mach/io.h:
#define __raw_writesl(p,d,l) do { *(int *)0 = 0; } while (0)
./arch/arm/kernel/traps.c: *(int *)0 = 0;
arch/arm/include/asm/bug.h | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/arch/arm/include/asm/bug.h b/arch/arm/include/asm/bug.h
index 4d88425..a58d863 100644
--- a/arch/arm/include/asm/bug.h
+++ b/arch/arm/include/asm/bug.h
@@ -12,7 +12,7 @@ extern void __bug(const char *file, int line) __attribute__((noreturn));
#else
/* this just causes an oops */
-#define BUG() do { *(int *)0 = 0; } while (1)
+#define BUG() do { *(volatile int *)0 = 0; } while(1)
#endif
--
1.7.1
You might try the generic bug patch and see if it also fixes this
problem - Simon
On Thu, Mar 31, 2011 at 1:15 PM, Omar Ramirez Luna <[email protected]> wrote:
> There are some cases where the code generated for BUG() results
> into an infinite while loop without causing a null dereference,
> this ends on a kernel being stuck on a loop and the user without
> a clue of what happened.
>
> E.g.: lib/scatterlist.c : __sg_alloc_table
>
> ? ? ? ?BUG_ON(nents > max_ents);
> ?438: ? 9a000000 ? ? ? ?bls ? ? 440 <__sg_alloc_table+0x20>
> ?43c: ? eafffffe ? ? ? ?b ? ? ? 43c <__sg_alloc_table+0x1c>
>
> Adding volatile makes the compiler to avoid optimizations on this
> code, which makes the panic to occur:
>
> ? ? ? ?BUG_ON(nents > max_ents);
> ?438: ? 9a000002 ? ? ? ?bls ? ? 448 <__sg_alloc_table+0x28>
> ?43c: ? e3a03000 ? ? ? ?mov ? ? r3, #0
> ?440: ? e5833000 ? ? ? ?str ? ? r3, [r3]
> ?444: ? eafffffc ? ? ? ?b ? ? ? 43c <__sg_alloc_table+0x1c>
>
> Seen with gnu/linux cs arm-2010q1-202 and arm2010.09-50.
>
> Signed-off-by: Omar Ramirez Luna <[email protected]>
> ---
>
> If needed I can change:
> ./arch/arm/mach-clps711x/include/mach/io.h:
> ? ? ? ?#define __raw_readsb(p,d,l) ?do { *(int *)0 = 0; } while (0)
> ./arch/arm/mach-clps711x/include/mach/io.h:
> ? ? ? ?#define __raw_readsl(p,d,l) ?do { *(int *)0 = 0; } while (0)
> ./arch/arm/mach-clps711x/include/mach/io.h:
> ? ? ? ?#define __raw_writesb(p,d,l) do { *(int *)0 = 0; } while (0)
> ./arch/arm/mach-clps711x/include/mach/io.h:
> ? ? ? ?#define __raw_writesl(p,d,l) do { *(int *)0 = 0; } while (0)
> ./arch/arm/kernel/traps.c: ? ? ?*(int *)0 = 0;
>
>
> ?arch/arm/include/asm/bug.h | ? ?2 +-
> ?1 files changed, 1 insertions(+), 1 deletions(-)
>
> diff --git a/arch/arm/include/asm/bug.h b/arch/arm/include/asm/bug.h
> index 4d88425..a58d863 100644
> --- a/arch/arm/include/asm/bug.h
> +++ b/arch/arm/include/asm/bug.h
> @@ -12,7 +12,7 @@ extern void __bug(const char *file, int line) __attribute__((noreturn));
> ?#else
>
> ?/* this just causes an oops */
> -#define BUG() ? ? ? ? ?do { *(int *)0 = 0; } while (1)
> +#define BUG() ? ? ? ? ?do { *(volatile int *)0 = 0; } while(1)
>
> ?#endif
>
> --
> 1.7.1
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to [email protected]
> More majordomo info at ?http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at ?http://www.tux.org/lkml/
>
On 3/31/2011 6:47 PM, Simon Glass wrote:
> You might try the generic bug patch and see if it also fixes this
> problem - Simon
It probably won't because presumably Omar has CONFIG_BUG=n otherwise
Omar would have sent a patch against __bug() in arch/arm/kernel/traps.c
--
Sent by an employee of the Qualcomm Innovation Center, Inc.
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum.
Hi Stephen,
Sorry for the confusion, but in fact I was talking about the patch to
make ARM use the generic bug handling via an undef instruction instead
of calling ______bug() or writing to memory address 0. Please see
here:
http://www.arm.linux.org.uk/developer/patches/viewpatch.php?id=6808/1
Regards,
Simon
On Fri, Apr 1, 2011 at 1:44 AM, Stephen Boyd <[email protected]> wrote:
> On 3/31/2011 6:47 PM, Simon Glass wrote:
>> You might try the generic bug patch and see if it also fixes this
>> problem - Simon
>
> It probably won't because presumably Omar has CONFIG_BUG=n otherwise
> Omar would have sent a patch against __bug() in arch/arm/kernel/traps.c
>
> --
> Sent by an employee of the Qualcomm Innovation Center, Inc.
> The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum.
>
>
(Please stop top posting)
On 4/1/2011 3:29 PM, Simon Glass wrote:
> Hi Stephen,
>
> Sorry for the confusion, but in fact I was talking about the patch to
> make ARM use the generic bug handling via an undef instruction instead
> of calling ______bug() or writing to memory address 0. Please see
> here:
>
> http://www.arm.linux.org.uk/developer/patches/viewpatch.php?id=6808/1
Yes I've seen your patch (and even posted comments on it which have not
been responded to).
Correct me if I'm wrong, but that patch with CONFIG_BUG=n would lead to
the same error that Omar is seeing because the code only modifies the
bug infrastructure when CONFIG_BUG=y.
--
Sent by an employee of the Qualcomm Innovation Center, Inc.
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum.
On Sun, Apr 3, 2011 at 12:15 AM, Stephen Boyd <[email protected]> wrote:
> (Please stop top posting)
>
> On 4/1/2011 3:29 PM, Simon Glass wrote:
>> Hi Stephen,
>>
>> Sorry for the confusion, but in fact I was talking about the patch to
>> make ARM use the generic bug handling via an undef instruction instead
>> of calling ______bug() or writing to memory address 0. Please see
>> here:
>>
>> http://www.arm.linux.org.uk/developer/patches/viewpatch.php?id=6808/1
>
> Yes I've seen your patch (and even posted comments on it which have not
> been responded to).
Hi Stephen,
Not yet! Don't worry I will get to it. I like the suggestion and am
pleased that you pointed me to it, thank you.
> Correct me if I'm wrong, but that patch with CONFIG_BUG=n would lead to
> the same error that Omar is seeing because the code only modifies the
> bug infrastructure when CONFIG_BUG=y.
Well if CONFIG_BUG=n then there is no bug infrastructure, The whole
file is skipped and it falls back to the asm-generic/bug.h which has
even more #ifdefs in it. But I think we end up here:
#define BUG() do {} while(0)
After all the patch removes the *(int*)0 = 0 code by virtue of
CONFIG_GENERIC_BUG=y, right? If I have this wrong then I will have to
break out the C preprocessor...
Regards,
Simon
>
> --
> Sent by an employee of the Qualcomm Innovation Center, Inc.
> The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum.
>
>
Hi Simon, Stephen,
On Sun, Apr 3, 2011 at 2:15 AM, Stephen Boyd <[email protected]> wrote:
> (Please stop top posting)
>
> On 4/1/2011 3:29 PM, Simon Glass wrote:
>> Hi Stephen,
>>
>> Sorry for the confusion, but in fact I was talking about the patch to
>> make ARM use the generic bug handling via an undef instruction instead
>> of calling ______bug() or writing to memory address 0. Please see
>> here:
>>
>> http://www.arm.linux.org.uk/developer/patches/viewpatch.php?id=6808/1
>
> Yes I've seen your patch (and even posted comments on it which have not
> been responded to).
>
> Correct me if I'm wrong, but that patch with CONFIG_BUG=n would lead to
> the same error that Omar is seeing because the code only modifies the
> bug infrastructure when CONFIG_BUG=y.
I am using CONFIG_BUG=y, however I don't have CONFIG_DEBUG_BUGVERBOSE
and hence I fall into the part which doesn't print the file and the
line where the BUG was found.
With Simon's patch if my .config had:
CONFIG_BUG=y
CONFIG_GENERIC_BUG is not set
CONFIG_DEBUG_BUGVERBOSE is not set
I would fall into the same BUG definition that is causing issues:
#define BUG() do { *(int *)0 = 0; } while (1)
OTOH, is not like "Use generic BUG() handler" gives the choice of
removing GENERIC_BUG given that it is not prompted in menuconfig and
auto selected, if this is the intention is there any reason to keep
the #else part of /* not CONFIG_GENERIC_BUG */? there is no way we can
use it with this patch, right?
Regards,
Omar
On Mon, Apr 4, 2011 at 7:29 PM, Ramirez Luna, Omar <[email protected]> wrote:
...
> I am using CONFIG_BUG=y, however I don't have CONFIG_DEBUG_BUGVERBOSE
> and hence I fall into the part which doesn't print the file and the
> line where the BUG was found.
>
> With Simon's patch if my .config had:
>
> CONFIG_BUG=y
> CONFIG_GENERIC_BUG is not set
In this case the patch is like a nop.
> CONFIG_DEBUG_BUGVERBOSE is not set
>
> I would fall into the same BUG definition that is causing issues:
>
> #define BUG() ? ? ? ? ? do { *(int *)0 = 0; } while (1)
>
> OTOH, is not like "Use generic BUG() handler" gives the choice of
> removing GENERIC_BUG given that it is not prompted in menuconfig and
> auto selected, if this is the intention is there any reason to keep
> the #else part of /* not CONFIG_GENERIC_BUG */? there is no way we can
> use it with this patch, right?
Well, er, the intention is that you use the patch. I kept the old code
around since people can then simply change the Kconfig option and be
back where they were, as indeed you have. I would be happy to remove
the old behavior, but I was concerned about a possible roasting in
this forum. Changing long-established behavior is sometimes tricky.
Regards,
Simon
>
> Regards,
>
> Omar
>
Hi,
On Tue, Apr 5, 2011 at 12:46 AM, Simon Glass <[email protected]> wrote:
> On Mon, Apr 4, 2011 at 7:29 PM, Ramirez Luna, Omar <[email protected]> wrote:
> ...
>> I am using CONFIG_BUG=y, however I don't have CONFIG_DEBUG_BUGVERBOSE
>> and hence I fall into the part which doesn't print the file and the
>> line where the BUG was found.
>>
>> With Simon's patch if my .config had:
>>
>> CONFIG_BUG=y
>> CONFIG_GENERIC_BUG is not set
>
> In this case the patch is like a nop.
>
>> CONFIG_DEBUG_BUGVERBOSE is not set
>>
>> I would fall into the same BUG definition that is causing issues:
>>
>> #define BUG() ? ? ? ? ? do { *(int *)0 = 0; } while (1)
>>
>> OTOH, is not like "Use generic BUG() handler" gives the choice of
>> removing GENERIC_BUG given that it is not prompted in menuconfig and
>> auto selected, if this is the intention is there any reason to keep
>> the #else part of /* not CONFIG_GENERIC_BUG */? there is no way we can
>> use it with this patch, right?
>
> Well, er, the intention is that you use the patch. I kept the old code
> around since people can then simply change the Kconfig option and be
> back where they were, as indeed you have. I would be happy to remove
> the old behavior, but I was concerned about a possible roasting in
> this forum. Changing long-established behavior is sometimes tricky.
Yes, but you can't change the Kconfig because it is not prompted with
your patch, if that was the intention then an option to de/select
GENERIC_BUG or not is needed:
arch/arm/Kconfig
@@ -204,6 +204,10 @@ config MMU
Select if you want MMU-based virtualised addressing space
support by paged memory management. If unsure, say 'Y'.
+config GENERIC_BUG
+ bool "Generic BUG"
+ default y
+ depends on BUG
Regards,
Omar
On Tue, Apr 5, 2011 at 9:02 AM, Ramirez Luna, Omar <[email protected]> wrote:
> Hi,
>
> On Tue, Apr 5, 2011 at 12:46 AM, Simon Glass <[email protected]> wrote:
>> On Mon, Apr 4, 2011 at 7:29 PM, Ramirez Luna, Omar <[email protected]> wrote:
>> ...
>>> OTOH, is not like "Use generic BUG() handler" gives the choice of
>>> removing GENERIC_BUG given that it is not prompted in menuconfig and
>>> auto selected, if this is the intention is there any reason to keep
>>> the #else part of /* not CONFIG_GENERIC_BUG */? there is no way we can
>>> use it with this patch, right?
>>
>> Well, er, the intention is that you use the patch. I kept the old code
>> around since people can then simply change the Kconfig option and be
>> back where they were, as indeed you have. I would be happy to remove
>> the old behavior, but I was concerned about a possible roasting in
>> this forum. Changing long-established behavior is sometimes tricky.
>
> Yes, but you can't change the Kconfig because it is not prompted with
> your patch, if that was the intention then an option to de/select
> GENERIC_BUG or not is needed:
>
> arch/arm/Kconfig
> @@ -204,6 +204,10 @@ config MMU
> ? ? ? ? ?Select if you want MMU-based virtualised addressing space
> ? ? ? ? ?support by paged memory management. If unsure, say 'Y'.
>
> +config GENERIC_BUG
> + ? ? ? bool "Generic BUG"
> + ? ? ? default y
> + ? ? ? depends on BUG
Yes that's right, it is a manual edit. But if there are no objections
I will create a patch to remove this and replace the old bug
infrastructure entirely. - Simon
>
> Regards,
>
> Omar
>
On 4/4/2011 6:55 PM, Simon Glass wrote:
> On Sun, Apr 3, 2011 at 12:15 AM, Stephen Boyd <[email protected]> wrote:
>> Yes I've seen your patch (and even posted comments on it which have not
>> been responded to).
> Hi Stephen,
>
> Not yet! Don't worry I will get to it. I like the suggestion and am
> pleased that you pointed me to it, thank you.
>
Great!
>> Correct me if I'm wrong, but that patch with CONFIG_BUG=n would lead to
>> the same error that Omar is seeing because the code only modifies the
>> bug infrastructure when CONFIG_BUG=y.
> Well if CONFIG_BUG=n then there is no bug infrastructure, The whole
> file is skipped and it falls back to the asm-generic/bug.h which has
> even more #ifdefs in it. But I think we end up here:
>
> #define BUG() do {} while(0)
>
> After all the patch removes the *(int*)0 = 0 code by virtue of
> CONFIG_GENERIC_BUG=y, right? If I have this wrong then I will have to
> break out the C preprocessor...
>
Ah you're right. Too many ifdefs going on there.
--
Sent by an employee of the Qualcomm Innovation Center, Inc.
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum.