Commit Graph

726 Commits

Author SHA1 Message Date
Matthieu Herrb
d7ac755f0b Use timingsafe_memcmp() to compare MIT-MAGIC-COOKIES CVE-2017-2624
Provide the function definition for systems that don't have it.

Signed-off-by: Matthieu Herrb <matthieu@herrb.eu>
Reviewed-by: Alan Coopersmith <alan.coopersmith@oracle.com>
2017-02-28 13:59:14 -05:00
Peter Hutterer
1b12249fd6 os: log a bug whenever WriteToClient is called from the input thread
The input thread should generate events, not send them. Make it easier to
find the instances where it's doing so.

Signed-off-by: Peter Hutterer <peter.hutterer@who-t.net>
Tested-by: Olivier Fourdan <ofourdan@redhat.com>
Reviewed-by: Olivier Fourdan <ofourdan@redhat.com>
2017-02-27 09:25:08 +10:00
Chris Wilson
acdb5bf2de os: Fix iteration over busfaults
Fixes a regression from

commit 41da295eb5
Author: Keith Packard <keithp@keithp.com>
Date:   Sun Nov 3 13:12:40 2013 -0800

    Trap SIGBUS to handle truncated shared memory segments

that causes the SIGBUS handler to fail to chain up correctly and
corrupts nearby memory instead.

Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Reviewed-by: Peter Hutterer <peter.hutterer@who-t.net>
Signed-off-by: Peter Hutterer <peter.hutterer@who-t.net>
2017-02-23 09:20:48 +10:00
Chris Wilson
dcb63deb1b inputthread: Initialise inputThreadInfo->changed before use
==8734== Thread 2 InputThread:
==8734== Conditional jump or move depends on uninitialised value(s)
==8734==    at 0x2FDB05: InputThreadDoWork (inputthread.c:333)
==8734==    by 0x6924423: start_thread (pthread_create.c:333)
==8734==    by 0x6C229BE: clone (clone.S:105)

Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Reviewed-by: Peter Hutterer <peter.hutterer@who-t.net>
Signed-off-by: Peter Hutterer <peter.hutterer@who-t.net>
2017-02-23 09:11:13 +10:00
Olivier Fourdan
23d85c5442 os: remove unused define MAX_TIMES_PER
Remove leftover from commit e10ba9e, MAX_TIMES_PER is not used anymore.

Signed-off-by: Olivier Fourdan <ofourdan@redhat.com>
Reviewed-by: Peter Hutterer <peter.hutterer@who-t.net>
Signed-off-by: Peter Hutterer <peter.hutterer@who-t.net>
2017-02-20 15:48:01 +10:00
Alan Coopersmith
4c00609c37 DetermineClientCmd: try using /proc/pid/cmdline on Solaris too
Solaris 11.3.5 introduced support for /proc/pid/cmdline, so try it
first, and if we can't open it, then fallback to /proc/pid/psinfo
as we did before.

Reviewed-by: Adam Jackson <ajax@redhat.com>
Signed-off-by: Alan Coopersmith <alan.coopersmith@oracle.com>
2017-02-14 11:33:47 -05:00
Keith Packard
785053d033 AttendClient of grab-pervious client must queue to saved_ready_clients [v2]
A client which is attended while a grab is blocking execution of its
requests needs to be placed in the saved_ready_clients list so that it
will get scheduled once the grab terminates. Otherwise, if the client
never sends another request, there is no way for it to be placed in
the ready_clients list.

v2: Wrap comment above mark_client_saved_ready.
    Remove test for OS_COMM_IGNORED which will always be true.

Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=99333
Reviewed-by: Adam Jackson <ajax@redhat.com>
Signed-off-by: Keith Packard <keithp@keithp.com>
2017-01-11 14:31:43 -05:00
Peter Hutterer
1b42f9505f os: return 0 from check_timers if we touched any of them
Fixes a regression introduced in 0b2f30834b. If a driver posts input
events during a timer function (wacom and synaptics do this during tap
timeouts), ProcessInputEvents() is not called for these events. There are no
new events on any fds, so the events just sit in the queue waiting for
something else to happen.

Fix this by simply returning 0 from check_timers if we ran at least one of
them or reset them all. This way the callers ospoll_wait will exit and
continue with normal processing.

Signed-off-by: Peter Hutterer <peter.hutterer@who-t.net>
Reviewed-by: Keith Packard <keithp@keithp.com>
2017-01-02 14:11:11 -05:00
Mihail Konev
5b74e260e0 os,dix: Depend custom libs on libs, not objects
The custom os/os.O library reuses *.o files of os/libos.la.

The current rule assumes automake puts all the objects into per-target
am__*_la_OBJECTS variable.  At least with AC_REPLACE_FUNCS, this no
longer holds (as wanted objects are put into LTLIBOBJS instead).

Depend on automake's result, the *.la library instead, to express demand
of any its dependencies being built.

Should be fixing randomly occuring "undefined reference to `strlcpy'"
errors when linking Xvfb and other DDX-es that could use os.O.

Reviewed-by: Adam Jackson <ajax@redhat.com>
Signed-off-by: Mihail Konev <k.mvc@ya.ru>
2017-01-02 12:45:28 -05:00
Keith Packard
9ed5b26354 os: Recompute whether any clients are ready after ProcessWorkQueue() (bug 98030)
If a work proc wakes up a sleeping client and it is ready to execute,
we need to re-compute the local 'are_ready' value before deciding
what timeout value to use in WaitForSomething.

Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=98030
Signed-off-by: Keith Packard <keithp@keithp.com>
Reviewed-by: Hans de Goede <hdegoede@redhat.com>
2016-10-28 09:00:46 -07:00
Peter Hutterer
5cb3283386 inputthread: On Linux leave the main thread's name as-is
On Linux, setting the main thread's name changes the program name
(/proc/self/comm). Setting it to MainThread breaks scripts that rely on
the command name, e.g. ps -C Xorg.

Signed-off-by: Peter Hutterer <peter.hutterer@who-t.net>
Signed-off-by: Hans de Goede <hdegoede@redhat.com>
2016-10-27 09:53:01 +10:00
Mihail Konev
5dcb0666b8 os/inputthread: Ensure pollfd refreshing
When putting a device node into a poll-request list, do not overwrite a
"please-remove" element with the same fd, so that a closed device file
is ospoll_remove'd prior to being ospoll_add'ed.

Before, the opposite order was possible, resulting in ospoll_add
considering the newly opened file being already polled, should it have a
fd for which the "please-remove" has not been procesed yet. In this
case, no further events would be seen from the device.

Signed-off-by: Mihail Konev <k.mvc@ya.ru>
Regressed-in: 52d6a1e832
Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=97880
Patchwork: https://patchwork.freedesktop.org/patch/113763/
Hit-and-Reduced-by: Hans de Goede <hdegoede@redhat.com>
Reviewed-and-Reduced-by: Peter Hutterer <peter.hutterer@who-t.net>
Reviewed-by: Keith Packard <keithp@keithp.com>
Signed-off-by: Keith Packard <keithp@keithp.com>
2016-10-15 13:39:26 -07:00
Jeremy Huddleston Sequoia
2740dc1937 dix: Silence TSan warnings when checking for pending input
V2: Moves InputCheckPending() into dix.h

Bumps required version of xproto to 7.0.30

==================
WARNING: ThreadSanitizer: data race (pid=4943)
  Read of size 4 at 0x00010c4e3854 by thread T8:
    #0 WaitForSomething WaitFor.c:237 (X11.bin+0x00010049216c)
    #1 Dispatch dispatch.c:413 (X11.bin+0x000100352ed9)
    #2 dix_main main.c:287 (X11.bin+0x00010036e894)
    #3 server_thread quartzStartup.c:66 (X11.bin+0x000100039e63)

  Previous write of size 4 at 0x00010c4e3854 by thread T12 (mutexes: write M856, write M1976):
    #0 mieqEnqueue mieq.c:263 (X11.bin+0x000100448d14)
    #1 DarwinSendDDXEvent darwinEvents.c:641 (X11.bin+0x000100033613)
    #2 DarwinProcessFDAdditionQueue_thread darwinEvents.c:338 (X11.bin+0x000100032039)

  Location is global 'miEventQueue' at 0x00010c4e3850 (X11.bin+0x0001005ab854)

  Mutex M856 (0x00010c4c8c80) created at:
    #0 pthread_mutex_lock <null>:144 (libclang_rt.tsan_osx_dynamic.dylib+0x0000000321fe)
    #1 DarwinListenOnOpenFD darwinEvents.c:300 (X11.bin+0x000100031607)
    #2 socket_handoff bundle-main.c:288 (X11.bin+0x000100002b40)
    #3 __do_request_fd_handoff_socket_block_invoke bundle-main.c:379 (X11.bin+0x0001000029ba)
    #4 __tsan::invoke_and_release_block(void*) <null>:144 (libclang_rt.tsan_osx_dynamic.dylib+0x00000005d97b)
    #5 _dispatch_client_callout <null>:33 (libdispatch.dylib+0x0000000020ef)

  Mutex M1976 (0x00010c4e3d68) created at:
    #0 pthread_mutex_init <null>:144 (libclang_rt.tsan_osx_dynamic.dylib+0x0000000253c3)
    #1 input_lock inputthread.c:103 (X11.bin+0x00010049fd10)
    #2 TimerSet WaitFor.c:343 (X11.bin+0x0001004926c2)
    #3 RootlessQueueRedisplay rootlessScreen.c:594 (X11.bin+0x000100065d7f)
    #4 RootlessInstallColormap rootlessScreen.c:514 (X11.bin+0x000100069f1a)
    #5 miSpriteInstallColormap misprite.c:562 (X11.bin+0x000100467095)
    #6 miCreateDefColormap micmap.c:270 (X11.bin+0x000100440399)
    #7 DarwinScreenInit darwin.c:285 (X11.bin+0x0001000303bb)
    #8 AddScreen dispatch.c:3908 (X11.bin+0x00010036c417)
    #9 InitOutput darwin.c:671 (X11.bin+0x00010002fdeb)
    #10 dix_main main.c:197 (X11.bin+0x00010036e228)
    #11 server_thread quartzStartup.c:66 (X11.bin+0x000100039e63)

  Thread T8 (tid=4198779, running) created by main thread at:
    #0 pthread_create <null>:144 (libclang_rt.tsan_osx_dynamic.dylib+0x000000024490)
    #1 create_thread quartzStartup.c:78 (X11.bin+0x000100039dad)
    #2 QuartzInitServer quartzStartup.c:95 (X11.bin+0x000100039c16)
    #3 X11ApplicationMain X11Application.m:1238 (X11.bin+0x00010001cde4)
    #4 X11ControllerMain X11Controller.m:984 (X11.bin+0x00010002a642)
    #5 server_main quartzStartup.c:136 (X11.bin+0x00010003a03b)
    #6 do_start_x11_server bundle-main.c:436 (X11.bin+0x000100002eb5)
    #7 _Xstart_x11_server mach_startupServer.c:189 (X11.bin+0x000100004e99)
    #8 mach_startup_server mach_startupServer.c:399 (X11.bin+0x000100005734)
    #9 mach_msg_server mach_msg.c:563 (libsystem_kernel.dylib+0x000000012186)
    #10 start <null>:29 (libdyld.dylib+0x000000005254)

  Thread T12 (tid=4198797, running) created by thread T8 at:
    #0 pthread_create <null>:144 (libclang_rt.tsan_osx_dynamic.dylib+0x000000024490)
    #1 create_thread darwinEvents.c:121 (X11.bin+0x000100031ecf)
    #2 DarwinEQInit darwinEvents.c:365 (X11.bin+0x000100031860)
    #3 InitInput darwin.c:571 (X11.bin+0x00010002ea09)
    #4 dix_main main.c:261 (X11.bin+0x00010036e7ce)
    #5 server_thread quartzStartup.c:66 (X11.bin+0x000100039e63)

SUMMARY: ThreadSanitizer: data race WaitFor.c:237 in WaitForSomething
==================
==================
WARNING: ThreadSanitizer: data race (pid=22841)
  Write of size 4 at 0x000105bbd864 by main thread (mutexes: write M1945):
    #0 mieqEnqueue mieq.c:263 (X11.bin+0x000100448cf4)
    #1 DarwinSendDDXEvent darwinEvents.c:642 (X11.bin+0x000100033693)
    #2 -[X11Controller set_window_menu:] X11Controller.m:275 (X11.bin+0x0001000222fd)
    #3 -[X11Application set_window_menu:] X11Application.m:486 (X11.bin+0x000100018b44)
    #4 -[X11Application handleMachMessage:] X11Application.m:177 (X11.bin+0x000100016678)
    #5 __NSFireMachPort <null>:69 (Foundation+0x00000009b62b)
    #6 X11ControllerMain X11Controller.m:984 (X11.bin+0x00010002a5f2)
    #7 server_main quartzStartup.c:136 (X11.bin+0x000100039ffb)
    #8 do_start_x11_server bundle-main.c:436 (X11.bin+0x000100002e65)
    #9 _Xstart_x11_server mach_startupServer.c:189 (X11.bin+0x000100004e49)
    #10 mach_startup_server mach_startupServer.c:399 (X11.bin+0x0001000056e4)
    #11 mach_msg_server mach_msg.c:563 (libsystem_kernel.dylib+0x000000012186)
    #12 start <null>:29 (libdyld.dylib+0x000000005254)

  Previous read of size 4 at 0x000105bbd864 by thread T7:
    #0 Dispatch dispatch.c:434 (X11.bin+0x000100352fc8)
    #1 dix_main main.c:287 (X11.bin+0x00010036e874)
    #2 server_thread quartzStartup.c:66 (X11.bin+0x000100039e23)

  Location is global 'miEventQueue' at 0x000105bbd860 (X11.bin+0x0001005ab864)

  Mutex M1945 (0x000105bbdd78) created at:
    #0 pthread_mutex_init <null>:144 (libclang_rt.tsan_osx_dynamic.dylib+0x0000000253c3)
    #1 input_lock inputthread.c:103 (X11.bin+0x00010049fd10)
    #2 TimerSet WaitFor.c:348 (X11.bin+0x0001004926c2)
    #3 RootlessQueueRedisplay rootlessScreen.c:594 (X11.bin+0x000100065d3f)
    #4 RootlessInstallColormap rootlessScreen.c:514 (X11.bin+0x000100069eda)
    #5 miSpriteInstallColormap misprite.c:562 (X11.bin+0x000100467075)
    #6 miCreateDefColormap micmap.c:270 (X11.bin+0x000100440379)
    #7 DarwinScreenInit darwin.c:285 (X11.bin+0x00010003036b)
    #8 AddScreen dispatch.c:3914 (X11.bin+0x00010036c3f7)
    #9 InitOutput darwin.c:671 (X11.bin+0x00010002fd9b)
    #10 dix_main main.c:197 (X11.bin+0x00010036e208)
    #11 server_thread quartzStartup.c:66 (X11.bin+0x000100039e23)

  Thread T7 (tid=4257217, running) created by main thread at:
    #0 pthread_create <null>:144 (libclang_rt.tsan_osx_dynamic.dylib+0x000000024490)
    #1 create_thread quartzStartup.c:78 (X11.bin+0x000100039d6d)
    #2 QuartzInitServer quartzStartup.c:95 (X11.bin+0x000100039bd6)
    #3 X11ApplicationMain X11Application.m:1238 (X11.bin+0x00010001cd94)
    #4 X11ControllerMain X11Controller.m:984 (X11.bin+0x00010002a5f2)
    #5 server_main quartzStartup.c:136 (X11.bin+0x000100039ffb)
    #6 do_start_x11_server bundle-main.c:436 (X11.bin+0x000100002e65)
    #7 _Xstart_x11_server mach_startupServer.c:189 (X11.bin+0x000100004e49)
    #8 mach_startup_server mach_startupServer.c:399 (X11.bin+0x0001000056e4)
    #9 mach_msg_server mach_msg.c:563 (libsystem_kernel.dylib+0x000000012186)
    #10 start <null>:29 (libdyld.dylib+0x000000005254)

SUMMARY: ThreadSanitizer: data race mieq.c:263 in mieqEnqueue
==================

Signed-off-by: Jeremy Huddleston Sequoia <jeremyhu@apple.com>
Reviewed-by: Keith Packard <keithp@keithp.com>
2016-09-22 14:55:01 -07:00
Keith Packard
7ff8a74945 os: Clear saved poll events in listen so that edge triggering works
When a client is marked as write blocked, clear any old 'write ready'
bit in the osfds structure so that a new indication of write ready
(which is marked as edge trigggered) will trigger the callback.

Signed-off-by: Keith Packard <keithp@keithp.com>
Reviewed-by: Jeremy Huddleston Sequoia <jeremyhu@apple.com>
Tested-by: Jeremy Huddleston Sequoia <jeremyhu@apple.com>
Tested-by: Matthieu Herrb <matthieu@herrb.eu>
2016-09-22 14:55:01 -07:00
Keith Packard
1d365f1ffe os: Ready clients with pending output aren't flushed, so set NewOutputPending
When a client with pending output is ready (has request data pending),
FlushAllOutput will skip it to get all of the requests processed
before sending any queued output. That means FlushAllOutput is going
to return with some output pending to a client which isn't known to be
write blocked. And that means NewOutputPending needs to be set so that
FlushAllOutput will get called again to actually go flush this client.

It might be interesting to try just flushing the client to send any
queued data along the way. This patch just restores the server
behavior to what it was before the ospoll changes.

Signed-off-by: Keith Packard <keithp@keithp.com>
Reviewed-by: Jeremy Huddleston Sequoia <jeremyhu@apple.com>
Tested-by: Jeremy Huddleston Sequoia <jeremyhu@apple.com>
Tested-by: Matthieu Herrb <matthieu@herrb.eu>
2016-09-22 14:55:00 -07:00
Keith Packard
86038dfd42 os: Add 'ospoll.h' to Makefile.am
Distribute this source file.

Signed-off-by: Keith Packard <keithp@keithp.com>
2016-09-16 10:57:23 -07:00
Alan Coopersmith
c4799f186b os: Use pthread_setname_np to set thread names if available
Autoconf logic borrowed from glib

Signed-off-by: Alan Coopersmith <alan.coopersmith@oracle.com>
Reviewed-by: Jeremy Huddleston Sequoia <jeremyhu@apple.com>
Tested-by: Jeremy Huddleston Sequoia <jeremyhu@apple.com>
2016-09-13 16:55:26 -04:00
Alan Coopersmith
75c1d04650 os: OsSigHandler should not show rtld errors for unrelated signals
If RTLD_DI_SETSIGNAL is set to let us turn runtime linker/loader errors
into catchable signals, then we should only show the errors when catching
that signal, instead of tossing out red herrings to distract people with
unrelated crashes long after their last failed symbol lookup (especially
when using drivers built to support multiple API's by checking which
symbols are available before calling them).

Signed-off-by: Alan Coopersmith <alan.coopersmith@oracle.com>
Reviewed-by: Keith Packard <keithp@keithp.com>
Reviewed-by: Aaron Plattner <aplattner@nvidia.com>
2016-09-13 16:16:40 -04:00
Jeremy Huddleston Sequoia
065eb66124 os/inputthread: Fix setting of cloexec on file descriptors
O_CLOEXEC is not a file bit.  It is not setable with F_SETFL.  One must
use it when calling open(2).  To set it cloexec on an existing fd,
F_SETFD and FD_CLOEXEC must be used.

This also fixes a build failure regression on configurations that don't
have O_CLOEXEC defined.

cf: http://pubs.opengroup.org/onlinepubs/9699919799/functions/fcntl.html
Regressed-in: 30ac756798

Signed-off-by: Jeremy Huddleston Sequoia <jeremyhu@apple.com>
Reviewed-by: Julien Cristau <jcristau@debian.org>
2016-09-13 16:10:56 -04:00
Michel Dänzer
b380f3ac51 dix: Pass ClientPtr to FlushCallback
This change has two effects:

1. Only calls FlushCallbacks when we're actually flushing data to a
   client. The unnecessary FlushCallback calls could cause significant
   performance degradation with compositing, which is significantly
   reduced even without any driver changes.

2. By passing the ClientPtr to FlushCallbacks, drivers can completely
   eliminate unnecessary flushing of GPU commands by keeping track of
   whether we're flushing any XDamageNotify events to the client for
   which the corresponding rendering commands haven't been flushed to
   the GPU yet.

Reviewed-by: Adam Jackson <ajax@redha.com>
Signed-off-by: Michel Dänzer <michel.daenzer@amd.com>
2016-08-15 14:02:48 -04:00
Keith Packard
67fc5d68f9 os: Abandon loop after poll call when array of fds has changed
If a file descriptor is added or removed from an ospoll callback, then
the arrays containing file descriptor information will have all of
their indices changed, so the loop state is no longer consistent. Just
bail out and let the caller come back around to try again.

Signed-off-by: Keith Packard <keithp@keithp.com>
Reviewed-by: Peter Hutterer <peter.hutterer@who-t.net>
2016-08-14 22:52:33 -07:00
Keith Packard
7d6fffb601 os: Delay freeing of epoll_wait return pointers until epoll is idle.
Instead of freeing the struct ospollfd elements when the fd is
removed by the user, delay that until epoll is idle so that we are
sure no epoll_event structures could contain the stale pointer. This
handles cases where an fd is removed from the ospoll callback
interface, and also in case the OS keeps stale pointers around after
the call to epoll_ctl with EPOLL_CTL_DEL.

Signed-off-by: Keith Packard <keithp@keithp.com>
Reviewed-by: Peter Hutterer <peter.hutterer@who-t.net>
2016-08-14 22:52:22 -07:00
Keith Packard
bf31d6f43e os: Allow re-registering fd with InputThreadRegisterDev
Calling InputThreadRegisterDev twice with the same fd should replace
the existing function and args instead of creating a new entry with
the same fd.

Signed-off-by: Keith Packard <keithp@keithp.com>
Reviewed-by: Peter Hutterer <peter.hutterer@who-t.net>
Signed-off-by: Peter Hutterer <peter.hutterer@who-t.net>
2016-08-12 10:33:21 +10:00
Peter Hutterer
2df2815d6a os: append, rather than prepend, any new input thread fds
xf86AddEnabledDevice() prepends the new fd to the list,
xf86RemoveEnabledDevice() then searches for a matching fd and removes that
entry. If this is done for the same fd (and since we lose all information but
the actual fd) we usually unregister virtual devices in reverse order, causing
a dereference of already released memory.

Case in point:
- the wacom driver calls xf86AddEnabledDevice() once for the physical device,
  then multiple times for the virtual subdevices
- when the physical device is unplugged, the driver calls
  xf86RemoveEnabledDevice() for the physical device
- all we have is the fd, so we end up removing the last virtual device from
  the fd set
- xf86DeleteInput() frees the physical device's pInfo
- the fd goes crazy with ENODEV, but a read_input() now passes the already
  freed pInfo for the physical device
- boom

Fix this by appending to the fd list to provide bug-for-bug compatibility with
the old SIGIO code. This needs to be fixed in the driver, but meanwhile not
crashing the server provides for better user experience.

Signed-off-by: Peter Hutterer <peter.hutterer@who-t.net>
Reviewed-by: Keith Packard <keithp@keithp.com>
2016-08-11 11:41:51 +10:00
Keith Packard
0b2f30834b os: Clean up WaitFor.c
Do all timer stuff before blocking, avoiding a bunch of duplicate code
and merge common code in WaitForSomething.

The WaitForSomething changes need a bit of explanation to show that
the new code is effectively equivalent to the old. Eliding error
checking and trivial bits we've got:

Before:

	if (ready clients)
		timeout = 0
	else
		compute timeout
	i = poll
	if (i <= 0) {
		if (ready clients)
			return TRUE;
		if (input)
			return FALSE;
		if (any ready timers) {
			run timers
			return FALSE;
		}
	} else {
		if (input)
			return FALSE;
		if (any ready timers) {
			run timers
			return FALSE;
		}
		if (ready clients)
			return TRUE;
	}

After:

	if (ready clients)
		timeout = 0;
	else
		compute timeout
		run_timers
	poll

	if (input)
		return FALSE;

	if (ready clients)
		return TRUE;

The old code would return TRUE if there were ready clients and input
pending. Dispatch would then schedule that ready client, but before
processing any requests, it would notice that there was input pending
and go process it. The new code just checks for input first, which is
effectively the same.

If the poll timed out and there weren't clients ready, then timers
would get run.

If the poll didn't time out, then timers would get run, even if there
were clients now ready. Now, if the timeout interval was zero, that
means that the timers must have been ready *before* poll was
invoked. In this case, we should simply run the timers before calling
poll -- no sense calling poll just to discard any data that it
generates.

If the timeout interval was non-zero, and poll didn't timeout, then
either there aren't any timers to run, or we got a surprise and hit a
timer exactly as a client became ready to run. This is the one case
where the new code is different from the old; the new code delays the
timer call until the next time WaitForSomething is called.

Signed-off-by: Keith Packard <keithp@keithp.com>
Reviewed-by: Adam Jackson <ajax@redhat.com>
2016-07-21 15:04:47 -04:00
Keith Packard
2ab8b1dcd3 os: Use xorg_list for struct _OsTimerRec
No sense having an open-coded linked list here, plus the doubly linked
list is more efficient

Signed-off-by: Keith Packard <keithp@keithp.com>
Reviewed-by: Adam Jackson <ajax@redhat.com>
2016-07-21 15:04:47 -04:00
Keith Packard
50779c494d os: Remove CheckConnections
poll provides per-fd notification of failure, so we don't need
CheckConnections anymore.

Signed-off-by: Keith Packard <keithp@keithp.com>
Reviewed-by: Adam Jackson <ajax@redhat.com>
2016-07-21 15:04:47 -04:00
Keith Packard
f0275b1e5a os: Leave stdin and stdout open
There's no reason to close these now that we don't care what file
descriptors we use.

Signed-off-by: Keith Packard <keithp@keithp.com>
Reviewed-by: Adam Jackson <ajax@redhat.com>
2016-07-21 15:04:47 -04:00
Keith Packard
8217c29d2d Allow 1024 and 2048 for LimitClients
There's no reason not to offer ridiculous numbers of clients; only a
few static data structures are arrays of this length.

Signed-off-by: Keith Packard <keithp@keithp.com>
Reviewed-by: Adam Jackson <ajax@redhat.com>
2016-07-21 15:04:47 -04:00
Keith Packard
e0edb963fe os: eliminate fd value limits for clients
With no code depending on the range of file descriptors, checking
for that can be eliminated.

Signed-off-by: Keith Packard <keithp@keithp.com>
Reviewed-by: Adam Jackson <ajax@redhat.com>
2016-07-21 15:04:47 -04:00
Keith Packard
30bc0732f9 os: Use ospoll for input thread [v2]
Replace use of select(2) to avoid fd limits. Note that
InputThreadFillPipe used select as well, but none of the files passed
were non-blocking, so there was no need for that code at all.

v2: Keep ospoll API usage single threaded to avoid re-entrancy issues

Signed-off-by: Keith Packard <keithp@keithp.com>
Reviewed-by: Adam Jackson <ajax@redhat.com>
2016-07-21 15:04:47 -04:00
Keith Packard
f993091e7d os: Switch server to poll(2) [v3]
Eliminates all of the fd_set mangling in the server main thread

v2: Listen for POLLOUT while writes are blocked.

v3: Only mark client not ready on EAGAIN return from read

Signed-off-by: Keith Packard <keithp@keithp.com>
Reviewed-by: Adam Jackson <ajax@redhat.com>
2016-07-21 15:04:47 -04:00
Keith Packard
8f1edf4bd3 dix: Use list for ready clients
This converts the dispatch loop into using a list of ready clients
instead of an array. This changes the WaitForSomething API so that it
notifies DIX when a client becomes ready to read, instead of returning
the set of ready clients.

Signed-off-by: Keith Packard <keithp@keithp.com>
Reviewed-by: Adam Jackson <ajax@redhat.com>
2016-07-21 15:04:47 -04:00
Keith Packard
d6eff3c31e os: Add ospoll interface [v2]
This provides a wrapper around poll or epoll providing a
callback-based interface for monitoring activity on a large set of
file descriptors.

v2: use xserver_poll API instead of poll. Don't use WSAPoll as
    that is broken.

Signed-off-by: Keith Packard <keithp@keithp.com>
Reviewed-by: Adam Jackson <ajax@redhat.com>
2016-07-21 15:04:47 -04:00
Brian M. Clapper
711c36558f os: Add poll emulation for mingw [v2]
v2: rename as 'xserver_poll' to avoid potential library name
    collisions. Provide 'xserver_poll.h' which uses the system
    poll where available and falls back to this emulation otherwise.
    Autodetects when this is required, building the emulation only
    then

Source: https://github.com/bmc/poll
Signed-off-by: Adam Jackson <ajax@redhat.com>
2016-07-19 11:08:21 -07:00
Keith Packard
8d3a368d89 os: InputThreadFillPipe doesn't need select or poll
The file descriptors passed to InputThreadFillPipe are always
blocking, so there's no need to use Select (or poll).

Signed-off-by: Keith Packard <keithp@keithp.com>
Reviewed-by: Adam Jackson <ajax@redhat.com>
2016-07-18 15:27:51 -04:00
Keith Packard
ef7ddbe242 os: Move ETEST macro from io.c to osdep.h
This lets other code share this functionality

Signed-off-by: Keith Packard <keithp@keithp.com>
Reviewed-by: Adam Jackson <ajax@redhat.com>
2016-07-18 15:27:51 -04:00
Keith Packard
e6636b4383 os: Compute timeout in milliseconds instead of struct timeval
The timeout resolution offered in the AdjustWaitForDelay call is
only milliseconds, so passing around the timeout as a pointer to a
struct timeval is not helpful. Doing everything in milliseconds up to
the point of the select call simplifies the code without affecting
functionality at all.

Signed-off-by: Keith Packard <keithp@keithp.com>
Reviewed-by: Adam Jackson <ajax@redhat.com>
2016-07-18 15:27:51 -04:00
Keith Packard
7762a602c1 dix/os: Merge priority computation into SmartScheduleClient
Instead of having scheduling done in two places (one in
WaitForSomething, and the other in SmartScheduleClient), just stick
all of the scheduling in SmartScheduleClient.

Signed-off-by: Keith Packard <keithp@keithp.com>
Reviewed-by: Adam Jackson <ajax@redhat.com>
2016-07-18 15:27:51 -04:00
Keith Packard
be5a513fee Remove AddEnabledDevice and AddGeneralSocket APIs
All uses of these interfaces should instead be using the NotifyFd API
instead.

Signed-off-by: Keith Packard <keithp@keithp.com>
Reviewed-by: Adam Jackson <ajax@redhat.com>
2016-07-18 15:27:51 -04:00
Keith Packard
9d15912aa4 Remove fd_set from Block/Wakeup handler API
This removes the last uses of fd_set from the server interfaces
outside of the OS layer itself.

Signed-off-by: Keith Packard <keithp@keithp.com>
Reviewed-by: Adam Jackson <ajax@redhat.com>
2016-07-18 15:27:51 -04:00
Keith Packard
c3fea428ae os: Use NotifyFd for ErrorConnMax
Instead of open-coding a single FD wait, use NotifyFd to wait for the
FD to become readable before returning the error message.

Signed-off-by: Keith Packard <keithp@keithp.com>
Reviewed-by: Adam Jackson <ajax@redhat.com>
2016-07-18 15:25:59 -04:00
Keith Packard
05a793f5b3 dix: Switch to the libXfont2 API (v2)
This new libXfont API eliminates exposing internal X server symbols to
the font library, replacing those with a struct full of the entire API
needed to use that library.

v2: Use libXfont2 instead of libXfont_2

Signed-off-by: Keith Packard <keithp@keithp.com>
Reviewed-by: Adam Jackson <ajax@redhat.com>
2016-07-18 15:25:59 -04:00
Adam Jackson
111a045dcf Revert "XDMCP: For IPv6 add link local addresses to the end of the list"
This reverts commit fdd448cd39.
2016-06-21 11:45:17 -04:00
Reinhard Max
fdd448cd39 XDMCP: For IPv6 add link local addresses to the end of the list
For link local addresses the XDMCP server would need to either know the
interface thru a scope identifier or try all available interfaces.  If
they don't this address will fail in which case the XDMCP server could
still try the other addresses passed - however some only try the first
address and then give up.

Even if this seems to be the wrong place to fix this it seems to be
easier than fixing all display servers.

[ajax: Cleaned up commit message]

Reviewed-by: Adam Jackson <ajax@redhat.com>
2016-06-21 11:43:09 -04:00
Adam Jackson
6178b1c91c dix: Use OsSignal() not signal()
As the man page for the latter states:

    The effects of signal() in a multithreaded process are unspecified.

We already have an interface to call sigaction() instead, use it.

Signed-off-by: Adam Jackson <ajax@redhat.com>
Reviewed-by: Keith Packard <keithp@keithp.com>
2016-06-08 11:36:31 -04:00
Keith Packard
8174daa6bd os: Do timers under input lock, not blocked signals
Timer processing can happen on either the main thread or the input
thread. As a result, it must be done under the input lock.

Signals are unrelated to timers now that SIGIO isn't used for input
processing, so stop blocking signals while processing timers.

Signed-off-by: Keith Packard <keithp@keithp.com>
Reviewed-by: Peter Hutterer <peter.hutterer@who-t.net>
2016-06-07 16:32:15 -07:00
Keith Packard
3735ab965a Merge remote-tracking branch 'daenzer/for-master' 2016-06-02 07:47:16 -07:00
Keith Packard
ce65463374 os: Initialize NotifyFds earlier in startup
If the server calls AbortServer during the first-time initialization
(which can happen if you start the server on an already using
DISPLAY), then the dbus code will shut down and call the notify fd
interface. If the notify fd list hasn't been initialized, the server
will crash.

Signed-off-by: Keith Packard <keithp@keithp.com>
Reviewed-by: Peter Hutterer <peter.hutterer@who-t.net>
2016-06-01 10:31:52 -07:00
Keith Packard
f0756793e4 os: Lock input while messing with input device list
The list of input devices may be changed by hotplugging while the
server is active, and those changes may come from either the main
thread or the input thread. That means the list of input devices needs
to be protected by a mutex.

This prevents input drivers from receiving I/O ready callbacks after
removing a device.

Signed-off-by: Keith Packard <keithp@keithp.com>
Reviewed-by: Peter Hutterer <peter.hutterer@who-t.net>
2016-06-01 10:31:52 -07:00
Jonas Ådahl
a779fda224 xwayland: Use the CLOCK_MONOTONIC clock
By default the X server will try CLOCK_MONOTONIC_COARSE before
CLOCK_MONOTONIC, while A Wayland compositor may only support getting
their timestamps from the CLOCK_MONOTONIC clock. This causes various
issues since it may happen that a timestamp from CLOCK_MONOTONIC
retrieved before a sending an X request will still be "later" than the
timestamp the X server than gets after receiving the request, due to the
fact that CLOCK_MONOTONIC_COARSE has a lower resolution.

To avoid these issues, make Xwayland always use CLOCK_MONOTONIC, so
that it becomes possible for Wayland compositor only supporting
CLOCK_MONOTONIC and X server to use the same clock.

Signed-off-by: Jonas Ådahl <jadahl@gmail.com>
Acked-by: Daniel Stone <daniels@collabora.com>
Tested-by: Olivier Fourdan <ofourdan@redhat.com>
Signed-off-by: Peter Hutterer <peter.hutterer@who-t.net>
2016-05-31 08:44:38 +10:00
Michel Dänzer
e156c0ccb5 os: Use strtok instead of xstrtokenize in ComputeLocalClient
Fixes leaking the memory pointed to by the members of the array returned
by xstrtokenize.

Reviewed-by: Adam Jackson <ajax@redhat.com>
2016-05-30 17:51:10 +09:00
Keith Packard
06bdc3bc1a os: fix input_mutex_count off-by-one in input_force_unlock
input_force_unlock was mis-using input_mutex_lock and leaving it set
to -1. As this is  executed from OsInit at each server generation, on
the second time through, the mutex would be left locked (!) due to the
trylock call. This caused input to fail after the first server reset.

Signed-off-by: Keith Packard <keithp@keithp.com>
Reviewed-by: Peter Hutterer <peter.hutterer@who-t.net>
2016-05-29 19:21:15 -07:00
Keith Packard
0d16a0c3b9 os: Increase default client buffer to 16kB
This matches a change made in xcb and improves performance for a small
increase in memory usage.

Signed-off-by: Keith Packard <keithp@keithp.com>
Reviewed-by: Hans de Goede <hdegoede@redhat.com>
2016-05-27 10:34:33 -07:00
Keith Packard
30ac756798 Create a threaded mechanism for input [v7]
The current SIGIO signal handler method, used at generation of input events,
has a bunch of oddities. This patch introduces an alternative way using a
thread, which is used to select() all input device file descriptors.

A mutex was used to control the access to input structures by the main and input
threads. Two pipes to emit alert events (such hotplug ones) and guarantee the
proper communication between them was also used.

Co-authored-by: Fernando Carrijo <fcarrijo@freedesktop.org>
Signed-off-by: Tiago Vignatti <tiago.vignatti@nokia.com>

v2: Fix non-Xorg link. Enable where supported by default.

    This also splits out the actual enabling of input threads to
    DDX-specific patches which follow

v3: Make the input lock recursive

v4: Use regular RECURSIVE_MUTEXes instead of rolling our own
    Respect the --disable-input-thread configuration option by
    providing stubs that expose the same API/ABI.

    Respond to style comments from Peter Hutterer.

v5: use __func__ in inputthread debug and error mesages.

    Respond to style comments from Peter Hutterer.

v6: use AX_PTHREAD instead of inlining pthread tests.

    Suggested by Emil Velikov <emil.l.velikov@gmail.com>

v7: Use pthread_sigmask instead of sigprocmask when using threads

    Suggested by Adam Jackson <ajax@redhat.com>

Signed-off-by: Adam Jackson <ajax@redhat.com>
Signed-off-by: Keith Packard <keithp@keithp.com>
Reviewed-by: Adam Jackson <ajax@redhat.com>
2016-05-26 16:07:54 -07:00
Keith Packard
6a5a4e6037 Remove SIGIO support for input [v5]
This removes all of the SIGIO handling support used for input
throughout the X server, preparing the way for using threads for input
handling instead.

Places calling OsBlockSIGIO and OsReleaseSIGIO are marked with calls
to stub functions input_lock/input_unlock so that we don't lose this
information.

xfree86 SIGIO support is reworked to use internal versions of
OsBlockSIGIO and OsReleaseSIGIO.

v2: Don't change locking order (Peter Hutterer)
v3: Comment weird && FALSE in xf86Helper.c
    Leave errno save/restore in xf86ReadInput
    Squash with stub adding patch (Peter Hutterer)
v4: Leave UseSIGIO config parameter so that
    existing config files don't break (Peter Hutterer)
v5: Split a couple of independent patch bits out
    of kinput.c (Peter Hutterer)

Signed-off-by: Keith Packard <keithp@keithp.com>
Reviewed-by: Peter Hutterer <peter.hutterer@who-t.net>
2016-05-26 16:07:54 -07:00
Adam Jackson
adefbaee49 os: Treat ssh as a non-local client (v4)
By the time we get to ComputeLocalClient, we've already done
NextAvailableClient → ReserveClientIds → DetermineClientCmd (assuming
we're built with #define CLIENTIDS), so we can look up the name of the
client process and refuse to treat ssh's X forwarding as if it were
local.

v2: (Michel Dänzer)
    * Only match "ssh" itself, not other executable names starting with
      that prefix.
    * Ignore executable path for the match.
v3: (Michel Dänzer)
    * Use GetClientCmdName (Mark Kettenis)
    * Perform check on Windows as well, but only ignore path on Cygwin
      (Martin Peres, Emil Velikov, Jon Turney)
v4: (Michel Dänzer)
    * Cut of any colon and whatever comes after it. (Adam Jackson)
    * Add bugzilla reference.

Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=93261

Signed-off-by: Adam Jackson <ajax@redhat.com>
Signed-off-by: Michel Dänzer <michel.daenzer@amd.com>
2016-03-28 12:41:53 -04:00
Jon TURNEY
30b7d7995e Fix build on Cygwin by ensuring WIN32 check triggers only on MinGW
The type of fd_mask was changed in Cygwin 2.4.0 headers from 'long' to
'unsigned long'.  This exposes an existing problem with winauth.c, which
includes Xwindows.h (which includes windows.h, which defines WIN32),
before including osdep.h, which causes the now conflicting definition of
fd_mask in osdep.h to be exposed:

In file included from ../os/osdep.h:198:18: error: conflicting types for
‘fd_mask’ typedef long int fd_mask; /usr/include/sys/select.h:46:23:
note: previous declaration of ‘fd_mask’ was here typedef unsigned long
fd_mask;

Adjust the include guards in osdep.h to make sure we only use WIN32
guarded code when not compiling for Cygwin (i.e. WIN32 && !__CYGWIN__)

This isn't a very elegant, but unfortunately appears to be the best
solution, since it doesn't seem to be possible to write the test in a
positive form.

Future work: Should also audit of all the other uses of WIN32 in
xserver, and make sure they are correct.

Reviewed-by: Adam Jackson <ajax@redhat.com>
Signed-off-by: Jon Turney <jon.turney@dronecode.org.uk>
2016-02-22 14:26:38 -05:00
Adam Jackson
b3e9c534e2 os: unifdef STREAMSCONN
Removed from xtrans in 2012, and never wired up in the modular build
anyway.

Signed-off-by: Adam Jackson <ajax@redhat.com>
Reviewed-by: Alan Coopersmith <alan.coopersmith@oracle.com>
2016-02-08 21:49:56 -05:00
Adam Jackson
6dcb73375e os: Failure to remove a non-existent log file is not an error
Signed-off-by: Adam Jackson <ajax@redhat.com>
Reviewed-by: Alan Coopersmith <alan.coopersmith@oracle.com>
2016-01-06 12:23:31 -05:00
Alan Coopersmith
ba71b69f94 Avoid segfault in CloseWellKnownConnections when using -displayfd
When -displayfd is looping through the possible display ids to use,
if it can't open all the listening sockets for one (say when :0 is
already in use), it calls CloseWellKnownConnections to close all
the ListenTransConns entries before the point that ListenTransFds
was allocated & initialized, so CloseWellKnownConnections would
segfault trying to read entries from a NULL ListenTransFds pointer.

Introduced by commit 7b02f0b8

Signed-off-by: Alan Coopersmith <alan.coopersmith@oracle.com>
Reviewed-by: Keith Packard <keithp@keithp.com>
2016-01-05 12:09:18 -08:00
Alan Coopersmith
edcb6426f2 Use unique logfile names when starting server with -displayfd
Fixes https://bugs.freedesktop.org/show_bug.cgi?id=93212

Previously all X servers started with -displayfd would overwrite
Xorg.0.log - now a temporary name of Xorg.pid-<pid>.log is used
until after -displayfd finds an open display - then it is renamed
to the traditional Xorg.<display>.log name.

Reviewed-by: Adam Jackson <ajax@redhat.com>
Signed-off-by: Alan Coopersmith <alan.coopersmith@oracle.com>
2016-01-05 14:51:16 -05:00
Keith Packard
7b02f0b87e os: Use NotifyFd interface for listen descriptors
Replace the custom path for dealing with new incoming connections with
the general-purpose NotifyFd API.

Reviewed-by: Adam Jackson <ajax@redhat.com>
Signed-off-by: Keith Packard <keithp@keithp.com>
2015-12-01 13:56:15 -05:00
Keith Packard
49c0f2413d os/xdmcp: Replace xdmcp block/wakeup handlers with timer and NotifyFd
This removes the block and wakeup handlers and replaces them with a
combination of a NotifyFd callback and timers.

Reviewed-by: Adam Jackson <ajax@redhat.com>
Signed-off-by: Keith Packard <keithp@keithp.com>
2015-12-01 13:56:10 -05:00
Keith Packard
4020aacd1f os: Implement support for NotifyFd X_NOTIFY_WRITE
This adds the ability to be notified when a file descriptor is
available for writing.

Reviewed-by: Adam Jackson <ajax@redhat.com>
Signed-off-by: Keith Packard <keithp@keithp.com>
2015-12-01 13:55:07 -05:00
Keith Packard
0c41b7af4a os: Add NotifyFd interfaces
This provides a callback-based interface to monitor file
descriptors beyond the usual client and device interfaces.

Modules within the server using file descriptors for reading and/or
writing can call

    Bool SetNotifyFd(int fd, NotifyFdProcPtr notify_fd, int mask, void *data);

mask can be any combination of X_NOTIFY_READ and X_NOTIFY_WRITE.

When 'fd' becomes readable or writable, the notify_fd function will be
called with the 'fd', the ready conditions and 'data' values as arguments,

When the module no longer needs to monitor the fd, it will call

    void RemoveNotifyFd(int fd);

RemoveNotifyFd may be called from the notify function.

Reviewed-by: Adam Jackson <ajax@redhat.com>
Signed-off-by: Keith Packard <keithp@keithp.com>
2015-12-01 13:55:03 -05:00
Keith Packard
e10ba9e4b5 Remove non-smart scheduler. Don't require setitimer.
This allows the server to call GetTimeInMillis() after each request is
processed to avoid needing setitimer. -dumbSched now turns off the
setitimer.

Reviewed-by: Adam Jackson <ajax@redhat.com>
Signed-off-by: Keith Packard <keithp@keithp.com>
2015-12-01 13:54:54 -05:00
Richard PALO
e6b106715f Replace 'sun' with '__sun'
Globally replace #ifdef and #if defined usage of 'sun' with '__sun'
such that strict ISO compiler modes such as -ansi or -std=c99 can be used.

Signed-off-by: Richard PALO <richard@NetBSD.org>
Reviewed-by: Alan Coopersmith <alan.coopersmith@oracle.com>
2015-11-30 11:51:22 -05:00
Martin Peres
50c1671647 os: make sure the clientsWritable fd_set is initialized before use
In WaitForSomething(), the fd_set clientsWritable may be used
unitialized when the boolean AnyClientsWriteBlocked is set in the
WakeupHandler(). This leads to a crash in FlushAllOutput() after
x11proto's commit 2c94cdb453bc641246cc8b9a876da9799bee1ce7.

The problem did not manifest before because both the XFD_SIZE and the
maximum number of clients were set to 256. As the connectionTranslation
table was initalized for the 256 clients to 0, the test on the index not
being 0 was aborting before dereferencing the client #0.

As of commit 2c94cdb453bc641246cc8b9a876da9799bee1ce7 in x11proto, the
XFD_SIZE got bumped to 512. This lead the OutputPending fd_set to have
any fd above 256 to be uninitialized which in turns lead to reading an
index after the end of the ConnectionTranslation table. This index would
then be used to find the client corresponding to the fd marked as
pending writes and would also result to an out-of-bound access which
would usually be the fatal one.

Fix this by zeroing the clientsWritable fd_set at the beginning of
WaitForSomething(). In this case, the bottom part of the loop, which
would indirectly call FlushAllOutput, will not do any work but the next
call to select will result in the execution of the right codepath. This
is exactly what we want because we need to know the writable clients
before handling them. In the end, it also makes sure that the fds above
MaxClient are initialized, preventing the crash in FlushAllOutput().

Thanks to everyone involved in tracking this one down!

Reported-by: Karol Herbst <freedesktop@karolherbst.de>
Reported-by: Tobias Klausmann <tobias.klausmann@mni.thm.de>
Signed-off-by: Martin Peres <martin.peres@linux.intel.com>
Tested-by: Tobias Klausmann <tobias.klausmann@mni.thm.de>
Tested-by: Martin Peres <martin.peres@linux.intel.com>
Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=91316
Cc: Ilia Mirkin  <imirkin@alum.mit.edu>
Cc: Olivier Fourdan <ofourdan@redhat.com
Cc: Adam Jackson <ajax@redhat.com>
Cc: Alan Coopersmith <alan.coopersmith@oracle.com
Cc: Chris Wilson <chris@chris-wilson.co.uk>
Reviewed-by: Alan Coopersmith <alan.coopersmith@oracle.com>
2015-10-27 16:11:32 -04:00
Jeremy Huddleston
ec6294116c osinit: Silence -Wunused-variable warnings
osinit.c:161:24: warning: unused variable 'devnull' [-Wunused-variable,Unused Entity Issue]
    static const char *devnull = "/dev/null";
                       ^
osinit.c:162:10: warning: unused variable 'fname' [-Wunused-variable,Unused Entity Issue]
    char fname[PATH_MAX];
         ^

Reviewed-by: Adam Jackson <ajax@redhat.com>
Signed-off-by: Jeremy Huddleston Sequoia <jeremyhu@apple.com>
2015-10-19 11:52:05 -04:00
Jeremy Huddleston
9a2a05a9a7 xdmcp: Declare XdmcpFatal _X_NORETURN
xdmcp.c:1404:1: warning: function 'XdmcpFatal' could be declared with attribute 'noreturn'
[-Wmissing-noreturn,Semantic Issue]

Signed-off-by: Jeremy Huddleston Sequoia <jeremyhu@apple.com>
Reviewed-by: Alan Coopersmith <alan.coopersmith@oracle.com>
2015-10-19 11:49:54 -04:00
Jeremy Huddleston
85eb90ea45 xdmauth: Correct miscall of abs() to instrad call labs()
xdmauth.c:230:13: warning: absolute value function 'abs' given an argument of type 'long' but has parameter of
type
'int'
      which may cause truncation of value [-Wabsolute-value,Semantic Issue]
        if (abs(now - client->time) > TwentyFiveMinutes) {
            ^
xdmauth.c:230:13: note: use function 'labs' instead [Semantic Issue]
        if (abs(now - client->time) > TwentyFiveMinutes) {
            ^~~
            labs
xdmauth.c:302:9: warning: absolute value function 'abs' given an argument of type 'long' but has parameter of type
'int' which
      may cause truncation of value [-Wabsolute-value,Semantic Issue]
    if (abs(client->time - now) > TwentyMinutes) {
        ^
xdmauth.c:302:9: note: use function 'labs' instead [Semantic Issue]
    if (abs(client->time - now) > TwentyMinutes) {
        ^~~
        labs

Signed-off-by: Jeremy Huddleston Sequoia <jeremyhu@apple.com>
Reviewed-by: Alan Coopersmith <alan.coopersmith@oracle.com>
2015-10-19 11:49:53 -04:00
Keith Packard
6c2c6fb5a7 Xext, os: Remove OS-internal usages within XACE and XSELinux
These extensions were accessing internal OS functions and
structures. Expose the necessary functionality to them and remove
their use of osdep.h

Reviewed-by: Alan Coopersmith <alan.coopersmith@oracle.com>
Signed-off-by: Keith Packard <keithp@keithp.com>
2015-09-24 13:38:24 -04:00
Keith Packard
db1089eafc os/xdmcp: Just send XDMCP keepalive packets once every three minutes
There was a complicated scheme to increase the time between keepalives
from 3 minutes up to as much as 24 hours in an attempt to reduce
network traffic from idle X terminals. X terminals receiving X
traffic, or receiving user input would use the 3 minute value; X
terminals without any network traffic would use a longer value.

However, this was actually broken -- any activity in the X server,
either client requests or user input, would end up resetting the
keepalive timeout, so a user mashing on the keyboard would never
discover that the XDMCP master had disappeared and have the session
terminated, which was precisely the design goal of the XDMCP keepalive
mechanism.

Instead of attempting to fix this, accept the cost of a pair of XDMCP
packets once every three minutes and just perform keepalives
regularly.

This will also make reworking the block and wakeup handler APIs to
eliminate select masks easier.

Reviewed-by: Adam Jackson <ajax@redhat.com>
Signed-off-by: Keith Packard <keithp@keithp.com>
2015-09-21 15:02:34 -04:00
Keith Packard
a3a4029133 os/xdmcp: Remove dead 'restart' code
The X server used to wait for the user to hit a key or move the mouse
before restarting the session after a keepalive failure. This,
presumably, was to avoid having the X server continuously spew XDMCP
protocol on the network while the XDM server was dead.

Switching into this state was removed from the server some time before
XFree86 4.3.99.16, so the remaining bits of code have been dead for
over a decade, and no-one ever noticed.

Reviewed-by: Adam Jackson <ajax@redhat.com>
Signed-off-by: Keith Packard <keithp@keithp.com>
2015-09-21 15:02:24 -04:00
Jon TURNEY
cdd1d58159 mingw: Fix NO_LOCAL_CLIENT_CRED build
Commit 4b4b9086 "os: support new implicit local user access mode [CVE-2015-3164
2/3]" carefully places the relevant code it adds under !NO_LOCAL_CLIENT_CRED,
but unfortunately doesn't notice that NO_LOCAL_CLIENT_CRED is defined as a
side-effect in the middle of GetLocalClientCreds(), so many of these checks
precede its definition.

Move the check if NO_LOCAL_CLIENT_CRED should be defined to configure.ac, so it
always occurs before it's first use.

v2:
Move check to configure.ac

v3:
Use AC_CACHE_CHECK and name cache varaible appropriately

[ajax: Massaged commit message]

Signed-off-by: Jon TURNEY <jon.turney@dronecode.org.uk>
Reviewed-by: Ray Strode <rstrode@redhat.com>
2015-09-21 13:07:02 -04:00
Daniel Drake
1f915e8b52 Keep SIGALRM restart flag after Popen
Commit 94ab7455 added SA_RESTART to the SIGALRM handler.  However, the
Popen code tears down and recreates the SIGALRM handler via OsSignal(),
and this flag is dropped at this time.

Clean the code to use just a single codepath for creating this signal
handler, always applying SA_RESTART.

[ajax: Fixed commit id]

Reviewed-by: Adam Jackson <ajax@redhat.com>
Signed-off-by: Daniel Drake <drake@endlessm.com>
2015-09-21 11:06:39 -04:00
Daniel Drake
94ab7455ab Allow system call restarts upon signal interruption
The X server frequently deals with SIGIO and SIGALRM interruptions.
If process execution is inside certain blocking system calls
when these signals arrive, e.g. with the kernel blocked on
a contended semaphore, the system calls will be interrupted.

Some system calls are automatically restartable (the kernel re-executes
them with the same parameters once the signal handler returns) but
only if the signal handler allows it.

Set SA_RESTART on the signal handlers to enable this convenient
behaviour.

Reviewed-by: Adam Jackson <ajax@redhat.com>
Signed-off-by: Daniel Drake <drake@endlessm.com>
2015-09-21 10:41:46 -04:00
Olivier Fourdan
d206c240c0 configurable maximum number of clients
Make the maximum number of clients user configurable, either from the command
line or from xorg.conf

This patch works by using the MAXCLIENTS (raised to 512) as the maximum
allowed number of clients, but allowing the actual limit to be set by the
user to a lower value (keeping the default of 256).

There is a limit size of 29 bits to be used to store both the client ID and
the X resources ID, so by reducing the number of clients allowed to connect to
the X server, the user can increase the number of X resources per client or
vice-versa.

Parts of this patch are based on a similar patch from Adam Jackson
<ajax@redhat.com>

This now requires at least xproto 7.0.28

Signed-off-by: Adam Jackson <ajax@redhat.com>
Signed-off-by: Olivier Fourdan <ofourdan@redhat.com>
Reviewed-by: Adam Jackson <ajax@redhat.com>
Signed-off-by: Keith Packard <keithp@keithp.com>
2015-08-24 00:00:18 -07:00
Adam Jackson
b51f7f8582 dix: Unexport various implementation details
Acked-by: Chris Wilson <chris@chris-wilson.co.uk>
Signed-off-by: Adam Jackson <ajax@redhat.com>
2015-07-08 16:40:57 -04:00
Vicente Olivert Riera
baa50f60ac backtrace.c: Fix word cast to a pointer
backtrace.c uses a word size provided by libunwind. In some
architectures like MIPS, libunwind makes that word size 64-bit for all
variants of the architecture.

In the lines #90 and #98, backtrace.c tries to do a cast to a pointer,
which fails in all MIPS variants with 32-bit pointers, like MIPS32 or
MIPS64 n32, because it's trying to do a cast from a 64-bit wide variable
to a 32-bit pointer:

Making all in os
make[2]: Entering directory
`/home/test/test/1/output/build/xserver_xorg-server-1.15.1/os'
  CC     WaitFor.lo
  CC     access.lo
  CC     auth.lo
  CC     backtrace.lo
backtrace.c: In function 'xorg_backtrace':
backtrace.c:90:20: error: cast to pointer from integer of different size
[-Werror=int-to-pointer-cast]
	 if (dladdr((void *)(pip.start_ip + off), &dlinfo) &&
dlinfo.dli_fname &&
		    ^
backtrace.c:98:13: error: cast to pointer from integer of different size
[-Werror=int-to-pointer-cast]
	     (void *)(pip.start_ip + off));
	     ^
cc1: some warnings being treated as errors
make[2]: *** [backtrace.lo] Error 1
make[2]: *** Waiting for unfinished jobs....

Making the cast to a pointer-sized integer, and then to a pointer fixes
the problem.

Related:
  https://bugs.freedesktop.org/show_bug.cgi?id=79939

Signed-off-by: Vicente Olivert Riera <Vincent.Riera@imgtec.com>
Reviewed-by: Keith Packard <keithp@keithp.com>
Signed-off-by: Keith Packard <keithp@keithp.com>
2015-05-26 14:35:55 -07:00
Ray Strode
4b4b9086d0 os: support new implicit local user access mode [CVE-2015-3164 2/3]
If the X server is started without a '-auth' argument, then
it gets started wide open to all local users on the system.

This isn't a great default access model, but changing it in
Xorg at this point would break backward compatibility.

Xwayland, on the other hand is new, and much more targeted
in scope.  It could, in theory, be changed to allow the much
more secure default of a "user who started X server can connect
clients to that server."

This commit paves the way for that change, by adding a mechanism
for DDXs to opt-in to that behavior.  They merely need to call

LocalAccessScopeUser()

in their init functions.

A subsequent commit will add that call for Xwayland.

Signed-off-by: Ray Strode <rstrode@redhat.com>
Reviewed-by: Daniel Stone <daniels@collabora.com>
Reviewed-by: Alan Coopersmith <alan.coopersmith@oracle.com>
Signed-off-by: Keith Packard <keithp@keithp.com>
2015-05-26 11:22:21 -07:00
Keith Packard
26e50e8b2c Merge remote-tracking branch 'jturney/mingw-build-fixes' 2015-05-11 15:36:53 -07:00
Colin Harrison
c7b49bdbb9 os/utils.c: Fix prototype for Win32TempDir()
xorg/xserver/os/utils.c: In function ‘Win32TempDir’:
xorg/xserver/os/utils.c:1643:1: warning: old-style function definition [-Wold-style-definition]

Signed-off-by: Colin Harrison <colin.harrison@virgin.net>
Reviewed-by: Jon TURNEY <jon.turney@dronecode.org.uk>
2015-04-22 12:55:32 +01:00
Colin Harrison
a9b4b7b796 os/utils.c: Don't try to build os_move_fd() for WIN32
Signed-off-by: Colin Harrison <colin.harrison@virgin.net>
Reviewed-by: Jon TURNEY <jon.turney@dronecode.org.uk>
2015-04-22 12:55:29 +01:00
Colin Harrison
8363ef2764 os/xdmcp.c: Include Xtrans.h when building for WIN32
Xtrans.h must be included on WIN32 to prototype _XSERVTransWSAStartup()

xserver/os/xdmcp.c: In function ‘get_addr_by_name’:
xserver/os/xdmcp.c:1483:5: error: implicit declaration of function ‘_XSERVTransWSAStartup’ [-Werror=implicit-function-declaration]

Signed-off-by: Colin Harrison <colin.harrison@virgin.net>
Reviewed-by: Jon TURNEY <jon.turney@dronecode.org.uk>
2015-04-22 12:55:15 +01:00
Alan Coopersmith
df4e41fdb4 Convert os/* to new *allocarray functions
v2: remove now useless parentheses

Signed-off-by: Alan Coopersmith <alan.coopersmith@oracle.com>
Reviewed-by: Matt Turner <mattst88@gmail.com>
2015-04-21 16:57:08 -07:00
Alan Coopersmith
ae75d50395 Add no-fail equivalents of allocarray & reallocarray
v2: Remove extra 's' from comment

Signed-off-by: Alan Coopersmith <alan.coopersmith@oracle.com>
Reviewed-by: Matt Turner <mattst88@gmail.com>
2015-04-21 16:57:08 -07:00
Alan Coopersmith
0887c9463f Import reallocarray() from OpenBSD
Wrapper for realloc() that checks for overflow when multiplying
arguments together, so we don't have to add overflow checks to
every single call.  For documentation on usage, see:
http://www.openbsd.org/cgi-bin/man.cgi/OpenBSD-current/man3/calloc.3

Signed-off-by: Alan Coopersmith <alan.coopersmith@oracle.com>
Acked-by: Matt Turner <mattst88@gmail.com>
2015-04-21 16:57:08 -07:00
Alan Coopersmith
b96dc99996 Add XNFcallocarray() to allow xnfcalloc() to check for overflow
The xnfcalloc() macro took two arguments but simply multiplied them
together without checking for overflow and defeating any overflow
checking that calloc() might have done.  Let's not do that.

The original XNFcalloc() function is left for now to preserve driver
ABI, but is marked as deprecated so it can be removed in a future round
of ABI break/cleanup.

Signed-off-by: Alan Coopersmith <alan.coopersmith@oracle.com>
Reviewed-by: Matt Turner <mattst88@gmail.com>
2015-04-21 16:57:07 -07:00
Jon TURNEY
d3080d421b os: Teach vpnprintf() how to handle "%*.*s"
XdmcpFatal uses the format specifier %*.*s, which vpnprintf() doesn't
understand, which causes a backtrace and prevents the reason for the XDMCP
failure being logged.

See also:
https://bugs.freedesktop.org/show_bug.cgi?id=66862
https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=758574

"%*.*s" is also currently used in a few other places, so teach vpnprintf() how
to handle it

$ fgrep -r "%*.*s" *
hw/dmx/config/scanner.l:    fprintf(stderr, "parse error on line %d at token \"%*.*s\"\n",
hw/dmx/dmxlog.c:        ErrorF("(%s) dmx[i%d/%*.*s]: ", type,
hw/dmx/input/dmxinputinit.c:                dmxLogCont(dmxInfo, "\t[i%d/%*.*s",
os/access.c:        ErrorF("Xserver: siAddrMatch(): type = %s, value = %*.*s -- %s\n",
os/access.c:                ("Xserver: siCheckAddr(): type = %s, value = %*.*s, len = %d -- %s\n",
os/xdmcp.c:    FatalError("XDMCP fatal error: %s %*.*s\n", type,

Signed-off-by: Jon TURNEY <jon.turney@dronecode.org.uk>
Reviewed-by: Colin Harrison <colin.harrison@virgin.net>
2015-03-16 16:34:50 +00:00
Jon TURNEY
491cf02e19 os: XDMCP options like -query etc. should imply -listen tcp
In X server 1.17, the default configuration is now -nolisten tcp.  In this
configuration, XDMCP options don't work usefully, as the X server is not
listening on the port for the display that it tells the display manager to
connect to.

Signed-off-by: Jon TURNEY <jon.turney@dronecode.org.uk>
Reviewed-by: Alan Coopersmith <alan.coopersmith@oracle.com>
Reviewed-by: Colin Harrison <colin.harrison@virgin.net>
2015-03-16 16:34:43 +00:00
Alan Coopersmith
7ea64fb437 Clear ListenTransConns entries in CloseWellKnownConnections
Since _XSERVTransClose frees the connection pointer passed to it,
remove that pointer from the array, so we don't try to double free it
if we come back into CloseWellKnownConnections again.

Should fix https://bugzilla.yoctoproject.org/show_bug.cgi?id=6665 in which
the shutdown section of the main() loop called CloseWellKnownConnections()
and then moved on to ddxGiveUp(), which failed to release the VT and thus
called AbortServer(), which called CloseWellKnownConnections() again.

Signed-off-by: Alan Coopersmith <alan.coopersmith@oracle.com>
Reviewed-by: Adam Jackson <ajax@redhat.com>
Signed-off-by: Peter Hutterer <peter.hutterer@who-t.net>
2015-03-13 12:31:21 +10:00
Dave Airlie
9d9bd38fe1 os/access: fix regression in server interpreted auth
This was reported on irc on Fedora when rawhide went to 1.17.1.

regression occured in: 2566835b43
 os: Eliminate uninitialized value warnings from access.c

siAddrMatch doesn't need addr to be a useful value, it checks
some things like localuser without having an address at all.

Signed-off-by: Dave Airlie <airlied@redhat.com>
Tested-by: Peter Hutterer <peter.hutterer@who-t.net>
Signed-off-by: Peter Hutterer <peter.hutterer@who-t.net>
2015-03-13 12:31:21 +10:00
Alan Coopersmith
9e002dfcd7 Get rid of const warnings in XSERVER_INPUT_EVENT dtrace probe calls
Use typedefs to work around dtrace dropping const qualifiers from probe
arguments when generating Xserver-dtrace.h.   Add new probes.h header to
avoid having to replicate these typedefs in every file with dtrace probes.

Gets rid of these warnings from gcc 4.8:
 getevents.c:1096:9:
  warning: passing argument 6 of '__dtrace_Xserver___input__event' discards
  'const' qualifier from pointer target type [enabled by default]
 getevents.c:1096:9:
  warning: passing argument 7 of '__dtrace_Xserver___input__event' disards
  'const' qualifier from pointer target type [enabled by default]
 getevents.c:1651:9:
  warning: passing argument 6 of '__dtrace_Xserver___input__event' disards
  'const' qualifier from pointer target type [enabled by default]
 getevents.c:1651:9:
  warning: passing argument 7 of '__dtrace_Xserver___input__event' disards
  'const' qualifier from pointer target type [enabled by default]
 getevents.c:1791:9:
  warning: passing argument 6 of '__dtrace_Xserver___input__event' disards
  'const' qualifier from pointer target type [enabled by default]
 getevents.c:1791:9:
  warning: passing argument 7 of '__dtrace_Xserver___input__event' disards
  'const' qualifier from pointer target type [enabled by default]
 getevents.c:1921:9:
  warning: passing argument 6 of '__dtrace_Xserver___input__event' disards
  'const' qualifier from pointer target type [enabled by default]
 getevents.c:1921:9:
  warning: passing argument 7 of '__dtrace_Xserver___input__event' disards
  'const' qualifier from pointer target type [enabled by default]

Signed-off-by: Alan Coopersmith <alan.coopersmith@oracle.com>
Reviewed-by: Peter Hutterer <peter.hutterer@who-t.net>
2015-02-10 18:14:44 -08:00
Nikhil Mahale
fe4c774c57 os: Fix timer race conditions
Fixing following kind of race-conditions -

    WaitForSomething()
    |
    ---->  // timers -> timer-1 -> timer-2 -> null
           while (timers && (int) (timers->expires - now) <= 0)
               // prototype - DoTimer(OsTimerPtr timer, CARD32 now, OsTimerPtr *prev)
               DoTimer(timers, now, &timers)
               |
               |
               ----> OsBlockSignals();  .... OS Signal comes just before blocking it,
                                        .... timer-1 handler gets called.
                                             // timer-1 gets served and scheduled again;
                                             // timers -> timer-2 -> timer-1 -> null
                                        ....
                     *prev = timer->next;
                      timer->next = NULL;   // timers -> null
                      // timers list gets corrupted here and timer-2 gets removed from list.

Fixes: https://bugs.freedesktop.org/show_bug.cgi?id=86288
Signed-off-by: Nikhil Mahale <nmahale@nvidia.com>
Reviewed-by: Julien Cristau <jcristau@debian.org>

v2: Apply warning fixes from Keith Packard <keithp@keithp.com>

Reviewed-by: Aaron Plattner <aplattner@nvidia.com>
Signed-off-by: Aaron Plattner <aplattner@nvidia.com>
Signed-off-by: Keith Packard <keithp@keithp.com>
2015-01-26 10:40:30 -08:00
Carl Worth
7b076fdfc0 os/xsha1.c: Add license and copyright attribution.
I'm interested in copying this code to the mesa project, but before
doing that it seems prudent to have the license and copyright
attributions in place before copying that. To get this list of names I
went through:

	git log -- os/xsha1.c
and:
	git log -- render/glyph.c

Signed-off-by: Keith Packard <keithp@keithp.com>
2015-01-02 13:39:52 -08:00
Alan Coopersmith
bebcac0cf7 Move RTLD_DI_SETSIGNAL code into a separate block to quiet warning
Gets rid of gcc 4.8 warning:
 osinit.c:211:9: warning: ISO C90 forbids mixed declarations and code
  [-Wdeclaration-after-statement]

Signed-off-by: Alan Coopersmith <alan.coopersmith@oracle.com>
Reviewed-by: Keith Packard <keithp@keithp.com>
Signed-off-by: Keith Packard <keithp@keithp.com>
2014-12-19 18:16:15 -08:00
Keith Packard
1559a94395 dix: GetHosts bounds check using wrong pointer value [CVE-2014-8092 pt. 6]
GetHosts saves the pointer to allocated memory in *data, and then
wants to bounds-check writes to that region, but was mistakenly using
a bare 'data' instead of '*data'. Also, data is declared as void **,
so we need a cast to turn it into a byte pointer so we can actually do
pointer comparisons.

Signed-off-by: Keith Packard <keithp@keithp.com>
Reviewed-by: Alan Coopersmith <alan.coopersmith@oracle.com>
Signed-off-by: Alan Coopersmith <alan.coopersmith@oracle.com>
2014-12-09 11:30:52 -08:00
Alan Coopersmith
bc8e20430b dix: integer overflow in GetHosts() [CVE-2014-8092 2/4]
GetHosts() iterates over all the hosts it has in memory, and copies
them to a buffer. The buffer length is calculated by iterating over
all the hosts and adding up all of their combined length. There is a
potential integer overflow, if there are lots and lots of hosts (with
a combined length of > ~4 gig). This should be possible by repeatedly
calling ProcChangeHosts() on 64bit machines with enough memory.

This patch caps the list at 1mb, because multi-megabyte hostname
lists for X access control are insane.

Reported-by: Ilja Van Sprundel <ivansprundel@ioactive.com>
Signed-off-by: Alan Coopersmith <alan.coopersmith@oracle.com>
Reviewed-by: Peter Hutterer <peter.hutterer@who-t.net>
2014-12-08 18:09:47 -08:00