Re: 2.5.31: modules don't work at all

Ed Tomlinson
2002-08-20T23:31:16Z

Andrew Morton wrote:

> Skip Ford wrote:
>>
>> ...
>> > I already know that the error that trips insmod occurs at
>> > in modules.c, line 831, when qm_symbols gets an error from
>> > copy_to_user():
>> >
>> > for (; i < mod->nsyms ; ++i, ++s, vals += 2) {
>> > len = strlen(s->name)+1;
>> > if (len > bufsize)
>> > goto calc_space_needed;
>> >
>> > here------> if (copy_to_user(strings, s->name, len)
>> > || __put_user(s->value, vals+0)
>> > || __put_user(space, vals+1))
>> > return -EFAULT;
>> >
>> > strings += len;
>> > bufsize -= len;
>> > space += len;
>> > }
>> >
>> > The values of strings and s->name are similar in 2.5.30+preempt
>> > (works) and 2.5.31+preempt (does not work). strings is 0x08______, and
>> > s->name is 0xc0______.
>>
>> If I back out this change to arch/i386/mm/fault.c then modules
>> successfully load. I have no idea if backing it out causes other
>> problems though.
>>
>> diff -Nru a/arch/i386/mm/fault.c b/arch/i386/mm/fault.c
>> --- a/arch/i386/mm/fault.c Sat Aug 10 18:42:20 2002
>> +++ b/arch/i386/mm/fault.c Sat Aug 10 18:42:20 2002
>> @@ -181,10 +181,10 @@
>> info.si_code = SEGV_MAPERR;
>>
>> /*
>> - * If we're in an interrupt or have no user
>> - * context, we must not take the fault..
>> + * If we're in an interrupt, have no user context or are running
>> in an
>> + * atomic region then we must not take the fault..
>> */
>> - if (in_interrupt() || !mm)
>> + if (preempt_count() || !mm)
>> goto no_context;
>>
>> down_read(&mm->mmap_sem);
>>
>
> Yes, that's the problem. qm_symbols() is performing copy_to_user()
> inside lock_kernel() and that's an "atomic copy_to_user()" in 2.5.31.
> But only if preempt is selected. The copy_to_user() doesn't work.
>
> There's nothing illegal about copy_to_user() inside lock_kernel().
>
> Linus, we can back out the preempt_count() test in there and
> perform the atomic copy_*_user via a current->flags bit, or
> we can do something else?

I am still seeing this problem with the todays bk current, which includes
the above 'fix'...

Turning off preempt now.

Ed Tomlinson