[RFC] Simplify async path based VFS functions: remove tevent and pthreadpool_tevent wrappers

Ralph Böhme slow at samba.org
Wed Dec 19 13:02:39 UTC 2018


Hi folks,

as you may or may not know, since a few months Samba has a fancy new machinery
that was added to properly support path based functions like getxattr() in the
Samba VFS.

The machinery consists of:

1. tevent wrapper

A tevent wrapper associates an existing tevent context with a bunch of function
pointers that get called before and after the tevent event handlers.

This is useful to hide the complexity of becoming the correct user at the right
time, especially in callbacks from tevent.

2. pthreadpool_tevent wrapper

pthreadpool_tevent wrapper hides the complexity of impersonating the correct
user and changing the current working directory to the base directory of a
service from the worker thread.

3. VFS glue

This is a structure and some functions that bring together user and root
pthreadpool_tevent and tevent wrappers, taking into account system support for
per-thread credentials and per-thread current working directory support.

VFS implementations can use the VFS glue and the supporting functions that act
on the glue in a boilerplate fashion to implement path based VFS functions,
including support for funning worker thread job functions as root and in the
correct working directory.

The problem
-----------

After some behind the scenes talks involving Jeremy, metze, Volker and myself,
the combined usage of the wrappers and glue in smbd was deemed too complex.

The function that creates the VFS glue object might serve as an example of the
complexity that is hiding in the dark:

        user_vfs_evg = smb_vfs_ev_glue_create(conn,
                                              user_ev_ctx,
                                              user_tp_fd_safe,
                                              user_tp_path_safe,
                                              user_tp_chdir_safe,
                                              conn->sconn->root_ev_ctx,
                                              root_tp_fd_safe,
                                              root_tp_path_safe,
                                              root_tp_chdir_safe);

*_ev_ctx are tevent wrapper contexts, *_tp_*safe are pthreadpool_tevent contexts
of various kinds. It's a no brainer for the use of the glue, but for anyone
working on the internals of the wrappers or any other subsystem that interfaces
with the wrapped tevent contexts, it requires a thorough understanding of the
inner workings.

After some serious brain twisting we (Volker, metze, myself) came up with the
following plan to unwind the wrappers.

The plan
--------

In the short term we basically remove the wrappers and change the only user in
the VFS that makes use of the glue, SMB_VFS_GETXATTR_AT_SEND/RECV, to do all the
required impersonation and chdir() calls by itself.

In Samba 4.5 that looks like this:

https://git.samba.org/?p=slow/samba.git;a=commitdiff;h=9db3a686ad00cfe0125469d6bfed5bbb9c162ccd

The complex, error prone code there was the reason that spurned the design of an
interface that takes care of all this, which lead to the wrappers and the VFS glue.

As all SMB2 operations are handle based, path based VFS functions get an
additional directory handle. The above commit may serve as an example:

    static struct tevent_req *vfswrap_getxattrat_send(
           struct vfs_handle_struct *handle,
           TALLOC_CTX *mem_ctx,
           struct tevent_context *ev,
           files_struct *dir_fsp,
           const char *path,
           const char *xattr_name,
           size_t alloc_hint)

Steps:

- remove tevent wrappers, possibly subject to debate as some functions are
  already shipping with 4.9 and are part of the tevent API

- remove use of tevent_wrappers in smbd

- remove pthreadpool_tevent wrappers

- rewrite default implementation of SMB_VFS_GETXATTR_AT_SEND and
  SMB_VFS_GET_DOSATTRIBUTES_SEND as described above

The longterm plan is to pass an additional argument to all syscall wrapping VFS
functions with the required internal information to perform impersonation and
chdir(), eg an "impersonation_context".

This, together with some helper functions that work on the object and can be
used the implementation of VFS functions, should result in reasonable complexity
both in the implementation and for the user of the new API.

Thoughts?

-slow

-- 
Ralph Boehme, Samba Team                https://samba.org/
Samba Developer, SerNet GmbH   https://sernet.de/en/samba/
GPG-Fingerprint   FAE2C6088A24252051C559E4AA1E9B7126399E46



More information about the samba-technical mailing list