lkml.org 
[lkml]   [2007]   [Jul]   [17]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
Patch in this message
/
Date
From
SubjectRe: Q: a bogus set_fs(USER_DS) in setup_frame/setup_rt_frame ?
On 07/17, Linus Torvalds wrote:
>
> On Tue, 17 Jul 2007, Oleg Nesterov wrote:
> >
> > I am really puzzled by set_fs(USER_DS) in setup_frame/setup_rt_frame.
> >
> > How is it possible that current->addr_limit != USER_DS ? If this _is_
> > possible, how can can we trust the result of access_ok() above?
>
> Heh. I think it's entirely historical.
>
> Please realize that the whole reason that function is called "set_fs()" is
> that it literally used to set the %fs segment register, not
> "->addr_limit".
>
> So I think the "set_fs(USER_DS)" is there _only_ to match the other
>
> regs->xds = __USER_DS;
> regs->xes = __USER_DS;
> regs->xss = __USER_DS;
> regs->xcs = __USER_CS;
>
> things, and never mattered.

Thanks!

> And now it matters even less, and has been
> copied to all other architectures where it is just totally insane.

Yes.

Also, sparc does something strange with do_sigaltstack(). It first copies
stack_t to the local variable, then sets KERNEL_DS to access it from
do_sigaltstack().

IOW, what's wrong with the patch below? Why should we ignore errors other
than -EFAULT?

Oleg.

--- arch/sparc64/kernel/signal.c~ 2007-05-21 13:57:49.000000000 +0400
+++ arch/sparc64/kernel/signal.c 2007-07-17 21:04:32.000000000 +0400
@@ -291,7 +291,6 @@ void do_rt_sigreturn(struct pt_regs *reg
__siginfo_fpu_t __user *fpu_save;
mm_segment_t old_fs;
sigset_t set;
- stack_t st;
int err;

/* Always make any pending restarted system calls return -EINTR */
@@ -327,20 +326,13 @@ void do_rt_sigreturn(struct pt_regs *reg
err |= restore_fpu_state(regs, &sf->fpu_state);

err |= __copy_from_user(&set, &sf->mask, sizeof(sigset_t));
- err |= __copy_from_user(&st, &sf->stack, sizeof(stack_t));
-
+ err |= do_sigaltstack(&st->stack, NULL, (unsigned long)sf);
+
if (err)
goto segv;
-
+
regs->tpc = tpc;
regs->tnpc = tnpc;
-
- /* It is more difficult to avoid calling this function than to
- call it and ignore errors. */
- old_fs = get_fs();
- set_fs(KERNEL_DS);
- do_sigaltstack((const stack_t __user *) &st, NULL, (unsigned long)sf);
- set_fs(old_fs);

sigdelsetmask(&set, ~_BLOCKABLE);
spin_lock_irq(&current->sighand->siglock);
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/

\
 
 \ /
  Last update: 2007-07-17 19:17    [W:0.040 / U:3.352 seconds]
©2003-2018 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site