futex_init() vs. KERNEL_DS, was Re: Boot crash on 68030

futex_init() vs. KERNEL_DS, was Re: Boot crash on 68030

[Finn Thain]

On Tue, 10 Dec 2013, Andreas Schwab wrote:

> Geert Uytterhoeven <geert@linux-m68k.org> writes:
> 
> >> Data read fault at 0x00000000 in Super Data (pc=0x3afec) BAD KERNEL 
> >> BUSERR
> 
> I think futex_init needs to do set_fs(USER_DS), since futex is about 
> user data.

Rhetorical question: does it make sense to explicitly switch to USER_DS 
when we know that current->mm may be invalid at the time?

Given that the presence of USER_DS or KERNEL_DS at initcall-time varies 
between architectures, perhaps this question must be left to arch 
implementors.

Moreover, architectures that have no need for a run-time test for usable 
futex_value ops have no real interest in such questions. AFAICT, only mips 
needs the run-time test.

For every other arch the availability of futex_value ops is known at 
compile-time. We don't need to pursue universal answers to questions like 
these:

1) the correctness of switching to USER_DS with mm->current == NULL [1]

2) the correctness of USER_DS vs. KERNEL_DS for (all) initcalls [2]

3) the correctness of allowing faults when get_fs() == KERNEL_DS [3]

4) the proposition that "[futex_value ops] explicitly take a __user 
   pointer and are expected to do whatever is needed to perform the 
   operation"  ... even during kernel initialization. [4]

The discussion of these questions in various threads on various lists did 
not resolve anything. Again, this is probably because all of these 
questions are actually arch implementation issues.

So it seems to me that the solution to the futex_init() problem is to 
evaluate the futex_cmpxchg_enabled assignment at compile-time where 
possible, and use the run-time test only on mips. Thoughts?

Finn


[1] http://marc.info/?l=linux-next&m=138876015228975&w=2

[2] https://lkml.org/lkml/2014/1/15/476
    http://marc.info/?l=linux-kernel&m=138971456630101&w=2

[3] http://marc.info/?l=linux-next&m=138876367730190&w=2

[4] http://marc.info/?l=linux-kernel&m=138970641726884&w=2

futex_init() vs. KERNEL_DS, was Re: Boot crash on 68030

[Finn Thain]

On Tue, 10 Dec 2013, Andreas Schwab wrote:

> Geert Uytterhoeven <geert@linux-m68k.org> writes:
> 
> >> Data read fault at 0x00000000 in Super Data (pc=0x3afec) BAD KERNEL 
> >> BUSERR
> 
> I think futex_init needs to do set_fs(USER_DS), since futex is about 
> user data.

Rhetorical question: does it make sense to explicitly switch to USER_DS 
when we know that current->mm may be invalid at the time?

Given that the presence of USER_DS or KERNEL_DS at initcall-time varies 
between architectures, perhaps this question must be left to arch 
implementors.

Moreover, architectures that have no need for a run-time test for usable 
futex_value ops have no real interest in such questions. AFAICT, only mips 
needs the run-time test.

For every other arch the availability of futex_value ops is known at 
compile-time. We don't need to pursue universal answers to questions like 
these:

1) the correctness of switching to USER_DS with mm->current == NULL [1]

2) the correctness of USER_DS vs. KERNEL_DS for (all) initcalls [2]

3) the correctness of allowing faults when get_fs() == KERNEL_DS [3]

4) the proposition that "[futex_value ops] explicitly take a __user 
   pointer and are expected to do whatever is needed to perform the 
   operation"  ... even during kernel initialization. [4]

The discussion of these questions in various threads on various lists did 
not resolve anything. Again, this is probably because all of these 
questions are actually arch implementation issues.

So it seems to me that the solution to the futex_init() problem is to 
evaluate the futex_cmpxchg_enabled assignment at compile-time where 
possible, and use the run-time test only on mips. Thoughts?

Finn


[1] http://marc.info/?l=linux-next&m=138876015228975&w=2

[2] https://lkml.org/lkml/2014/1/15/476
    http://marc.info/?l=linux-kernel&m=138971456630101&w=2

[3] http://marc.info/?l=linux-next&m=138876367730190&w=2

[4] http://marc.info/?l=linux-kernel&m=138970641726884&w=2

Re: futex_init() vs. KERNEL_DS, was Re: Boot crash on 68030

[Heiko Carstens]

On Fri, Feb 28, 2014 at 02:12:22PM +1100, Finn Thain wrote:
> > >> Data read fault at 0x00000000 in Super Data (pc=0x3afec) BAD KERNEL 
> > >> BUSERR
> > 
> > I think futex_init needs to do set_fs(USER_DS), since futex is about 
> > user data.
> 
> Rhetorical question: does it make sense to explicitly switch to USER_DS 
> when we know that current->mm may be invalid at the time?
> 
> Given that the presence of USER_DS or KERNEL_DS at initcall-time varies 
> between architectures, perhaps this question must be left to arch 
> implementors.

All architectures start with KERNEL_DS, otherwise the first couple of system
calls within kernel_init() would all fail and the system wouldn't boot.

> Moreover, architectures that have no need for a run-time test for usable 
> futex_value ops have no real interest in such questions. AFAICT, only mips 
> needs the run-time test.
> 
> For every other arch the availability of futex_value ops is known at 
> compile-time. We don't need to pursue universal answers to questions like 
> these:
> 
> 1) the correctness of switching to USER_DS with mm->current == NULL [1]

I "fixed" s390, so it should survive an early switch to USER_DS and a
subsequent failing user space access now.

Anyway,

> The discussion of these questions in various threads on various lists did 
> not resolve anything. Again, this is probably because all of these 
> questions are actually arch implementation issues.
> 
> So it seems to me that the solution to the futex_init() problem is to 
> evaluate the futex_cmpxchg_enabled assignment at compile-time where 
> possible, and use the run-time test only on mips. Thoughts?

Such a patch exists already (bottom of message):

http://marc.info/?l=linux-next&m=138875880028607&w=2

Re: futex_init() vs. KERNEL_DS, was Re: Boot crash on 68030

[Heiko Carstens]

On Fri, Feb 28, 2014 at 02:12:22PM +1100, Finn Thain wrote:
> > >> Data read fault at 0x00000000 in Super Data (pc=0x3afec) BAD KERNEL 
> > >> BUSERR
> > 
> > I think futex_init needs to do set_fs(USER_DS), since futex is about 
> > user data.
> 
> Rhetorical question: does it make sense to explicitly switch to USER_DS 
> when we know that current->mm may be invalid at the time?
> 
> Given that the presence of USER_DS or KERNEL_DS at initcall-time varies 
> between architectures, perhaps this question must be left to arch 
> implementors.

All architectures start with KERNEL_DS, otherwise the first couple of system
calls within kernel_init() would all fail and the system wouldn't boot.

> Moreover, architectures that have no need for a run-time test for usable 
> futex_value ops have no real interest in such questions. AFAICT, only mips 
> needs the run-time test.
> 
> For every other arch the availability of futex_value ops is known at 
> compile-time. We don't need to pursue universal answers to questions like 
> these:
> 
> 1) the correctness of switching to USER_DS with mm->current == NULL [1]

I "fixed" s390, so it should survive an early switch to USER_DS and a
subsequent failing user space access now.

Anyway,

> The discussion of these questions in various threads on various lists did 
> not resolve anything. Again, this is probably because all of these 
> questions are actually arch implementation issues.
> 
> So it seems to me that the solution to the futex_init() problem is to 
> evaluate the futex_cmpxchg_enabled assignment at compile-time where 
> possible, and use the run-time test only on mips. Thoughts?

Such a patch exists already (bottom of message):

http://marc.info/?l=linux-next&m=138875880028607&w=2

Re: futex_init() vs. KERNEL_DS, was Re: Boot crash on 68030

[Finn Thain]

On Fri, 28 Feb 2014, Heiko Carstens wrote:

> On Fri, Feb 28, 2014 at 02:12:22PM +1100, Finn Thain wrote:
> > > > > Data read fault at 0x00000000 in Super Data (pc=0x3afec) BAD 
> > > > > KERNEL BUSERR
> > > 
> > > I think futex_init needs to do set_fs(USER_DS), since futex is about 
> > > user data.
> > 
> > Rhetorical question: does it make sense to explicitly switch to 
> > USER_DS when we know that current->mm may be invalid at the time?
> > 
> > Given that the presence of USER_DS or KERNEL_DS at initcall-time 
> > varies between architectures, perhaps this question must be left to 
> > arch implementors.
> 
> All architectures start with KERNEL_DS, otherwise the first couple of 
> system calls within kernel_init() would all fail and the system wouldn't 
> boot.

I was alluding to the assertion that "USER_DS is the normal operating 
state": https://lkml.org/lkml/2014/1/15/476

> 
> > Moreover, architectures that have no need for a run-time test for 
> > usable futex_value ops have no real interest in such questions. 
> > AFAICT, only mips needs the run-time test.
> > 
> > For every other arch the availability of futex_value ops is known at 
> > compile-time. We don't need to pursue universal answers to questions 
> > like these:
> > 
> > 1) the correctness of switching to USER_DS with mm->current == NULL
> 
> I "fixed" s390, so it should survive an early switch to USER_DS and a 
> subsequent failing user space access now.
> 
> Anyway,
> 
> > The discussion of these questions in various threads on various lists 
> > did not resolve anything. Again, this is probably because all of these 
> > questions are actually arch implementation issues.
> > 
> > So it seems to me that the solution to the futex_init() problem is to 
> > evaluate the futex_cmpxchg_enabled assignment at compile-time where 
> > possible, and use the run-time test only on mips. Thoughts?
> 
> Such a patch exists already (bottom of message):
> 
> http://marc.info/?l=linux-next&m=138875880028607&w=2
> 

Thanks. I gather that you don't need this patch on s390 any more. Though 
it does promise to save some cycles on any architecture that can optimize 
away all the "if (!futex_cmpxchg_enabled) return" tests.

With your patch, the nak'd patch* from Geert would be obsolete unless it 
was useful to mips (or deemed correct by fiat).

Finn


* http://marc.info/?l=linux-kernel&m=138675863423989&w=2