Re: [PATCH v1 06/11] landlock: Add support for file reparenting with LANDLOCK_ACCESS_FS_REFER

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



This error is because clang does not behave like GCC: check_access_path_dual() should be marked as __always_inline, or I should change from BUILD_BUG_ON() to WARN_ON_ONCE() if needed. I'll fix that in the next series.

On 22/02/2022 04:16, kernel test robot wrote:
Hi "Mickaël,

I love your patch! Yet something to improve:

[auto build test ERROR on cfb92440ee71adcc2105b0890bb01ac3cddb8507]

url:    https://github.com/0day-ci/linux/commits/Micka-l-Sala-n/Landlock-file-linking-and-renaming-support/20220222-051842
base:   cfb92440ee71adcc2105b0890bb01ac3cddb8507
config: hexagon-randconfig-r002-20220221 (https://download.01.org/0day-ci/archive/20220222/202202221149.qLO9DEqo-lkp@xxxxxxxxx/config)
compiler: clang version 15.0.0 (https://github.com/llvm/llvm-project d271fc04d5b97b12e6b797c6067d3c96a8d7470e)
reproduce (this is a W=1 build):
         wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
         chmod +x ~/bin/make.cross
         # https://github.com/0day-ci/linux/commit/c68b879f54d6262963d435a18cedbc238b7faeaf
         git remote add linux-review https://github.com/0day-ci/linux
         git fetch --no-tags linux-review Micka-l-Sala-n/Landlock-file-linking-and-renaming-support/20220222-051842
         git checkout c68b879f54d6262963d435a18cedbc238b7faeaf
         # save the config file to linux build tree
         mkdir build_dir
         COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross W=1 O=build_dir ARCH=hexagon SHELL=/bin/bash

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@xxxxxxxxx>

All errors (new ones prefixed by >>):

security/landlock/fs.c:463:2: error: call to __compiletime_assert_228 declared with 'error' attribute: BUILD_BUG_ON failed: !layer_masks_dst_parent
            BUILD_BUG_ON(!layer_masks_dst_parent);
            ^
    include/linux/build_bug.h:50:2: note: expanded from macro 'BUILD_BUG_ON'
            BUILD_BUG_ON_MSG(condition, "BUILD_BUG_ON failed: " #condition)
            ^
    include/linux/build_bug.h:39:37: note: expanded from macro 'BUILD_BUG_ON_MSG'
    #define BUILD_BUG_ON_MSG(cond, msg) compiletime_assert(!(cond), msg)
                                        ^
    include/linux/compiler_types.h:346:2: note: expanded from macro 'compiletime_assert'
            _compiletime_assert(condition, msg, __compiletime_assert_, __COUNTER__)
            ^
    include/linux/compiler_types.h:334:2: note: expanded from macro '_compiletime_assert'
            __compiletime_assert(condition, msg, prefix, suffix)
            ^
    include/linux/compiler_types.h:327:4: note: expanded from macro '__compiletime_assert'
                            prefix ## suffix();                             \
                            ^
    <scratch space>:170:1: note: expanded from here
    __compiletime_assert_228
    ^
security/landlock/fs.c:670:2: error: call to __compiletime_assert_229 declared with 'error' attribute: BUILD_BUG_ON failed: !layer_masks_dom
            BUILD_BUG_ON(!layer_masks_dom);
            ^
    include/linux/build_bug.h:50:2: note: expanded from macro 'BUILD_BUG_ON'
            BUILD_BUG_ON_MSG(condition, "BUILD_BUG_ON failed: " #condition)
            ^
    include/linux/build_bug.h:39:37: note: expanded from macro 'BUILD_BUG_ON_MSG'
    #define BUILD_BUG_ON_MSG(cond, msg) compiletime_assert(!(cond), msg)
                                        ^
    include/linux/compiler_types.h:346:2: note: expanded from macro 'compiletime_assert'
            _compiletime_assert(condition, msg, __compiletime_assert_, __COUNTER__)
            ^
    include/linux/compiler_types.h:334:2: note: expanded from macro '_compiletime_assert'
            __compiletime_assert(condition, msg, prefix, suffix)
            ^
    include/linux/compiler_types.h:327:4: note: expanded from macro '__compiletime_assert'
                            prefix ## suffix();                             \
                            ^
    <scratch space>:174:1: note: expanded from here
    __compiletime_assert_229
    ^
    2 errors generated.


vim +/error +463 security/landlock/fs.c

    401	
    402	/**
    403	 * check_access_path_dual - Check a source and a destination accesses
    404	 *
    405	 * @domain: Domain to check against.
    406	 * @path: File hierarchy to walk through.
    407	 * @child_is_directory: Must be set to true if the (original) leaf is a
    408	 *     directory, false otherwise.
    409	 * @access_request_dst_parent: Accesses to check, once @layer_masks_dst_parent
    410	 *     is equal to @layer_masks_src_parent (if any).
    411	 * @layer_masks_dst_parent: Pointer to a matrix of layer masks per access
    412	 *     masks, identifying the layers that forbid a specific access.  Bits from
    413	 *     this matrix can be unset according to the @path walk.  An empty matrix
    414	 *     means that @domain allows all possible Landlock accesses (i.e. not only
    415	 *     those identified by @access_request_dst_parent).  This matrix can
    416	 *     initially refer to domain layer masks and, when the accesses for the
    417	 *     destination and source are the same, to request layer masks.
    418	 * @access_request_src_parent: Similar to @access_request_dst_parent but for an
    419	 *     initial source path request.  Only taken into account if
    420	 *     @layer_masks_src_parent is not NULL.
    421	 * @layer_masks_src_parent: Similar to @layer_masks_dst_parent but for an
    422	 *     initial source path walk.  This can be NULL if only dealing with a
    423	 *     destination access request (i.e. not a rename nor a link action).
    424	 * @layer_masks_child: Similar to @layer_masks_src_parent but only for the
    425	 *     linked or renamed inode (without hierarchy).  This is only used if
    426	 *     @layer_masks_src_parent is not NULL.
    427	 *
    428	 * This helper first checks that the destination has a superset of restrictions
    429	 * compared to the source (if any) for a common path.  It then checks that the
    430	 * collected accesses and the remaining ones are enough to allow the request.
    431	 *
    432	 * Returns:
    433	 * - 0 if the access request is granted;
    434	 * - -EACCES if it is denied because of access right other than
    435	 *   LANDLOCK_ACCESS_FS_REFER;
    436	 * - -EXDEV if the renaming or linking would be a privileged escalation
    437	 *   (according to each layered policies), or if LANDLOCK_ACCESS_FS_REFER is
    438	 *   not allowed by the source or the destination.
    439	 */
    440	static int check_access_path_dual(const struct landlock_ruleset *const domain,
    441			const struct path *const path,
    442			bool child_is_directory,
    443			const access_mask_t access_request_dst_parent,
    444			layer_mask_t (*const
    445				layer_masks_dst_parent)[LANDLOCK_NUM_ACCESS_FS],
    446			const access_mask_t access_request_src_parent,
    447			layer_mask_t (*layer_masks_src_parent)[LANDLOCK_NUM_ACCESS_FS],
    448			layer_mask_t (*layer_masks_child)[LANDLOCK_NUM_ACCESS_FS])
    449	{
    450		bool allowed_dst_parent = false, allowed_src_parent = false, is_dom_check;
    451		struct path walker_path;
    452		access_mask_t access_masked_dst_parent, access_masked_src_parent;
    453	
    454		if (!access_request_dst_parent && !access_request_src_parent)
    455			return 0;
    456		if (WARN_ON_ONCE(!domain || !path))
    457			return 0;
    458		if (is_nouser_or_private(path->dentry))
    459			return 0;
    460		if (WARN_ON_ONCE(domain->num_layers < 1))
    461			return -EACCES;
    462	
  > 463		BUILD_BUG_ON(!layer_masks_dst_parent);
    464		if (layer_masks_src_parent) {
    465			if (WARN_ON_ONCE(!layer_masks_child))
    466				return -EACCES;
    467			access_masked_dst_parent = access_masked_src_parent =
    468				get_handled_accesses(domain);
    469			is_dom_check = true;
    470		} else {
    471			if (WARN_ON_ONCE(layer_masks_child))
    472				return -EACCES;
    473			access_masked_dst_parent = access_request_dst_parent;
    474			access_masked_src_parent = access_request_src_parent;
    475			is_dom_check = false;
    476		}
    477	
    478		walker_path = *path;
    479		path_get(&walker_path);
    480		/*
    481		 * We need to walk through all the hierarchy to not miss any relevant
    482		 * restriction.
    483		 */
    484		while (true) {
    485			struct dentry *parent_dentry;
    486			const struct landlock_rule *rule;
    487	
    488			/*
    489			 * If at least all accesses allowed on the destination are
    490			 * already allowed on the source, respectively if there is at
    491			 * least as much as restrictions on the destination than on the
    492			 * source, then we can safely refer files from the source to
    493			 * the destination without risking a privilege escalation.
    494			 * This is crucial for standalone multilayered security
    495			 * policies.  Furthermore, this helps avoid policy writers to
    496			 * shoot themselves in the foot.
    497			 */
    498			if (is_dom_check && is_superset(child_is_directory,
    499						layer_masks_dst_parent,
    500						layer_masks_src_parent,
    501						layer_masks_child)) {
    502				allowed_dst_parent =
    503					scope_to_request(access_request_dst_parent,
    504							layer_masks_dst_parent);
    505				allowed_src_parent =
    506					scope_to_request(access_request_src_parent,
    507							layer_masks_src_parent);
    508	
    509				/* Stops when all accesses are granted. */
    510				if (allowed_dst_parent && allowed_src_parent)
    511					break;
    512	
    513				/*
    514				 * Downgrades checks from domain handled accesses to
    515				 * requested accesses.
    516				 */
    517				is_dom_check = false;
    518				access_masked_dst_parent = access_request_dst_parent;
    519				access_masked_src_parent = access_request_src_parent;
    520			}
    521	
    522			rule = find_rule(domain, walker_path.dentry);
    523			allowed_dst_parent = unmask_layers(rule, access_masked_dst_parent,
    524					layer_masks_dst_parent);
    525			allowed_src_parent = unmask_layers(rule, access_masked_src_parent,
    526					layer_masks_src_parent);
    527	
    528			/* Stops when a rule from each layer grants access. */
    529			if (allowed_dst_parent && allowed_src_parent)
    530				break;
    531	
    532	jump_up:
    533			if (walker_path.dentry == walker_path.mnt->mnt_root) {
    534				if (follow_up(&walker_path)) {
    535					/* Ignores hidden mount points. */
    536					goto jump_up;
    537				} else {
    538					/*
    539					 * Stops at the real root.  Denies access
    540					 * because not all layers have granted access.
    541					 */
    542					allowed_dst_parent = false;
    543					break;
    544				}
    545			}
    546			if (unlikely(IS_ROOT(walker_path.dentry))) {
    547				/*
    548				 * Stops at disconnected root directories.  Only allows
    549				 * access to internal filesystems (e.g. nsfs, which is
    550				 * reachable through /proc/<pid>/ns/<namespace>).
    551				 */
    552				allowed_dst_parent = !!(walker_path.mnt->mnt_flags &
    553						MNT_INTERNAL);
    554				break;
    555			}
    556			parent_dentry = dget_parent(walker_path.dentry);
    557			dput(walker_path.dentry);
    558			walker_path.dentry = parent_dentry;
    559		}
    560		path_put(&walker_path);
    561	
    562		if (allowed_dst_parent && allowed_src_parent)
    563			return 0;
    564	
    565		/*
    566		 * Unfortunately, we cannot prioritize EACCES over EXDEV for all
    567		 * RENAME_EXCHANGE cases because it depends on the source and
    568		 * destination order.  This could be changed with a new
    569		 * security_path_rename hook implementation.
    570		 */
    571		if (likely(is_eacces(layer_masks_dst_parent, access_request_dst_parent)
    572					|| is_eacces(layer_masks_src_parent,
    573						access_request_src_parent)))
    574			return -EACCES;
    575	
    576		/*
    577		 * Gracefully forbids reparenting if the destination directory
    578		 * hierarchy is not a superset of restrictions of the source directory
    579		 * hierarchy, or if LANDLOCK_ACCESS_FS_REFER is not allowed by the
    580		 * source or the destination.
    581		 */
    582		return -EXDEV;
    583	}
    584	

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@xxxxxxxxxxxx



[Index of Archives]     [Kernel Newbies]     [Security]     [Netfilter]     [Bugtraq]     [Linux FS]     [Yosemite Forum]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Samba]     [Video 4 Linux]     [Device Mapper]     [Linux Resources]

  Powered by Linux