lists.openwall.net   lists  /  announce  owl-users  owl-dev  john-users  john-dev  passwdqc-users  yescrypt  popa3d-users  /  oss-security  kernel-hardening  musl  sabotage  tlsify  passwords  /  crypt-dev  xvendor  /  Bugtraq  Full-Disclosure  linux-kernel  linux-netdev  linux-ext4  linux-hardening  linux-cve-announce  PHC 
Open Source and information security mailing list archives
 
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Date:   Mon, 17 Oct 2022 18:16:36 +0800
From:   kernel test robot <lkp@...el.com>
To:     Kees Cook <keescook@...omium.org>, Mimi Zohar <zohar@...ux.ibm.com>
Cc:     kbuild-all@...ts.01.org, Kees Cook <keescook@...omium.org>,
        Dmitry Kasatkin <dmitry.kasatkin@...il.com>,
        Paul Moore <paul@...l-moore.com>,
        James Morris <jmorris@...ei.org>,
        "Serge E. Hallyn" <serge@...lyn.com>, Takashi Iwai <tiwai@...e.de>,
        Jonathan McDowell <noodles@...com>,
        Casey Schaufler <casey@...aufler-ca.com>,
        linux-integrity@...r.kernel.org,
        linux-security-module@...r.kernel.org,
        Mickaël Salaün <mic@...ikod.net>,
        KP Singh <kpsingh@...nel.org>,
        John Johansen <john.johansen@...onical.com>,
        linux-kernel@...r.kernel.org, linux-hardening@...r.kernel.org
Subject: Re: [PATCH 5/9] LSM: Introduce inode_post_setattr hook

Hi Kees,

I love your patch! Yet something to improve:

[auto build test ERROR on kees/for-next/pstore]
[also build test ERROR on pcmoore-audit/next pcmoore-selinux/next linus/master v6.1-rc1 next-20221017]
[cannot apply to zohar-integrity/next-integrity]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url:    https://github.com/intel-lab-lkp/linux/commits/Kees-Cook/integrity-Move-hooks-into-LSM/20221014-063953
base:   https://git.kernel.org/pub/scm/linux/kernel/git/kees/linux.git for-next/pstore
patch link:    https://lore.kernel.org/r/20221013223654.659758-5-keescook%40chromium.org
patch subject: [PATCH 5/9] LSM: Introduce inode_post_setattr hook
config: m68k-randconfig-r002-20221012
compiler: m68k-linux-gcc (GCC) 12.1.0
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/intel-lab-lkp/linux/commit/4a05d49d9e6e09539a85db353b335221f1181f38
        git remote add linux-review https://github.com/intel-lab-lkp/linux
        git fetch --no-tags linux-review Kees-Cook/integrity-Move-hooks-into-LSM/20221014-063953
        git checkout 4a05d49d9e6e09539a85db353b335221f1181f38
        # save the config file
        mkdir build_dir && cp config build_dir/.config
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-12.1.0 make.cross W=1 O=build_dir ARCH=m68k SHELL=/bin/bash

If you fix the issue, kindly add following tag where applicable
| Reported-by: kernel test robot <lkp@...el.com>

All errors (new ones prefixed by >>):

   fs/attr.c: In function 'notify_change':
>> fs/attr.c:426:17: error: implicit declaration of function 'security_inode_post_setattr'; did you mean 'security_inode_post_setxattr'? [-Werror=implicit-function-declaration]
     426 |                 security_inode_post_setattr(mnt_userns, dentry, ia_valid);
         |                 ^~~~~~~~~~~~~~~~~~~~~~~~~~~
         |                 security_inode_post_setxattr
   cc1: some warnings being treated as errors


vim +426 fs/attr.c

   290	
   291	/**
   292	 * notify_change - modify attributes of a filesytem object
   293	 * @mnt_userns:	user namespace of the mount the inode was found from
   294	 * @dentry:	object affected
   295	 * @attr:	new attributes
   296	 * @delegated_inode: returns inode, if the inode is delegated
   297	 *
   298	 * The caller must hold the i_mutex on the affected object.
   299	 *
   300	 * If notify_change discovers a delegation in need of breaking,
   301	 * it will return -EWOULDBLOCK and return a reference to the inode in
   302	 * delegated_inode.  The caller should then break the delegation and
   303	 * retry.  Because breaking a delegation may take a long time, the
   304	 * caller should drop the i_mutex before doing so.
   305	 *
   306	 * Alternatively, a caller may pass NULL for delegated_inode.  This may
   307	 * be appropriate for callers that expect the underlying filesystem not
   308	 * to be NFS exported.  Also, passing NULL is fine for callers holding
   309	 * the file open for write, as there can be no conflicting delegation in
   310	 * that case.
   311	 *
   312	 * If the inode has been found through an idmapped mount the user namespace of
   313	 * the vfsmount must be passed through @mnt_userns. This function will then
   314	 * take care to map the inode according to @mnt_userns before checking
   315	 * permissions. On non-idmapped mounts or if permission checking is to be
   316	 * performed on the raw inode simply passs init_user_ns.
   317	 */
   318	int notify_change(struct user_namespace *mnt_userns, struct dentry *dentry,
   319			  struct iattr *attr, struct inode **delegated_inode)
   320	{
   321		struct inode *inode = dentry->d_inode;
   322		umode_t mode = inode->i_mode;
   323		int error;
   324		struct timespec64 now;
   325		unsigned int ia_valid = attr->ia_valid;
   326	
   327		WARN_ON_ONCE(!inode_is_locked(inode));
   328	
   329		error = may_setattr(mnt_userns, inode, ia_valid);
   330		if (error)
   331			return error;
   332	
   333		if ((ia_valid & ATTR_MODE)) {
   334			umode_t amode = attr->ia_mode;
   335			/* Flag setting protected by i_mutex */
   336			if (is_sxid(amode))
   337				inode->i_flags &= ~S_NOSEC;
   338		}
   339	
   340		now = current_time(inode);
   341	
   342		attr->ia_ctime = now;
   343		if (!(ia_valid & ATTR_ATIME_SET))
   344			attr->ia_atime = now;
   345		else
   346			attr->ia_atime = timestamp_truncate(attr->ia_atime, inode);
   347		if (!(ia_valid & ATTR_MTIME_SET))
   348			attr->ia_mtime = now;
   349		else
   350			attr->ia_mtime = timestamp_truncate(attr->ia_mtime, inode);
   351	
   352		if (ia_valid & ATTR_KILL_PRIV) {
   353			error = security_inode_need_killpriv(dentry);
   354			if (error < 0)
   355				return error;
   356			if (error == 0)
   357				ia_valid = attr->ia_valid &= ~ATTR_KILL_PRIV;
   358		}
   359	
   360		/*
   361		 * We now pass ATTR_KILL_S*ID to the lower level setattr function so
   362		 * that the function has the ability to reinterpret a mode change
   363		 * that's due to these bits. This adds an implicit restriction that
   364		 * no function will ever call notify_change with both ATTR_MODE and
   365		 * ATTR_KILL_S*ID set.
   366		 */
   367		if ((ia_valid & (ATTR_KILL_SUID|ATTR_KILL_SGID)) &&
   368		    (ia_valid & ATTR_MODE))
   369			BUG();
   370	
   371		if (ia_valid & ATTR_KILL_SUID) {
   372			if (mode & S_ISUID) {
   373				ia_valid = attr->ia_valid |= ATTR_MODE;
   374				attr->ia_mode = (inode->i_mode & ~S_ISUID);
   375			}
   376		}
   377		if (ia_valid & ATTR_KILL_SGID) {
   378			if ((mode & (S_ISGID | S_IXGRP)) == (S_ISGID | S_IXGRP)) {
   379				if (!(ia_valid & ATTR_MODE)) {
   380					ia_valid = attr->ia_valid |= ATTR_MODE;
   381					attr->ia_mode = inode->i_mode;
   382				}
   383				attr->ia_mode &= ~S_ISGID;
   384			}
   385		}
   386		if (!(attr->ia_valid & ~(ATTR_KILL_SUID | ATTR_KILL_SGID)))
   387			return 0;
   388	
   389		/*
   390		 * Verify that uid/gid changes are valid in the target
   391		 * namespace of the superblock.
   392		 */
   393		if (ia_valid & ATTR_UID &&
   394		    !vfsuid_has_fsmapping(mnt_userns, inode->i_sb->s_user_ns,
   395					  attr->ia_vfsuid))
   396			return -EOVERFLOW;
   397		if (ia_valid & ATTR_GID &&
   398		    !vfsgid_has_fsmapping(mnt_userns, inode->i_sb->s_user_ns,
   399					  attr->ia_vfsgid))
   400			return -EOVERFLOW;
   401	
   402		/* Don't allow modifications of files with invalid uids or
   403		 * gids unless those uids & gids are being made valid.
   404		 */
   405		if (!(ia_valid & ATTR_UID) &&
   406		    !vfsuid_valid(i_uid_into_vfsuid(mnt_userns, inode)))
   407			return -EOVERFLOW;
   408		if (!(ia_valid & ATTR_GID) &&
   409		    !vfsgid_valid(i_gid_into_vfsgid(mnt_userns, inode)))
   410			return -EOVERFLOW;
   411	
   412		error = security_inode_setattr(mnt_userns, dentry, attr);
   413		if (error)
   414			return error;
   415		error = try_break_deleg(inode, delegated_inode);
   416		if (error)
   417			return error;
   418	
   419		if (inode->i_op->setattr)
   420			error = inode->i_op->setattr(mnt_userns, dentry, attr);
   421		else
   422			error = simple_setattr(mnt_userns, dentry, attr);
   423	
   424		if (!error) {
   425			fsnotify_change(dentry, ia_valid);
 > 426			security_inode_post_setattr(mnt_userns, dentry, ia_valid);

-- 
0-DAY CI Kernel Test Service
https://01.org/lkp

View attachment "config" of type "text/plain" (122001 bytes)

Powered by blists - more mailing lists