• Dave Marchevsky's avatar
    fuse: Rearrange fuse_allow_current_process checks · b1387777
    Dave Marchevsky authored
    This is a followup to a previous commit of mine [0], which added the
    allow_sys_admin_access && capable(CAP_SYS_ADMIN) check.  This patch
    rearranges the order of checks in fuse_allow_current_process without
    changing functionality.
    
    Commit 9ccf47b2 ("fuse: Add module param for CAP_SYS_ADMIN access
    bypassing allow_other") added allow_sys_admin_access &&
    capable(CAP_SYS_ADMIN) check to the beginning of the function, with the
    reasoning that allow_sys_admin_access should be an 'escape hatch' for users
    with CAP_SYS_ADMIN, allowing them to skip any subsequent checks.
    
    However, placing this new check first results in many capable() calls when
    allow_sys_admin_access is set, where another check would've also returned
    1.  This can be problematic when a BPF program is tracing capable() calls.
    
    At Meta we ran into such a scenario recently.  On a host where
    allow_sys_admin_access is set but most of the FUSE access is from processes
    which would pass other checks - i.e.  they don't need CAP_SYS_ADMIN 'escape
    hatch' - this results in an unnecessary capable() call for each fs op.  We
    also have a daemon tracing capable() with BPF and doing some data
    collection, so tracing these extraneous capable() calls has the potential
    to regress performance for an application doing many FUSE ops.
    
    So rearrange the order of these checks such that CAP_SYS_ADMIN 'escape
    hatch' is checked last.  Add a small helper, fuse_permissible_uidgid, to
    make the logic easier to understand.  Previously, if allow_other is set on
    the fuse_conn, uid/git checking doesn't happen as current_in_userns result
    is returned.  These semantics are maintained here: fuse_permissible_uidgid
    check only happens if allow_other is not set.
    Signed-off-by: default avatarDave Marchevsky <davemarchevsky@fb.com>
    Suggested-by: default avatarAndrii Nakryiko <andrii@kernel.org>
    Reviewed-by: default avatarChristian Brauner (Microsoft) <brauner@kernel.org>
    Signed-off-by: default avatarMiklos Szeredi <mszeredi@redhat.com>
    b1387777
dir.c 49.4 KB