[PATCH] Documentation: Add a guide to porting drivers to PCuABI - linux-morello - op-lists.linaro.org

1 Aug 2022

Add a small guide documenting the most common changes required to
get drivers (and potentially other subsystems) compatible with
the pure-capability user ABI (PCuABI).
There is no obvious subdirectory for this type of document, and it
is transient in nature, so add it at the top-level.
Signed-off-by: Kevin Brodsky kevin.brodsky@arm.com
---
Rendered version:
  https://git.morello-project.org/morello/kernel/linux/-/blob/review/kevin/por...
Documentation/pcuabi-porting.rst | 188 +++++++++++++++++++++++++++++++
 1 file changed, 188 insertions(+)
 create mode 100644 Documentation/pcuabi-porting.rst

diff --git a/Documentation/pcuabi-porting.rst b/Documentation/pcuabi-porting.rst
new file mode 100644
index 000000000000..9c78523e39b7
--- /dev/null
+++ b/Documentation/pcuabi-porting.rst
@@ -0,0 +1,188 @@
+=================================
+Adding PCuABI support to drivers 
+=================================
+
+This document provides a non-exhaustive overview of the most common
+changes required to support the pure-capability user ABI (PCuABI) in
+arbitrary drivers. It may also be helpful for core subsystems, though
+note that more extensive changes may be required than for drivers with
+straightforward interactions with userspace.
+
+.. _user pointer documentation: core-api/user_ptr.rst
+
+User pointer representation and conversions
+===========================================
+
+The appropriate API to represent and convert user pointers is described
+in the `user pointer documentation`_. A few examples of modifications
+required for PCuABI compliance:
+
++--------------------------------+------------------------------------+
+| Invalid code                   | Potential replacement              |
++================================+====================================+
+| ``(unsigned long)uptr``        | ``user_ptr_addr(uptr)``            |
++--------------------------------+------------------------------------+
+| ``(void __user *)u64``         | | ``uaddr_to_user_ptr(u64)``       |
+|                                | | ``as_user_ptr(u64)``             |
++--------------------------------+------------------------------------+
+| ``get_user(uptr, &uarg->ptr)`` | ``get_user_ptr(uptr, &uarg->ptr)`` |
++--------------------------------+------------------------------------+
+| ``IS_ERR(ubuf)``               | ``USER_PTR_IS_ERR(ubuf)``          |
++--------------------------------+------------------------------------+
+
+``ioctl`` handlers' third argument
+==================================
+
+Traditionally, the type of the third argument of ``ioctl`` handlers is
+``unsigned long``. Unfortunately this is no longer appropriate in PCuABI
+in many cases, as this argument may represent a pointer. A larger type,
+able to hold a capability, is therefore necessary: this type is
+``user_uintptr_t``.
+
+The prototype of most ``ioctl`` methods has been modified to take
+``user_uintptr_t`` as third argument (``arg``), notably
+``struct file_operations::unlocked_ioctl`` and
+``struct block_device_operations::ioctl``. As a consequence, any driver
+implementing such methods must change the prototype of the callback
+accordingly, that is replace ``unsigned long`` with ``user_uintptr_t``,
+as the prototypes must match exactly.
+
+Additionally, if the handler passes down ``arg`` to other functions,
+such functions must also represent this argument as ``user_uintptr_t``
+**if they handle any request type where arg is a pointer** - in other
+words, if they cast ``arg`` to a user pointer, e.g. ``void __user *``.
+Otherwise, it is acceptable to leave these functions unchanged (i.e.
+keep taking an ``unsigned long``).
+
+Compat ``ioctl`` handlers
+=========================
+
+Third argument
+--------------
+
+Unlike native ``ioctl`` handlers, the type of the third argument of
+compat ``ioctl`` handlers remains ``unsigned long``, as this remains an
+appropriate type to represent compat pointers in both compat32 and
+compat64. As a result, a native handler can no longer be used as a compat
+handler, because their prototypes differ. An appropriate compat handler
+must therefore always be chosen.
+
+Compat user pointers should always be converted to native user pointers
+using ``compat_ptr()``. Unfortunately this is not always the case in
+existing drivers, especially in compat ``ioctl`` handlers, which may
+simply pass their arguments through to the native handler without using
+the appropriate pointer conversion. This is complicated by the fact that
+the actual type of the third argument (``arg``) may depend on the
+request type, just like in native handlers. The subsections below cover
+the various possible situations.
+
+``arg`` is always a pointer
+~~~~~~~~~~~~~~~~~~~~~~~~~~~
+
+If ``arg`` always represents a pointer, it can be unconditionally
+converted to a native pointer and passed to the native handler, for
+instance::
+
+  static long my_compat_ioctl(struct file *file, unsigned int cmd,
+                              unsigned long arg)
+  {
+          return my_ioctl(file, cmd, (user_uintptr_t)compat_ptr(arg));
+  }
+
+For such a trivial ``file_operations`` compat handler, there is in fact
+no need to create a compat handler; the ``compat_ptr_ioctl`` helper can
+be used instead::
+
+   static const struct file_operations my_fops = {
+          ...
+          .compat_ioctl = compat_ptr_ioctl,
+          ...
+   };
+
+``arg`` is never a pointer
+~~~~~~~~~~~~~~~~~~~~~~~~~~
+
+If ``arg`` never represents a pointer, it can directly be passed to
+the native handler, optionally cast to ``user_uintptr_t``.
+
+Similarly, in this situation, the ``compat_noptr_ioctl`` helper can be
+used instead of writing a trivial ``file_operations`` compat handler::
+
+   static const struct file_operations my_fops = {
+          ...
+          .compat_ioctl = compat_noptr_ioctl,
+          ...
+   };
+
+``arg`` is sometimes a pointer
+~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
+
+If ``arg`` represents a pointer for certain request types but not
+others, then the compat handler should ensure that the appropriate
+conversion is made depending on the request type, i.e. ``compat_ptr()``
+is used **if and only if arg is a pointer for the specific request
+type.**
+
+Struct layout / 32-bit assumptions
+----------------------------------
+
+Aside from the conversion of ``arg``, existing compat ``ioctl`` handlers
+should be examined carefully as they typically include two different
+types of transformations:
+
+1. Struct layout transformation, which may include pointer conversions.
+2. 32-bit-specific transformations, e.g. for 32-bit time representation.
+
+The difficulty with supporting compat64 is that the first transformation
+may still required, while the second transformation is not relevant and
+should not be carried out in compat64 (some ``#ifdef CONFIG_COMPAT32``
+may be required).
+
+When a given request takes a pointer to a struct, and that struct
+contains types that differ in compat, it is normally represented as
+``struct compat_<name>`` in the compat handler. It may happen that some
+of the types used in this struct are only appropriate for compat32. The
+preferred approach in this case is to change these types to appropriate
+``compat_*`` types, for instance ``compat_long`` instead of ``s32``.
+This holds even if the entire transformation is unnecessary in compat64;
+this is so that the compat handler is kept as generic as possible.
+
+When the struct contains pointers, they must be represented as
+``compat_uptr_t`` (preferred) or ``compat_caddr_t`` in its compat
+counterpart, and conversions between compat user pointers and native
+user pointers must always be made using ``compat_ptr()`` and
+``ptr_to_compat()``.
+
+``__user`` annotation fixups
+============================
+
+Most complex types involving ``__user`` fail to build in PCuABI, notably
+double user pointers (user pointer to a user pointer). This is because
+using ``__capability`` as a prefix to ``*`` only has the intended
+meaning in a limited number of situations. Otherwise, the compiler will
+typically throw the following error::
+
+  error: use of __capability is ambiguous
+
+A fixup is then required, as described in section "PCuABI-specific
+changes" of the `user pointer documentation`_. For instance::
+
+  diff --git a/net/socket.c b/net/socket.c
+  index 8597fbacb089..ab2a610825cc 100644
+  --- a/net/socket.c
+  +++ b/net/socket.c
+  @@ -3156,7 +3169,11 @@ void socket_seq_show(struct seq_file *seq)
+    * the next page isn't readable/writable, we get a fault. To prevent
+    * that, copy back and forth to the full size.
+    */
+  +#ifdef CONFIG_CHERI_PURECAP_UABI
+  +int get_user_ifreq(struct ifreq *ifr, void * __capability *ifrdata, void __user *arg)
+  +#else
+   int get_user_ifreq(struct ifreq *ifr, void __user **ifrdata, void __user *arg)
+  +#endif
+   {
+          if (in_compat_syscall()) {
+                  struct compat_ifreq *ifr32 = (struct compat_ifreq *)ifr;
+
+Fortunately, ``__user`` is mostly used in simple types, and such fixups
+are rarely needed in driver code.
-- 
2.34.1


    