Bug 253278 - x11-servers/xorg-server: Lock file: Various fixes
Summary: x11-servers/xorg-server: Lock file: Various fixes
Status: In Progress
Alias: None
Product: Ports & Packages
Classification: Unclassified
Component: Individual Port(s) (show other bugs)
Version: Latest
Hardware: Any Any
: --- Affects Only Me
Assignee: freebsd-x11 (Nobody)
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2021-02-05 19:23 UTC by Olivier Certner
Modified: 2021-02-15 12:27 UTC (History)
1 user (show)

See Also:
bugzilla: maintainer-feedback? (x11)


Attachments
Patch against the ports tree (5.02 KB, patch)
2021-02-05 19:23 UTC, Olivier Certner
no flags Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Olivier Certner 2021-02-05 19:23:36 UTC
Created attachment 222190 [details]
Patch against the ports tree

Several fixes:

1. Create a lock file in the case of an explicitly requested display even if
"-displayfd" was specified. This is because, in this case, the server creation
process is essentially the same as when "-displayfd" is not specified. The only
difference with the latter case should be that Xorg outputs the passed display
to the display FD (only the display selection logic is bypassed).

2. Properly indicate an unexpected problem with link(2), instead of assuming
that a failure always means that the file indeed exists.

3. Workaround for what appears to be a FreeBSD bug (link returns EPERM when
hard linking a file whose permissions are the result of creating a file in a
directory with sticky bit, although creating a separate copy is perfectly
possible). Additional benefit: Simplifies the cumbersome logic, which on POSIX
systems is unnecessary IMHO (initial lock file creation with O_EXCL is enough
to ensure mutual exclusion).

Again I'm submitting this here, since upstream seems inactive.
Comment 1 Niclas Zeising freebsd_committer 2021-02-05 23:15:01 UTC
I am quite skeptical of pulling this in without it first being submitted and accepted upstream.
Can you explain exactly what problem you are trying to solve?
Comment 2 Olivier Certner 2021-02-08 16:39:44 UTC
Hi, sorry for not having stated the high-level use case here:
- Launch X with "startx", specifying '/usr/local/bin/Xorg' as the server.
E.g.: "startx -- /usr/local/bin/Xorg :1"
You'll get an immediate crash saying "Can't read lock file /tmp/.X1-lock". Although no lock file exists at all under '/tmp'.

The error comes from the fact that some lock file is prepared under another name (e.g., for display :1, under name '/tmp/.tX1-lock'), and this file is then hardlink to the final lock name using link(2), which improperly fails. 


The point of fixes 2 and 3 is to respectively better report on this problem and solve it. 3 is a simplification of the existing code (no functional change intended), which has the effect of working around a FreeBSD bug in link(2) that I'm going to submit as a separate bug with patch. Fix 1 is unrelated, done while I was looking at this code, and its usefulness is debatable, especially once bug #253277 is fixed.

Even with this patch, pursuing with:
"startx -- /usr/local/bin/Xorg :1"
still doesn't work, because Xorg must be run as root. Substitution with:
"startx -- /usr/local/bin/X :1"
or
"startx -- /usr/local/bin/Xorg.wrap :1"
makes it work, but then the fixes are not needed, since link(2) works.

Given all that, here is a path I find reasonable:
1. Focus should be #253277, this one is secondary.
2. I'll submit these fixes upstream.

Maybe should we keep this PR open and I'll link the upstream bug here. Or I/you can close it. Don't know what's the policy in such cases.

Thanks.