Re: what's papered over by set_fs(USER_DS) in amd64 signal delivery?

[Brian Gerst]

On Sat, Sep 25, 2010 at 1:20 AM, Al Viro <viro@xxxxxxxxxxxxxxxxxx> wrote:
> On Fri, Sep 24, 2010 at 11:51:11PM -0400, Brian Gerst wrote:
>> > Again, I agree that it almost certainly can be dropped. ??I really wonder
>> > about the history, though. ??It predates git and bk by far (late 1996).
>> > Linus, do you have any recollection regarding that stuff?
>> >
>>
>> In the beginning, the i386 kernel used a non-flat segmented memory
>> layout. ÂUSER_[CD]S were 3GB segments at base 0, and KERNEL_[CD]S were
>> 1GB segments at base 3GB. ÂThis meant that the kernel could not access
>> userspace addresses without using a fs segment override (%fs was saved
>> in pt_regs, reloaded with USER_DS on kernel entry, and restored on
>> kernel exit). ÂYou had to reload %fs with KERNEL_DS for the *_user
>> functions to address the kernel segment.
>
> I know.
>
>> v2.1.2 introduced the modern flat memory layout with 4GB segments at
>> base 0. Â%fs no longer was used for userspace access, so it wasn't
>> saved in pt_regs or touched in any way until a task switch. ÂInstead
>> of the hardware enforcing the limit, the check was moved to software.
>
> Yes.
>
>> Originally the signal handler had to set regs->xfs = USER_DS so that
>> the signal handler had a known state when it ran. ÂThat had nothing to
>> do with the kernel's userspace access mechanism. ÂIt was converted to
>> do both the immediate reloading of the %fs register (since it was no
>> longer saved in pt_regs and wouldn't get restored on kernel exit), and
>> to a new set_fs(USER_DS) call which meant something completely
>> different. ÂThat is the origin of the code we are trying to remove
>> now.
>
> That still makes no sense. Â2.0 mechanism guaranteed that even if you forgot
> to restore %fs to USER_DS, you wouldn't leak that to userland. ÂBut this
> one didn't - each place like that became a roothole, no matter what you
> did on signal delivery. ÂSimply because there might have been no unblocked
> signals with userland handlers. ÂIOW, that set_fs() seems to have been
> useless from the day 1

That's what I was getting at. ÂThe code it replaced did something
totally different (setting up user-mode register state). ÂThe asm()
part of the replacement was the correct thing to do. ÂThe set_fs()
call was unnecessary.

> unless I'm missing something really subtle, like
> e.g. some processes deliberately running (in 2.0) with %fs set to something
> with lower limit, with signal handlers allowed to switch back to normal
> for duration. ÂAnd even that would've been broken, since there wouldn't be
> a matching set_fs() in sigreturn()...

If the user process used a different %fs it didn't matter to the
kernel, because %fs was saved to pt_regs and reloaded with USER_DS on
kernel entry.

--
Brian Gerst
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/