| 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
| ||
|
Message-ID: <202210171846.fk6J9DmH-lkp@intel.com>
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