Page MenuHomeFreeBSD

Properly implement Linux32 set_thread_area()
Needs ReviewPublic

Authored by kib on Feb 10 2024, 7:13 AM.
Tags
None
Referenced Files
Unknown Object (File)
Mon, Apr 8, 11:12 PM
Unknown Object (File)
Mon, Apr 8, 1:36 PM
Unknown Object (File)
Mon, Mar 25, 10:39 PM
Unknown Object (File)
Mar 7 2024, 4:50 PM
Unknown Object (File)
Mar 2 2024, 4:45 PM
Unknown Object (File)
Feb 19 2024, 9:07 AM
Unknown Object (File)
Feb 19 2024, 9:07 AM
Unknown Object (File)
Feb 19 2024, 9:06 AM

Details

Reviewers
brooks
dchagin
Summary

Also add get_thread_area()

Diff Detail

Repository
rG FreeBSD src repository
Lint
Lint Skipped
Unit
Tests Skipped

Event Timeline

kib requested review of this revision.Feb 10 2024, 7:13 AM

Drop committed/unrelated changes.
Implement get_thread_area()

iwtcex_gmail.com added a subscriber: iwtcex_gmail.com.

First, let's reference the PR in question: https://bugs.freebsd.org/bugzilla/show_bug.cgi?id=247327.

Second, I'm not aware of any apps that actually depend on set_thread_area's ability to set descriptor attributes other than the base address. (glibc/musl/Wine don't really need that.) My issue with the existing implementation is mostly that it doesn't handle the entry_number = -1 case properly. I do appreciate that this implementation avoids the issue with FSBase MSR value being written into GUFS32_SEL's descriptor, which indeed is something that clashes with Wine's assumptions (with their new wow64 stuff, to be precise). But that also could be addressed more directly.

Third, there is no point to using Linux's descriptor indices 6, 7, 8 (or GUDATA_SEL for that matter) anywhere in this code unless we intend to place the dedicated TLS descriptor entries in those positions, which we won't. That was blatantly incorrect and still is.

First, let's reference the PR in question: https://bugs.freebsd.org/bugzilla/show_bug.cgi?id=247327.

Second, I'm not aware of any apps that actually depend on set_thread_area's ability to set descriptor attributes other than the base address. (glibc/musl/Wine don't really need that.) My issue with the existing implementation is mostly that it doesn't handle the entry_number = -1 case properly. I do appreciate that this implementation avoids the issue with FSBase MSR value being written into GUFS32_SEL's descriptor, which indeed is something that clashes with Wine's assumptions (with their new wow64 stuff, to be precise). But that also could be addressed more directly.

I am implementing the syscall, not just scratching your itch.

Third, there is no point to using Linux's descriptor indices 6, 7, 8 (or GUDATA_SEL for that matter) anywhere in this code unless we intend to place the dedicated TLS descriptor entries in those positions, which we won't. That was blatantly incorrect and still is.

I tend to agree with the statement that 6/GUDATA_SEL indexes handling is a gross hack, but also I am not sure that dropping it is wise. It is possible to pass either 6 directly, or current %gs/%fs selector (with masked RPL) on Linux, and I want to give such consumers a chance to work on FreeBSD. For me, this is in fact allowed, from reading the set_thread_area(2) Linux man page.

I am most interested in whether this patch fixes original issue(s) with Wine.

Forgot to write that linux_set_cloned_tls needs an adjustment to work with these changes.

I tend to agree with the statement that 6/GUDATA_SEL indexes handling is a gross hack, but also I am not sure that dropping it is wise.

The hack only appears to work because glibc is careful enough to not make any assumptions about the number: https://sourceware.org/git/?p=glibc.git;a=blob;f=sysdeps/i386/nptl/tls.h;h=84d148ce950f98e983d545f27982677db098e623;hb=HEAD#l189. That was the case even before set_thread_area was implemented in Linuxulator as far as I can tell.

It is possible to pass either 6 directly, or current %gs/%fs selector (with masked RPL) on Linux, and I want to give such consumers a chance to work on FreeBSD. For me, this is in fact allowed, from reading the set_thread_area(2) Linux man page.

Yes indeed it's possible on Linux. Such application will eventually crash once it tries to use a segment register with the corresponding selector, so the best we can do would be returning EINVAL and printing a diagnostic warning. Anything else would just obscure the cause of the crash.

kib edited the summary of this revision. (Show Details)

Handle clone.
Since clone is changed to set %gs to first linux tls segment, drop SEGUGS cases in syscalls.