About Kernel Documentation Linux Kernel Contact Linux Resources Linux Blog

Documentation / ioctl / botching-up-ioctls.txt


Based on kernel version 4.16.1. Page generated on 2018-04-09 11:53 EST.

1	(How to avoid) Botching up ioctls
2	=================================
3	
4	From: http://blog.ffwll.ch/2013/11/botching-up-ioctls.html
5	
6	By: Daniel Vetter, Copyright © 2013 Intel Corporation
7	
8	One clear insight kernel graphics hackers gained in the past few years is that
9	trying to come up with a unified interface to manage the execution units and
10	memory on completely different GPUs is a futile effort. So nowadays every
11	driver has its own set of ioctls to allocate memory and submit work to the GPU.
12	Which is nice, since there's no more insanity in the form of fake-generic, but
13	actually only used once interfaces. But the clear downside is that there's much
14	more potential to screw things up.
15	
16	To avoid repeating all the same mistakes again I've written up some of the
17	lessons learned while botching the job for the drm/i915 driver. Most of these
18	only cover technicalities and not the big-picture issues like what the command
19	submission ioctl exactly should look like. Learning these lessons is probably
20	something every GPU driver has to do on its own.
21	
22	
23	Prerequisites
24	-------------
25	
26	First the prerequisites. Without these you have already failed, because you
27	will need to add a 32-bit compat layer:
28	
29	 * Only use fixed sized integers. To avoid conflicts with typedefs in userspace
30	   the kernel has special types like __u32, __s64. Use them.
31	
32	 * Align everything to the natural size and use explicit padding. 32-bit
33	   platforms don't necessarily align 64-bit values to 64-bit boundaries, but
34	   64-bit platforms do. So we always need padding to the natural size to get
35	   this right.
36	
37	 * Pad the entire struct to a multiple of 64-bits if the structure contains
38	   64-bit types - the structure size will otherwise differ on 32-bit versus
39	   64-bit. Having a different structure size hurts when passing arrays of
40	   structures to the kernel, or if the kernel checks the structure size, which
41	   e.g. the drm core does.
42	
43	 * Pointers are __u64, cast from/to a uintprt_t on the userspace side and
44	   from/to a void __user * in the kernel. Try really hard not to delay this
45	   conversion or worse, fiddle the raw __u64 through your code since that
46	   diminishes the checking tools like sparse can provide. The macro
47	   u64_to_user_ptr can be used in the kernel to avoid warnings about integers
48	   and pointres of different sizes.
49	
50	
51	Basics
52	------
53	
54	With the joys of writing a compat layer avoided we can take a look at the basic
55	fumbles. Neglecting these will make backward and forward compatibility a real
56	pain. And since getting things wrong on the first attempt is guaranteed you
57	will have a second iteration or at least an extension for any given interface.
58	
59	 * Have a clear way for userspace to figure out whether your new ioctl or ioctl
60	   extension is supported on a given kernel. If you can't rely on old kernels
61	   rejecting the new flags/modes or ioctls (since doing that was botched in the
62	   past) then you need a driver feature flag or revision number somewhere.
63	
64	 * Have a plan for extending ioctls with new flags or new fields at the end of
65	   the structure. The drm core checks the passed-in size for each ioctl call
66	   and zero-extends any mismatches between kernel and userspace. That helps,
67	   but isn't a complete solution since newer userspace on older kernels won't
68	   notice that the newly added fields at the end get ignored. So this still
69	   needs a new driver feature flags.
70	
71	 * Check all unused fields and flags and all the padding for whether it's 0,
72	   and reject the ioctl if that's not the case. Otherwise your nice plan for
73	   future extensions is going right down the gutters since someone will submit
74	   an ioctl struct with random stack garbage in the yet unused parts. Which
75	   then bakes in the ABI that those fields can never be used for anything else
76	   but garbage.
77	
78	 * Have simple testcases for all of the above.
79	
80	
81	Fun with Error Paths
82	--------------------
83	
84	Nowadays we don't have any excuse left any more for drm drivers being neat
85	little root exploits. This means we both need full input validation and solid
86	error handling paths - GPUs will die eventually in the oddmost corner cases
87	anyway:
88	
89	 * The ioctl must check for array overflows. Also it needs to check for
90	   over/underflows and clamping issues of integer values in general. The usual
91	   example is sprite positioning values fed directly into the hardware with the
92	   hardware just having 12 bits or so. Works nicely until some odd display
93	   server doesn't bother with clamping itself and the cursor wraps around the
94	   screen.
95	
96	 * Have simple testcases for every input validation failure case in your ioctl.
97	   Check that the error code matches your expectations. And finally make sure
98	   that you only test for one single error path in each subtest by submitting
99	   otherwise perfectly valid data. Without this an earlier check might reject
100	   the ioctl already and shadow the codepath you actually want to test, hiding
101	   bugs and regressions.
102	
103	 * Make all your ioctls restartable. First X really loves signals and second
104	   this will allow you to test 90% of all error handling paths by just
105	   interrupting your main test suite constantly with signals. Thanks to X's
106	   love for signal you'll get an excellent base coverage of all your error
107	   paths pretty much for free for graphics drivers. Also, be consistent with
108	   how you handle ioctl restarting - e.g. drm has a tiny drmIoctl helper in its
109	   userspace library. The i915 driver botched this with the set_tiling ioctl,
110	   now we're stuck forever with some arcane semantics in both the kernel and
111	   userspace.
112	
113	 * If you can't make a given codepath restartable make a stuck task at least
114	   killable. GPUs just die and your users won't like you more if you hang their
115	   entire box (by means of an unkillable X process). If the state recovery is
116	   still too tricky have a timeout or hangcheck safety net as a last-ditch
117	   effort in case the hardware has gone bananas.
118	
119	 * Have testcases for the really tricky corner cases in your error recovery code
120	   - it's way too easy to create a deadlock between your hangcheck code and
121	   waiters.
122	
123	
124	Time, Waiting and Missing it
125	----------------------------
126	
127	GPUs do most everything asynchronously, so we have a need to time operations and
128	wait for outstanding ones. This is really tricky business; at the moment none of
129	the ioctls supported by the drm/i915 get this fully right, which means there's
130	still tons more lessons to learn here.
131	
132	 * Use CLOCK_MONOTONIC as your reference time, always. It's what alsa, drm and
133	   v4l use by default nowadays. But let userspace know which timestamps are
134	   derived from different clock domains like your main system clock (provided
135	   by the kernel) or some independent hardware counter somewhere else. Clocks
136	   will mismatch if you look close enough, but if performance measuring tools
137	   have this information they can at least compensate. If your userspace can
138	   get at the raw values of some clocks (e.g. through in-command-stream
139	   performance counter sampling instructions) consider exposing those also.
140	
141	 * Use __s64 seconds plus __u64 nanoseconds to specify time. It's not the most
142	   convenient time specification, but it's mostly the standard.
143	
144	 * Check that input time values are normalized and reject them if not. Note
145	   that the kernel native struct ktime has a signed integer for both seconds
146	   and nanoseconds, so beware here.
147	
148	 * For timeouts, use absolute times. If you're a good fellow and made your
149	   ioctl restartable relative timeouts tend to be too coarse and can
150	   indefinitely extend your wait time due to rounding on each restart.
151	   Especially if your reference clock is something really slow like the display
152	   frame counter. With a spec lawyer hat on this isn't a bug since timeouts can
153	   always be extended - but users will surely hate you if their neat animations
154	   starts to stutter due to this.
155	
156	 * Consider ditching any synchronous wait ioctls with timeouts and just deliver
157	   an asynchronous event on a pollable file descriptor. It fits much better
158	   into event driven applications' main loop.
159	
160	 * Have testcases for corner-cases, especially whether the return values for
161	   already-completed events, successful waits and timed-out waits are all sane
162	   and suiting to your needs.
163	
164	
165	Leaking Resources, Not
166	----------------------
167	
168	A full-blown drm driver essentially implements a little OS, but specialized to
169	the given GPU platforms. This means a driver needs to expose tons of handles
170	for different objects and other resources to userspace. Doing that right
171	entails its own little set of pitfalls:
172	
173	 * Always attach the lifetime of your dynamically created resources to the
174	   lifetime of a file descriptor. Consider using a 1:1 mapping if your resource
175	   needs to be shared across processes -  fd-passing over unix domain sockets
176	   also simplifies lifetime management for userspace.
177	
178	 * Always have O_CLOEXEC support.
179	
180	 * Ensure that you have sufficient insulation between different clients. By
181	   default pick a private per-fd namespace which forces any sharing to be done
182	   explicitly. Only go with a more global per-device namespace if the objects
183	   are truly device-unique. One counterexample in the drm modeset interfaces is
184	   that the per-device modeset objects like connectors share a namespace with
185	   framebuffer objects, which mostly are not shared at all. A separate
186	   namespace, private by default, for framebuffers would have been more
187	   suitable.
188	
189	 * Think about uniqueness requirements for userspace handles. E.g. for most drm
190	   drivers it's a userspace bug to submit the same object twice in the same
191	   command submission ioctl. But then if objects are shareable userspace needs
192	   to know whether it has seen an imported object from a different process
193	   already or not. I haven't tried this myself yet due to lack of a new class
194	   of objects, but consider using inode numbers on your shared file descriptors
195	   as unique identifiers - it's how real files are told apart, too.
196	   Unfortunately this requires a full-blown virtual filesystem in the kernel.
197	
198	
199	Last, but not Least
200	-------------------
201	
202	Not every problem needs a new ioctl:
203	
204	 * Think hard whether you really want a driver-private interface. Of course
205	   it's much quicker to push a driver-private interface than engaging in
206	   lengthy discussions for a more generic solution. And occasionally doing a
207	   private interface to spearhead a new concept is what's required. But in the
208	   end, once the generic interface comes around you'll end up maintainer two
209	   interfaces. Indefinitely.
210	
211	 * Consider other interfaces than ioctls. A sysfs attribute is much better for
212	   per-device settings, or for child objects with fairly static lifetimes (like
213	   output connectors in drm with all the detection override attributes). Or
214	   maybe only your testsuite needs this interface, and then debugfs with its
215	   disclaimer of not having a stable ABI would be better.
216	
217	Finally, the name of the game is to get it right on the first attempt, since if
218	your driver proves popular and your hardware platforms long-lived then you'll
219	be stuck with a given ioctl essentially forever. You can try to deprecate
220	horrible ioctls on newer iterations of your hardware, but generally it takes
221	years to accomplish this. And then again years until the last user able to
222	complain about regressions disappears, too.
Hide Line Numbers


About Kernel Documentation Linux Kernel Contact Linux Resources Linux Blog