illegal instructions / irqs disabled warning

illegal instructions / irqs disabled warning

[Johannes Berg]

[-- Attachment #1: Type: text/plain, Size: 1832 bytes --]

So I run across this warning: 
[ 2724.595549] Debug: sleeping function called from invalid context at arch/ppc/kernel/traps.c:528
[ 2724.595563] in_atomic():0, irqs_disabled():1
[ 2724.595568] Call trace:
[ 2724.595573]  [c001fe54] __might_sleep+0xd4/0xf0
[ 2724.595592]  [c0005ad8] program_check_exception+0xb8/0x520
[ 2724.595606]  [c0004f04] ret_from_except_full+0x0/0x4c

when I wrote a program calling illegal instructions. I then checked out
why this happened and the cause is the might_sleep() here:

#define __get_user_check(x, ptr, size)                                  \
({                                                                      \
        long __gu_err = -EFAULT;                                        \
        unsigned long  __gu_val = 0;                                    \
        const __typeof__(*(ptr)) __user *__gu_addr = (ptr);             \
        might_sleep();                                                  \
...

I then figured I could use __copy_from_user_inatomic to access the
instruction word to fix this (as far as I can tell only the warning is
annoying, because the instruction already failed at that point so it
must be in memory, right?)

But here's the actual question:
static inline unsigned long __copy_from_user(void *to,
                const void __user *from, unsigned long size)
{
        might_sleep();
        return __copy_from_user_inatomic(to, from, size);
}

Does that mean __copy_from_user_inatomic isn't actually valid to call in
atomic context? Or is this only so that kernel developers that use
powerpc see the bugs their code would have on other platforms? The magic
in get_user_asm thoroughly confuses me.

[Somehow I got sidetracked. I only wanted to look at implementing
backtrace support for oprofile...]

johannes

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 832 bytes --]

Re: illegal instructions / irqs disabled warning

[Kumar Gala]

On Dec 14, 2005, at 8:05 AM, Johannes Berg wrote:

> So I run across this warning:
> [ 2724.595549] Debug: sleeping function called from invalid context  
> at arch/ppc/kernel/traps.c:528
> [ 2724.595563] in_atomic():0, irqs_disabled():1
> [ 2724.595568] Call trace:
> [ 2724.595573]  [c001fe54] __might_sleep+0xd4/0xf0
> [ 2724.595592]  [c0005ad8] program_check_exception+0xb8/0x520
> [ 2724.595606]  [c0004f04] ret_from_except_full+0x0/0x4c
>
> when I wrote a program calling illegal instructions. I then checked  
> out
> why this happened and the cause is the might_sleep() here:
>
> #define __get_user_check(x, ptr,  
> size)                                  \
> ({                                                                     
>   \
>         long __gu_err = - 
> EFAULT;                                        \
>         unsigned long  __gu_val =  
> 0;                                    \
>         const __typeof__(*(ptr)) __user *__gu_addr =  
> (ptr);             \
>         might_sleep 
> ();                                                  \
> ...
>
> I then figured I could use __copy_from_user_inatomic to access the
> instruction word to fix this (as far as I can tell only the warning is
> annoying, because the instruction already failed at that point so it
> must be in memory, right?)

Its highly likely, but not guaranteed on all systems.  In truth this  
is most likely a bug.

> But here's the actual question:
> static inline unsigned long __copy_from_user(void *to,
>                 const void __user *from, unsigned long size)
> {
>         might_sleep();
>         return __copy_from_user_inatomic(to, from, size);
> }
>
> Does that mean __copy_from_user_inatomic isn't actually valid to  
> call in
> atomic context? Or is this only so that kernel developers that use
> powerpc see the bugs their code would have on other platforms? The  
> magic
> in get_user_asm thoroughly confuses me.

What do you mean atomic context?  Not exactly sure what your asking  
about.

- kumar

Re: illegal instructions / irqs disabled warning

[Johannes Berg]

[-- Attachment #1: Type: text/plain, Size: 1407 bytes --]

On Wed, 2005-12-14 at 08:41 -0600, Kumar Gala wrote:

> Its highly likely, but not guaranteed on all systems.  In truth this  
> is most likely a bug.

Ok.

> > But here's the actual question:
> > static inline unsigned long __copy_from_user(void *to,
> >                 const void __user *from, unsigned long size)
> > {
> >         might_sleep();
> >         return __copy_from_user_inatomic(to, from, size);
> > }
> >
> > Does that mean __copy_from_user_inatomic isn't actually valid to  
> > call in
> > atomic context? Or is this only so that kernel developers that use
> > powerpc see the bugs their code would have on other platforms? The  
> > magic
> > in get_user_asm thoroughly confuses me.
> 
> What do you mean atomic context?  Not exactly sure what your asking  
> about.

Well, the above code loading the instruction that faulted is called from
the interrupt context where the kernel tries to fix it up. If that
instruction is not in memory for whatever reason, then it shouldn't try
to fix it up bug kill the program with SIGILL or SIGSEGV or whatever...
This is what I came from, and tried looking into how to implement that.
But then I ran across the code in __copy_from_user which just calls
__copy_from_user_inatomic so I'm wondering if it is actually possible to
safely copy data (the instruction) from user space in an interrupt.

Thanks,
johannes

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 832 bytes --]

Re: illegal instructions / irqs disabled warning

[Kumar Gala]

On Dec 14, 2005, at 8:49 AM, Johannes Berg wrote:

> On Wed, 2005-12-14 at 08:41 -0600, Kumar Gala wrote:
>
>> Its highly likely, but not guaranteed on all systems.  In truth this
>> is most likely a bug.
>
> Ok.
>
>>> But here's the actual question:
>>> static inline unsigned long __copy_from_user(void *to,
>>>                 const void __user *from, unsigned long size)
>>> {
>>>         might_sleep();
>>>         return __copy_from_user_inatomic(to, from, size);
>>> }
>>>
>>> Does that mean __copy_from_user_inatomic isn't actually valid to
>>> call in
>>> atomic context? Or is this only so that kernel developers that use
>>> powerpc see the bugs their code would have on other platforms? The
>>> magic
>>> in get_user_asm thoroughly confuses me.
>>
>> What do you mean atomic context?  Not exactly sure what your asking
>> about.
>
> Well, the above code loading the instruction that faulted is called  
> from
> the interrupt context where the kernel tries to fix it up. If that
> instruction is not in memory for whatever reason, then it shouldn't  
> try
> to fix it up bug kill the program with SIGILL or SIGSEGV or  
> whatever...
> This is what I came from, and tried looking into how to implement  
> that.
> But then I ran across the code in __copy_from_user which just calls
> __copy_from_user_inatomic so I'm wondering if it is actually  
> possible to
> safely copy data (the instruction) from user space in an interrupt.

It is, we just need to ensure that the user page we are copying from  
is in memory.

But, now that I go back and look at your log message, its odd. I  
didn't expect ProgramExceptions to be executed with the MSR[EE] = 0.  
Now, I'm wondering why that is.

- kumar

Re: illegal instructions / irqs disabled warning

[Johannes Berg]

[-- Attachment #1: Type: text/plain, Size: 635 bytes --]

On Wed, 2005-12-14 at 09:07 -0600, Kumar Gala wrote:

> It is, we just need to ensure that the user page we are copying from  
> is in memory.

So in the interrupt handler we need to ensure this and then call
__copy_from_user_inatomic instead?

> But, now that I go back and look at your log message, its odd. I  
> didn't expect ProgramExceptions to be executed with the MSR[EE] = 0.  
> Now, I'm wondering why that is.

Hm. I can tell you what I did: Something like 
  mtspr 940, r3

in userland :)
Not sure if it was 940, might have been one of the other registers
reserved for performance monitoring.

johannes

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 832 bytes --]

Re: illegal instructions / irqs disabled warning

[Andy Fleming]

On Dec 14, 2005, at 08:05, Johannes Berg wrote:
>
> Does that mean __copy_from_user_inatomic isn't actually valid to  
> call in
> atomic context? Or is this only so that kernel developers that use
> powerpc see the bugs their code would have on other platforms? The  
> magic
> in get_user_asm thoroughly confuses me.
>
> [Somehow I got sidetracked. I only wanted to look at implementing
> backtrace support for oprofile...]


What platform is this on?  The copy from user stuff is a bit out of  
my area of expertise, but I've got an implementation of backtrace for  
oprofile running on 85xx.  Mostly, it's just a hacked up version of  
the ppc64 patch that was floating around.

Andy

Re: illegal instructions / irqs disabled warning

[Paul Mackerras]

Johannes Berg writes:

> So I run across this warning: 
> [ 2724.595549] Debug: sleeping function called from invalid context at arch/ppc/kernel/traps.c:528
> [ 2724.595563] in_atomic():0, irqs_disabled():1

Hmmm.  What kernel version is this?  I assume you are using ARCH=ppc,
is that right?

> But here's the actual question:
> static inline unsigned long __copy_from_user(void *to,
>                 const void __user *from, unsigned long size)
> 
>         might_sleep();
>         return __copy_from_user_inatomic(to, from, size);
> 
> 
> Does that mean __copy_from_user_inatomic isn't actually valid to call in
> atomic context?

No, it doesn't mean that.  However, if the page isn't in memory,
__copy_from_user_inatomic won't try to bring it in if it has been
called from an atomic context (i.e. preempt or interrupts disabled).

The real question is why we have interrupts disabled in the illegal
instruction handler.  There was a reason why I wanted interrupts
disabled on entry to program_check_exception which I don't recall
clearly at the moment, but I think program_check_exception should be
doing a local_irq_enable() at some point (and it also shouldn't be
trying to emulate instructions for the kernel).

Paul.

Re: illegal instructions / irqs disabled warning

[Kumar Gala]

On Dec 14, 2005, at 4:07 PM, Paul Mackerras wrote:

> Johannes Berg writes:
>
>> So I run across this warning:
>> [ 2724.595549] Debug: sleeping function called from invalid  
>> context at arch/ppc/kernel/traps.c:528
>> [ 2724.595563] in_atomic():0, irqs_disabled():1
>
> Hmmm.  What kernel version is this?  I assume you are using ARCH=ppc,
> is that right?
>
>> But here's the actual question:
>> static inline unsigned long __copy_from_user(void *to,
>>                 const void __user *from, unsigned long size)
>>
>>         might_sleep();
>>         return __copy_from_user_inatomic(to, from, size);
>>
>>
>> Does that mean __copy_from_user_inatomic isn't actually valid to  
>> call in
>> atomic context?
>
> No, it doesn't mean that.  However, if the page isn't in memory,
> __copy_from_user_inatomic won't try to bring it in if it has been
> called from an atomic context (i.e. preempt or interrupts disabled).
>
> The real question is why we have interrupts disabled in the illegal
> instruction handler.  There was a reason why I wanted interrupts
> disabled on entry to program_check_exception which I don't recall
> clearly at the moment, but I think program_check_exception should be
> doing a local_irq_enable() at some point (and it also shouldn't be
> trying to emulate instructions for the kernel).

I'm glad I'm not the only one thinking that interrupts disabled in  
program_check_exception was a little odd.  It looks like  
AltiVecAssist is handling MSR[EE] properly.

The same issue existed in math-emu which is really just another case  
of emulation.

- kumar

Re: illegal instructions / irqs disabled warning

[Johannes Berg]

Andy Fleming wrote:

> What platform is this on?  The copy from user stuff is a bit out of
> my area of expertise, but I've got an implementation of backtrace for
> oprofile running on 85xx.  Mostly, it's just a hacked up version of
> the ppc64 patch that was floating around.

ppc32, compiling the kernel with ARCH=ppc still.

johannes

Re: illegal instructions / irqs disabled warning

[Johannes Berg]

Paul Mackerras wrote:
>> So I run across this warning:
>> [ 2724.595549] Debug: sleeping function called from invalid context at
>> arch/ppc/kernel/traps.c:528
>> [ 2724.595563] in_atomic():0, irqs_disabled():1
>
> Hmmm.  What kernel version is this?  I assume you are using ARCH=ppc,
> is that right?

2.6.15-rc5, IIRC (might have been rc1, I currently need to use both
kernels depending on what I want to do), and yes, ARCH=ppc.

> No, it doesn't mean that.  However, if the page isn't in memory,
> __copy_from_user_inatomic won't try to bring it in if it has been
> called from an atomic context (i.e. preempt or interrupts disabled).

Good, thanks. So essentially __copy_from_user has the same logic (since it
just calls __copy_from_user_inatomic) but calls might_sleep() to show the
developer what he's doing wrong although it won't actually hurt. Right?

> The real question is why we have interrupts disabled in the illegal
> instruction handler.  There was a reason why I wanted interrupts
> disabled on entry to program_check_exception which I don't recall
> clearly at the moment, but I think program_check_exception should be
> doing a local_irq_enable() at some point (and it also shouldn't be
> trying to emulate instructions for the kernel).

I don't have the code here right now but I did have a userspace program
that it tried to emulate (so I'm not sure if it would try to emulate
kernel instructions). And it wouldn't emulate that instruction, my program
died with SIGILL, but obviously it still needs to load it to see what kind
of instruction it was.

johannes

Re: illegal instructions / irqs disabled warning

[Kumar Gala]

On Dec 15, 2005, at 3:44 AM, Johannes Berg wrote:

> Paul Mackerras wrote:
>>> So I run across this warning:
>>> [ 2724.595549] Debug: sleeping function called from invalid  
>>> context at
>>> arch/ppc/kernel/traps.c:528
>>> [ 2724.595563] in_atomic():0, irqs_disabled():1
>>
>> Hmmm.  What kernel version is this?  I assume you are using ARCH=ppc,
>> is that right?
>
> 2.6.15-rc5, IIRC (might have been rc1, I currently need to use both
> kernels depending on what I want to do), and yes, ARCH=ppc.
>
>> No, it doesn't mean that.  However, if the page isn't in memory,
>> __copy_from_user_inatomic won't try to bring it in if it has been
>> called from an atomic context (i.e. preempt or interrupts disabled).
>
> Good, thanks. So essentially __copy_from_user has the same logic  
> (since it
> just calls __copy_from_user_inatomic) but calls might_sleep() to  
> show the
> developer what he's doing wrong although it won't actually hurt.  
> Right?
>
>> The real question is why we have interrupts disabled in the illegal
>> instruction handler.  There was a reason why I wanted interrupts
>> disabled on entry to program_check_exception which I don't recall
>> clearly at the moment, but I think program_check_exception should be
>> doing a local_irq_enable() at some point (and it also shouldn't be
>> trying to emulate instructions for the kernel).
>
> I don't have the code here right now but I did have a userspace  
> program
> that it tried to emulate (so I'm not sure if it would try to emulate
> kernel instructions). And it wouldn't emulate that instruction, my  
> program
> died with SIGILL, but obviously it still needs to load it to see  
> what kind
> of instruction it was.

Out of interest, is there a reason you dont handle the emulation in  
user space?

- kumar

Re: illegal instructions / irqs disabled warning

[Johannes Berg]

On Do, 2005-12-15 at 08:04 -0600, Kumar Gala wrote:

> Out of interest, is there a reason you dont handle the emulation in  
> user space?

I'm not even interested in emulating that at all, the kernel just
checked if it could emulate it (couldn't, so my program died with
SIGILL)...
To tell the truth, I was just testing what would happen and wrote a
buggy program.

johannes

Re: illegal instructions / irqs disabled warning

[Kumar Gala]

On Dec 15, 2005, at 8:10 AM, Johannes Berg wrote:

> On Do, 2005-12-15 at 08:04 -0600, Kumar Gala wrote:
>
>> Out of interest, is there a reason you dont handle the emulation in
>> user space?
>
> I'm not even interested in emulating that at all, the kernel just
> checked if it could emulate it (couldn't, so my program died with
> SIGILL)...
> To tell the truth, I was just testing what would happen and wrote a
> buggy program.

Ahh, you should check out LTP its got some tests like this which  
generate random instruction images and see what happens.  Helps  
finding kernel bugs.

- kumar