2013-05-20 08:17:24

by Chen Gang

[permalink] [raw]
Subject: [PATCH] include/linux/posix_acl.h: need 'return NULL' when BUG(), if neither CONFIG_BUG nor HAVE_ARCH_BUG is defined.


If neither CONFIG_BUG nor HAVE_ARCH_BUG is defined, the BUG() will
defined as empty (e.g. randconfig with MMU for arm s5pv210)

In this case, need 'return NULL' to let upper caller knows the failure.


Signed-off-by: Chen Gang <[email protected]>
---
include/linux/posix_acl.h | 1 +
1 files changed, 1 insertions(+), 0 deletions(-)

diff --git a/include/linux/posix_acl.h b/include/linux/posix_acl.h
index 7931efe..2c48d06 100644
--- a/include/linux/posix_acl.h
+++ b/include/linux/posix_acl.h
@@ -104,6 +104,7 @@ static inline struct posix_acl **acl_by_type(struct inode *inode, int type)
return &inode->i_default_acl;
default:
BUG();
+ return NULL;
}
}

--
1.7.7.6


2013-05-20 14:40:41

by Eric W. Biederman

[permalink] [raw]
Subject: Re: [PATCH] include/linux/posix_acl.h: need 'return NULL' when BUG(), if neither CONFIG_BUG nor HAVE_ARCH_BUG is defined.



Chen Gang <[email protected]> wrote:

>
>If neither CONFIG_BUG nor HAVE_ARCH_BUG is defined, the BUG() will
>defined as empty (e.g. randconfig with MMU for arm s5pv210)
>
>In this case, need 'return NULL' to let upper caller knows the failure.

Seriously? The correct fix it would seem is to give a useful default BUG definition. Say *NULL.

Further we should never hit that code in the first place if it calls BUG. So upper callers should never get there.

If it is ever possible to get there the callers need to be fixed.

Eric

>Signed-off-by: Chen Gang <[email protected]>
>---
> include/linux/posix_acl.h | 1 +
> 1 files changed, 1 insertions(+), 0 deletions(-)
>
>diff --git a/include/linux/posix_acl.h b/include/linux/posix_acl.h
>index 7931efe..2c48d06 100644
>--- a/include/linux/posix_acl.h
>+++ b/include/linux/posix_acl.h
>@@ -104,6 +104,7 @@ static inline struct posix_acl **acl_by_type(struct
>inode *inode, int type)
> return &inode->i_default_acl;
> default:
> BUG();
>+ return NULL;
> }
> }
>
>--
>1.7.7.6

2013-05-20 16:31:58

by Russell King - ARM Linux

[permalink] [raw]
Subject: Re: [PATCH] include/linux/posix_acl.h: need 'return NULL' when BUG(), if neither CONFIG_BUG nor HAVE_ARCH_BUG is defined.

On Mon, May 20, 2013 at 07:40:24AM -0700, Eric W. Biederman wrote:
> Seriously? The correct fix it would seem is to give a useful default BUG
> definition. Say *NULL.

Absolutely.

The real question is... how is this happening. I've seen it occasionally
in the randconfig builds - though I don't have any logs to hand which
show that in the build system at the moment. So, looking at the files:

arch/arm/include/asm/bug.h:
#ifdef CONFIG_BUG
#define BUG() _BUG(__FILE__, __LINE__, BUG_INSTR_VALUE)
#define _BUG(file, line, value) __BUG(file, line, value)
#ifdef CONFIG_DEBUG_BUGVERBOSE
#define __BUG(__file, __line, __value) \
...
unreachable(); \
...
#else /* not CONFIG_DEBUG_BUGVERBOSE */
#define __BUG(__file, __line, __value) \
...
unreachable(); \
...
#endif /* CONFIG_DEBUG_BUGVERBOSE */

#define HAVE_ARCH_BUG
#endif /* CONFIG_BUG */

#include <asm-generic/bug.h>

So here, whenever we have CONFIG_BUG enabled, we always define a non-empty
BUG() macro to override the generic version. So far so good.

include/asm-generic/bug.h:

#ifdef CONFIG_BUG
... we're providing our own version ...

#else /* !CONFIG_BUG */
#ifndef HAVE_ARCH_BUG
#define BUG() do {} while(0)
#endif

#ifndef HAVE_ARCH_BUG_ON
#define BUG_ON(condition) do { if (condition) ; } while(0)
#endif

So, the generic stuff becomes no-ops which just fall through. Seriously?
Is this really what we want?

So either we need to fix the generic bug.h code, or fix every site which
uses BUG()/BUG_ON().

Note - I'm not saying *NULL as you did, because that isn't guaranteed to
fault on all platforms. NoMMU may allow *NULL to succeed without causing
an exception.

2013-05-21 01:52:49

by Chen Gang

[permalink] [raw]
Subject: Re: [PATCH] include/linux/posix_acl.h: need 'return NULL' when BUG(), if neither CONFIG_BUG nor HAVE_ARCH_BUG is defined.

On 05/21/2013 12:31 AM, Russell King - ARM Linux wrote:
> On Mon, May 20, 2013 at 07:40:24AM -0700, Eric W. Biederman wrote:
>> Seriously? The correct fix it would seem is to give a useful default BUG
>> definition. Say *NULL.
>
> Absolutely.
>
> The real question is... how is this happening. I've seen it occasionally
> in the randconfig builds - though I don't have any logs to hand which
> show that in the build system at the moment. So, looking at the files:
>

Maybe it is my fault for the incorrect configuration.

My operation:
make EXTRA_CFLAGS=-W ARCH=arm randconfig
make EXTRA_CFLAGS=-W ARCH=arm menuconfig
set 'arm-linux-gnu-' as cross compiler prefix.
select "MMU-based Paged Memory" in "System Type".
select "Samsung S5PV210/S5PC110" in "ARM System type"
make EXTRA_CFLAGS=-W ARCH=arm
...

In menuconfig, I do not touch the "S5PV210 Machines" or "S5PC110
Machines" in "System Type", and no machines will be chosen as default.

I think, it may be my fault, or need provide a default machine for S5P*
with randconfig (also better to select MMU automatically).


Thanks.
--
Chen Gang

Asianux Corporation

2013-05-21 11:01:00

by Russell King - ARM Linux

[permalink] [raw]
Subject: Re: [PATCH] include/linux/posix_acl.h: need 'return NULL' when BUG(), if neither CONFIG_BUG nor HAVE_ARCH_BUG is defined.

On Tue, May 21, 2013 at 09:51:57AM +0800, Chen Gang wrote:
> On 05/21/2013 12:31 AM, Russell King - ARM Linux wrote:
> > On Mon, May 20, 2013 at 07:40:24AM -0700, Eric W. Biederman wrote:
> >> Seriously? The correct fix it would seem is to give a useful default BUG
> >> definition. Say *NULL.
> >
> > Absolutely.
> >
> > The real question is... how is this happening. I've seen it occasionally
> > in the randconfig builds - though I don't have any logs to hand which
> > show that in the build system at the moment. So, looking at the files:
> >
>
> Maybe it is my fault for the incorrect configuration.
>
> My operation:
> make EXTRA_CFLAGS=-W ARCH=arm randconfig
> make EXTRA_CFLAGS=-W ARCH=arm menuconfig
> set 'arm-linux-gnu-' as cross compiler prefix.
> select "MMU-based Paged Memory" in "System Type".
> select "Samsung S5PV210/S5PC110" in "ARM System type"
> make EXTRA_CFLAGS=-W ARCH=arm
> ...
>
> In menuconfig, I do not touch the "S5PV210 Machines" or "S5PC110
> Machines" in "System Type", and no machines will be chosen as default.

You know, telling us that you're using a random configuration to produce
a problem and not providing the actual configuration file is utterly
insane. How many iterations do you think it might take to generate a
random configuration which shows the same problem you're seeing?

2013-05-21 11:17:10

by Chen Gang

[permalink] [raw]
Subject: Re: [PATCH] include/linux/posix_acl.h: need 'return NULL' when BUG(), if neither CONFIG_BUG nor HAVE_ARCH_BUG is defined.

On 05/21/2013 06:59 PM, Russell King - ARM Linux wrote:
> On Tue, May 21, 2013 at 09:51:57AM +0800, Chen Gang wrote:
>> On 05/21/2013 12:31 AM, Russell King - ARM Linux wrote:
>>> On Mon, May 20, 2013 at 07:40:24AM -0700, Eric W. Biederman wrote:
>>>> Seriously? The correct fix it would seem is to give a useful default BUG
>>>> definition. Say *NULL.
>>>
>>> Absolutely.
>>>
>>> The real question is... how is this happening. I've seen it occasionally
>>> in the randconfig builds - though I don't have any logs to hand which
>>> show that in the build system at the moment. So, looking at the files:
>>>
>>
>> Maybe it is my fault for the incorrect configuration.
>>
>> My operation:
>> make EXTRA_CFLAGS=-W ARCH=arm randconfig
>> make EXTRA_CFLAGS=-W ARCH=arm menuconfig
>> set 'arm-linux-gnu-' as cross compiler prefix.
>> select "MMU-based Paged Memory" in "System Type".
>> select "Samsung S5PV210/S5PC110" in "ARM System type"
>> make EXTRA_CFLAGS=-W ARCH=arm
>> ...
>>
>> In menuconfig, I do not touch the "S5PV210 Machines" or "S5PC110
>> Machines" in "System Type", and no machines will be chosen as default.
>
> You know, telling us that you're using a random configuration to produce
> a problem and not providing the actual configuration file is utterly
> insane. How many iterations do you think it might take to generate a
> random configuration which shows the same problem you're seeing?
>
>

Oh, it is my fault for my original reply (it is another issue which
also need further discussed)

But for current issue, we can repeat it only in menuconfig, unset " >
General setup > Configure standard kernel features (expert users) >
BUG() Support ", it will report related warnings.

For arm architecture, allmodconfig

make ARCH=arm allmodconfig
make ARCH=arm menuconfig
set "arm-linux-gnu-" as cross compiler prefix.
unset " > General setup > Configure standard kernel features (expert users) > BUG() Support "
make ARCH=arm
...
kernel/sched/core.c:2353:1: warning: control reaches end of non-void function [-Wreturn-type]
mm/bootmem.c:386:1: warning: control reaches end of non-void function [-Wreturn-type]
include/linux/posix_acl.h:108:1: warning: control reaches end of non-void function [-Wreturn-type]
...

For x86 architecture, allmodconfig:

make allmodconfig
make menuconfig
unset " > General setup > Configure standard kernel features (expert users) > BUG() Support "
make ARCH=arm
...
arch/x86/kvm/x86.c:4298:5: warning: ‘exchanged’ may be used uninitialized in this function [-Wuninitialized]
kernel/sched/core.c:2353:1: warning: control reaches end of non-void function [-Wreturn-type]
mm/mempolicy.c:1755:1: warning: control reaches end of non-void function [-Wreturn-type]
include/linux/posix_acl.h:108:1: warning: control reaches end of non-void function [-Wreturn-type]
...


So, we need fix the generic bug.h code, it is not arm specific.


Thanks.--
Chen Gang

Asianux Corporation