| Message ID | 20240812144936.1616628-1-mic@digikod.net (mailing list archive) |
|---|---|
| State | Handled Elsewhere |
| Delegated to: | Paul Moore |
| Headers | show |
| Series | fs,security: Fix file_set_fowner LSM hook inconsistencies | expand |
On Mon, Aug 12, 2024 at 10:49 AM Mickaël Salaün <mic@digikod.net> wrote: > diff --git a/include/linux/lsm_hook_defs.h b/include/linux/lsm_hook_defs.h > index 44488b1ab9a9..974bcc1c8f8f 100644 > --- a/include/linux/lsm_hook_defs.h > +++ b/include/linux/lsm_hook_defs.h > @@ -196,7 +196,6 @@ LSM_HOOK(int, 0, file_mprotect, struct vm_area_struct *vma, > LSM_HOOK(int, 0, file_lock, struct file *file, unsigned int cmd) > LSM_HOOK(int, 0, file_fcntl, struct file *file, unsigned int cmd, > unsigned long arg) > -LSM_HOOK(void, LSM_RET_VOID, file_set_fowner, struct file *file) As I mentioned in the other thread, I don't want to see the file_set_owner hook removed at this point in time. I'm open to the idea of moving it around, but as of right now I think it is important to keep it around. > LSM_HOOK(int, 0, file_send_sigiotask, struct task_struct *tsk, > struct fown_struct *fown, int sig) > LSM_HOOK(int, 0, file_receive, struct file *file)
Hi Mickaël,
kernel test robot noticed the following build errors:
[auto build test ERROR on pcmoore-selinux/next]
[also build test ERROR on linus/master v6.11-rc3 next-20240812]
[cannot apply to brauner-vfs/vfs.all]
[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/Micka-l-Sala-n/fs-security-Fix-file_set_fowner-LSM-hook-inconsistencies/20240813-004648
base: https://git.kernel.org/pub/scm/linux/kernel/git/pcmoore/selinux.git next
patch link: https://lore.kernel.org/r/20240812144936.1616628-1-mic%40digikod.net
patch subject: [PATCH] fs,security: Fix file_set_fowner LSM hook inconsistencies
config: i386-buildonly-randconfig-002-20240813 (https://download.01.org/0day-ci/archive/20240813/202408130919.naHeqbVw-lkp@intel.com/config)
compiler: gcc-12 (Debian 12.2.0-14) 12.2.0
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20240813/202408130919.naHeqbVw-lkp@intel.com/reproduce)
If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202408130919.naHeqbVw-lkp@intel.com/
All errors (new ones prefixed by >>):
In file included from include/linux/rculist.h:11,
from include/linux/dcache.h:8,
from include/linux/fs.h:8,
from include/uapi/linux/aio_abi.h:31,
from include/linux/syscalls.h:82,
from fs/fcntl.c:8:
fs/fcntl.c: In function 'f_getowner_uids':
>> fs/fcntl.c:250:50: error: invalid type argument of '->' (have 'struct fown_struct')
250 | fown_cred = rcu_dereference(filp->f_owner->cred);
| ^~
include/linux/rcupdate.h:527:17: note: in definition of macro '__rcu_dereference_check'
527 | typeof(*p) *local = (typeof(*p) *__force)READ_ONCE(p); \
| ^
include/linux/rcupdate.h:747:28: note: in expansion of macro 'rcu_dereference_check'
747 | #define rcu_dereference(p) rcu_dereference_check(p, 0)
| ^~~~~~~~~~~~~~~~~~~~~
fs/fcntl.c:250:21: note: in expansion of macro 'rcu_dereference'
250 | fown_cred = rcu_dereference(filp->f_owner->cred);
| ^~~~~~~~~~~~~~~
>> fs/fcntl.c:250:50: error: invalid type argument of '->' (have 'struct fown_struct')
250 | fown_cred = rcu_dereference(filp->f_owner->cred);
| ^~
include/linux/rcupdate.h:527:38: note: in definition of macro '__rcu_dereference_check'
527 | typeof(*p) *local = (typeof(*p) *__force)READ_ONCE(p); \
| ^
include/linux/rcupdate.h:747:28: note: in expansion of macro 'rcu_dereference_check'
747 | #define rcu_dereference(p) rcu_dereference_check(p, 0)
| ^~~~~~~~~~~~~~~~~~~~~
fs/fcntl.c:250:21: note: in expansion of macro 'rcu_dereference'
250 | fown_cred = rcu_dereference(filp->f_owner->cred);
| ^~~~~~~~~~~~~~~
In file included from <command-line>:
>> fs/fcntl.c:250:50: error: invalid type argument of '->' (have 'struct fown_struct')
250 | fown_cred = rcu_dereference(filp->f_owner->cred);
| ^~
include/linux/compiler_types.h:490:23: note: in definition of macro '__compiletime_assert'
490 | if (!(condition)) \
| ^~~~~~~~~
include/linux/compiler_types.h:510:9: note: in expansion of macro '_compiletime_assert'
510 | _compiletime_assert(condition, msg, __compiletime_assert_, __COUNTER__)
| ^~~~~~~~~~~~~~~~~~~
include/asm-generic/rwonce.h:36:9: note: in expansion of macro 'compiletime_assert'
36 | compiletime_assert(__native_word(t) || sizeof(t) == sizeof(long long), \
| ^~~~~~~~~~~~~~~~~~
include/asm-generic/rwonce.h:36:28: note: in expansion of macro '__native_word'
36 | compiletime_assert(__native_word(t) || sizeof(t) == sizeof(long long), \
| ^~~~~~~~~~~~~
include/asm-generic/rwonce.h:49:9: note: in expansion of macro 'compiletime_assert_rwonce_type'
49 | compiletime_assert_rwonce_type(x); \
| ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
include/linux/rcupdate.h:527:50: note: in expansion of macro 'READ_ONCE'
527 | typeof(*p) *local = (typeof(*p) *__force)READ_ONCE(p); \
| ^~~~~~~~~
include/linux/rcupdate.h:675:9: note: in expansion of macro '__rcu_dereference_check'
675 | __rcu_dereference_check((p), __UNIQUE_ID(rcu), \
| ^~~~~~~~~~~~~~~~~~~~~~~
include/linux/rcupdate.h:747:28: note: in expansion of macro 'rcu_dereference_check'
747 | #define rcu_dereference(p) rcu_dereference_check(p, 0)
| ^~~~~~~~~~~~~~~~~~~~~
fs/fcntl.c:250:21: note: in expansion of macro 'rcu_dereference'
250 | fown_cred = rcu_dereference(filp->f_owner->cred);
| ^~~~~~~~~~~~~~~
>> fs/fcntl.c:250:50: error: invalid type argument of '->' (have 'struct fown_struct')
250 | fown_cred = rcu_dereference(filp->f_owner->cred);
| ^~
include/linux/compiler_types.h:490:23: note: in definition of macro '__compiletime_assert'
490 | if (!(condition)) \
| ^~~~~~~~~
include/linux/compiler_types.h:510:9: note: in expansion of macro '_compiletime_assert'
510 | _compiletime_assert(condition, msg, __compiletime_assert_, __COUNTER__)
| ^~~~~~~~~~~~~~~~~~~
include/asm-generic/rwonce.h:36:9: note: in expansion of macro 'compiletime_assert'
36 | compiletime_assert(__native_word(t) || sizeof(t) == sizeof(long long), \
| ^~~~~~~~~~~~~~~~~~
include/asm-generic/rwonce.h:36:28: note: in expansion of macro '__native_word'
36 | compiletime_assert(__native_word(t) || sizeof(t) == sizeof(long long), \
| ^~~~~~~~~~~~~
include/asm-generic/rwonce.h:49:9: note: in expansion of macro 'compiletime_assert_rwonce_type'
49 | compiletime_assert_rwonce_type(x); \
| ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
include/linux/rcupdate.h:527:50: note: in expansion of macro 'READ_ONCE'
527 | typeof(*p) *local = (typeof(*p) *__force)READ_ONCE(p); \
| ^~~~~~~~~
include/linux/rcupdate.h:675:9: note: in expansion of macro '__rcu_dereference_check'
675 | __rcu_dereference_check((p), __UNIQUE_ID(rcu), \
| ^~~~~~~~~~~~~~~~~~~~~~~
include/linux/rcupdate.h:747:28: note: in expansion of macro 'rcu_dereference_check'
747 | #define rcu_dereference(p) rcu_dereference_check(p, 0)
| ^~~~~~~~~~~~~~~~~~~~~
fs/fcntl.c:250:21: note: in expansion of macro 'rcu_dereference'
250 | fown_cred = rcu_dereference(filp->f_owner->cred);
| ^~~~~~~~~~~~~~~
>> fs/fcntl.c:250:50: error: invalid type argument of '->' (have 'struct fown_struct')
250 | fown_cred = rcu_dereference(filp->f_owner->cred);
| ^~
include/linux/compiler_types.h:490:23: note: in definition of macro '__compiletime_assert'
490 | if (!(condition)) \
| ^~~~~~~~~
include/linux/compiler_types.h:510:9: note: in expansion of macro '_compiletime_assert'
510 | _compiletime_assert(condition, msg, __compiletime_assert_, __COUNTER__)
| ^~~~~~~~~~~~~~~~~~~
include/asm-generic/rwonce.h:36:9: note: in expansion of macro 'compiletime_assert'
36 | compiletime_assert(__native_word(t) || sizeof(t) == sizeof(long long), \
| ^~~~~~~~~~~~~~~~~~
include/asm-generic/rwonce.h:36:28: note: in expansion of macro '__native_word'
36 | compiletime_assert(__native_word(t) || sizeof(t) == sizeof(long long), \
| ^~~~~~~~~~~~~
include/asm-generic/rwonce.h:49:9: note: in expansion of macro 'compiletime_assert_rwonce_type'
49 | compiletime_assert_rwonce_type(x); \
| ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
include/linux/rcupdate.h:527:50: note: in expansion of macro 'READ_ONCE'
527 | typeof(*p) *local = (typeof(*p) *__force)READ_ONCE(p); \
| ^~~~~~~~~
include/linux/rcupdate.h:675:9: note: in expansion of macro '__rcu_dereference_check'
675 | __rcu_dereference_check((p), __UNIQUE_ID(rcu), \
| ^~~~~~~~~~~~~~~~~~~~~~~
include/linux/rcupdate.h:747:28: note: in expansion of macro 'rcu_dereference_check'
747 | #define rcu_dereference(p) rcu_dereference_check(p, 0)
| ^~~~~~~~~~~~~~~~~~~~~
fs/fcntl.c:250:21: note: in expansion of macro 'rcu_dereference'
250 | fown_cred = rcu_dereference(filp->f_owner->cred);
| ^~~~~~~~~~~~~~~
>> fs/fcntl.c:250:50: error: invalid type argument of '->' (have 'struct fown_struct')
250 | fown_cred = rcu_dereference(filp->f_owner->cred);
| ^~
include/linux/compiler_types.h:490:23: note: in definition of macro '__compiletime_assert'
490 | if (!(condition)) \
| ^~~~~~~~~
include/linux/compiler_types.h:510:9: note: in expansion of macro '_compiletime_assert'
510 | _compiletime_assert(condition, msg, __compiletime_assert_, __COUNTER__)
| ^~~~~~~~~~~~~~~~~~~
include/asm-generic/rwonce.h:36:9: note: in expansion of macro 'compiletime_assert'
36 | compiletime_assert(__native_word(t) || sizeof(t) == sizeof(long long), \
| ^~~~~~~~~~~~~~~~~~
include/asm-generic/rwonce.h:36:28: note: in expansion of macro '__native_word'
36 | compiletime_assert(__native_word(t) || sizeof(t) == sizeof(long long), \
| ^~~~~~~~~~~~~
include/asm-generic/rwonce.h:49:9: note: in expansion of macro 'compiletime_assert_rwonce_type'
49 | compiletime_assert_rwonce_type(x); \
| ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
include/linux/rcupdate.h:527:50: note: in expansion of macro 'READ_ONCE'
527 | typeof(*p) *local = (typeof(*p) *__force)READ_ONCE(p); \
| ^~~~~~~~~
include/linux/rcupdate.h:675:9: note: in expansion of macro '__rcu_dereference_check'
675 | __rcu_dereference_check((p), __UNIQUE_ID(rcu), \
| ^~~~~~~~~~~~~~~~~~~~~~~
include/linux/rcupdate.h:747:28: note: in expansion of macro 'rcu_dereference_check'
747 | #define rcu_dereference(p) rcu_dereference_check(p, 0)
| ^~~~~~~~~~~~~~~~~~~~~
fs/fcntl.c:250:21: note: in expansion of macro 'rcu_dereference'
250 | fown_cred = rcu_dereference(filp->f_owner->cred);
| ^~~~~~~~~~~~~~~
>> fs/fcntl.c:250:50: error: invalid type argument of '->' (have 'struct fown_struct')
250 | fown_cred = rcu_dereference(filp->f_owner->cred);
| ^~
include/linux/compiler_types.h:490:23: note: in definition of macro '__compiletime_assert'
490 | if (!(condition)) \
| ^~~~~~~~~
include/linux/compiler_types.h:510:9: note: in expansion of macro '_compiletime_assert'
510 | _compiletime_assert(condition, msg, __compiletime_assert_, __COUNTER__)
| ^~~~~~~~~~~~~~~~~~~
include/asm-generic/rwonce.h:36:9: note: in expansion of macro 'compiletime_assert'
36 | compiletime_assert(__native_word(t) || sizeof(t) == sizeof(long long), \
| ^~~~~~~~~~~~~~~~~~
include/asm-generic/rwonce.h:49:9: note: in expansion of macro 'compiletime_assert_rwonce_type'
49 | compiletime_assert_rwonce_type(x); \
| ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
include/linux/rcupdate.h:527:50: note: in expansion of macro 'READ_ONCE'
527 | typeof(*p) *local = (typeof(*p) *__force)READ_ONCE(p); \
| ^~~~~~~~~
include/linux/rcupdate.h:675:9: note: in expansion of macro '__rcu_dereference_check'
675 | __rcu_dereference_check((p), __UNIQUE_ID(rcu), \
| ^~~~~~~~~~~~~~~~~~~~~~~
include/linux/rcupdate.h:747:28: note: in expansion of macro 'rcu_dereference_check'
747 | #define rcu_dereference(p) rcu_dereference_check(p, 0)
| ^~~~~~~~~~~~~~~~~~~~~
fs/fcntl.c:250:21: note: in expansion of macro 'rcu_dereference'
250 | fown_cred = rcu_dereference(filp->f_owner->cred);
| ^~~~~~~~~~~~~~~
>> fs/fcntl.c:250:50: error: invalid type argument of '->' (have 'struct fown_struct')
250 | fown_cred = rcu_dereference(filp->f_owner->cred);
| ^~
include/linux/compiler_types.h:466:27: note: in definition of macro '__unqual_scalar_typeof'
466 | _Generic((x), \
| ^
include/asm-generic/rwonce.h:50:9: note: in expansion of macro '__READ_ONCE'
50 | __READ_ONCE(x); \
| ^~~~~~~~~~~
include/linux/rcupdate.h:527:50: note: in expansion of macro 'READ_ONCE'
527 | typeof(*p) *local = (typeof(*p) *__force)READ_ONCE(p); \
| ^~~~~~~~~
include/linux/rcupdate.h:675:9: note: in expansion of macro '__rcu_dereference_check'
675 | __rcu_dereference_check((p), __UNIQUE_ID(rcu), \
| ^~~~~~~~~~~~~~~~~~~~~~~
include/linux/rcupdate.h:747:28: note: in expansion of macro 'rcu_dereference_check'
747 | #define rcu_dereference(p) rcu_dereference_check(p, 0)
| ^~~~~~~~~~~~~~~~~~~~~
fs/fcntl.c:250:21: note: in expansion of macro 'rcu_dereference'
250 | fown_cred = rcu_dereference(filp->f_owner->cred);
| ^~~~~~~~~~~~~~~
In file included from ./arch/x86/include/generated/asm/rwonce.h:1,
from include/linux/compiler.h:305,
from include/linux/export.h:5,
from include/linux/linkage.h:7,
from include/linux/fs.h:5:
>> fs/fcntl.c:250:50: error: invalid type argument of '->' (have 'struct fown_struct')
250 | fown_cred = rcu_dereference(filp->f_owner->cred);
| ^~
include/asm-generic/rwonce.h:44:73: note: in definition of macro '__READ_ONCE'
44 | #define __READ_ONCE(x) (*(const volatile __unqual_scalar_typeof(x) *)&(x))
| ^
include/linux/rcupdate.h:527:50: note: in expansion of macro 'READ_ONCE'
527 | typeof(*p) *local = (typeof(*p) *__force)READ_ONCE(p); \
| ^~~~~~~~~
include/linux/rcupdate.h:675:9: note: in expansion of macro '__rcu_dereference_check'
675 | __rcu_dereference_check((p), __UNIQUE_ID(rcu), \
| ^~~~~~~~~~~~~~~~~~~~~~~
include/linux/rcupdate.h:747:28: note: in expansion of macro 'rcu_dereference_check'
747 | #define rcu_dereference(p) rcu_dereference_check(p, 0)
| ^~~~~~~~~~~~~~~~~~~~~
fs/fcntl.c:250:21: note: in expansion of macro 'rcu_dereference'
250 | fown_cred = rcu_dereference(filp->f_owner->cred);
| ^~~~~~~~~~~~~~~
>> fs/fcntl.c:250:50: error: invalid type argument of '->' (have 'struct fown_struct')
250 | fown_cred = rcu_dereference(filp->f_owner->cred);
| ^~
include/linux/rcupdate.h:530:19: note: in definition of macro '__rcu_dereference_check'
530 | ((typeof(*p) __force __kernel *)(local)); \
| ^
include/linux/rcupdate.h:747:28: note: in expansion of macro 'rcu_dereference_check'
747 | #define rcu_dereference(p) rcu_dereference_check(p, 0)
| ^~~~~~~~~~~~~~~~~~~~~
fs/fcntl.c:250:21: note: in expansion of macro 'rcu_dereference'
250 | fown_cred = rcu_dereference(filp->f_owner->cred);
| ^~~~~~~~~~~~~~~
vim +250 fs/fcntl.c
239
240 #ifdef CONFIG_CHECKPOINT_RESTORE
241 static int f_getowner_uids(struct file *filp, unsigned long arg)
242 {
243 struct user_namespace *user_ns = current_user_ns();
244 uid_t __user *dst = (void __user *)arg;
245 const struct cred *fown_cred;
246 uid_t src[2];
247 int err;
248
249 rcu_read_lock();
> 250 fown_cred = rcu_dereference(filp->f_owner->cred);
251 src[0] = from_kuid(user_ns, fown_cred->uid);
252 src[1] = from_kuid(user_ns, fown_cred->euid);
253 rcu_read_unlock();
254
255 err = put_user(src[0], &dst[0]);
256 err |= put_user(src[1], &dst[1]);
257
258 return err;
259 }
260 #else
261 static int f_getowner_uids(struct file *filp, unsigned long arg)
262 {
263 return -EINVAL;
264 }
265 #endif
266
Hi Mickaël, kernel test robot noticed the following build errors: [auto build test ERROR on pcmoore-selinux/next] [also build test ERROR on linus/master v6.11-rc3 next-20240812] [cannot apply to brauner-vfs/vfs.all] [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/Micka-l-Sala-n/fs-security-Fix-file_set_fowner-LSM-hook-inconsistencies/20240813-004648 base: https://git.kernel.org/pub/scm/linux/kernel/git/pcmoore/selinux.git next patch link: https://lore.kernel.org/r/20240812144936.1616628-1-mic%40digikod.net patch subject: [PATCH] fs,security: Fix file_set_fowner LSM hook inconsistencies config: x86_64-buildonly-randconfig-003-20240813 (https://download.01.org/0day-ci/archive/20240813/202408130900.y6a7Si8X-lkp@intel.com/config) compiler: clang version 18.1.5 (https://github.com/llvm/llvm-project 617a15a9eac96088ae5e9134248d8236e34b91b1) reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20240813/202408130900.y6a7Si8X-lkp@intel.com/reproduce) If you fix the issue in a separate patch/commit (i.e. not just a new version of the same patch/commit), kindly add following tags | Reported-by: kernel test robot <lkp@intel.com> | Closes: https://lore.kernel.org/oe-kbuild-all/202408130900.y6a7Si8X-lkp@intel.com/ All errors (new ones prefixed by >>): >> fs/fcntl.c:250:43: error: member reference type 'struct fown_struct' is not a pointer; did you mean to use '.'? 250 | fown_cred = rcu_dereference(filp->f_owner->cred); | ~~~~~~~~~~~~~^~ | . include/linux/rcupdate.h:747:50: note: expanded from macro 'rcu_dereference' 747 | #define rcu_dereference(p) rcu_dereference_check(p, 0) | ^ include/linux/rcupdate.h:675:27: note: expanded from macro 'rcu_dereference_check' 675 | __rcu_dereference_check((p), __UNIQUE_ID(rcu), \ | ^ include/linux/rcupdate.h:527:10: note: expanded from macro '__rcu_dereference_check' 527 | typeof(*p) *local = (typeof(*p) *__force)READ_ONCE(p); \ | ^ >> fs/fcntl.c:250:43: error: member reference type 'struct fown_struct' is not a pointer; did you mean to use '.'? 250 | fown_cred = rcu_dereference(filp->f_owner->cred); | ~~~~~~~~~~~~~^~ | . include/linux/rcupdate.h:747:50: note: expanded from macro 'rcu_dereference' 747 | #define rcu_dereference(p) rcu_dereference_check(p, 0) | ^ include/linux/rcupdate.h:675:27: note: expanded from macro 'rcu_dereference_check' 675 | __rcu_dereference_check((p), __UNIQUE_ID(rcu), \ | ^ include/linux/rcupdate.h:527:31: note: expanded from macro '__rcu_dereference_check' 527 | typeof(*p) *local = (typeof(*p) *__force)READ_ONCE(p); \ | ^ >> fs/fcntl.c:250:43: error: member reference type 'struct fown_struct' is not a pointer; did you mean to use '.'? 250 | fown_cred = rcu_dereference(filp->f_owner->cred); | ~~~~~~~~~~~~~^~ | . include/linux/rcupdate.h:747:50: note: expanded from macro 'rcu_dereference' 747 | #define rcu_dereference(p) rcu_dereference_check(p, 0) | ^ include/linux/rcupdate.h:675:27: note: expanded from macro 'rcu_dereference_check' 675 | __rcu_dereference_check((p), __UNIQUE_ID(rcu), \ | ^ include/linux/rcupdate.h:527:53: note: expanded from macro '__rcu_dereference_check' 527 | typeof(*p) *local = (typeof(*p) *__force)READ_ONCE(p); \ | ^ note: (skipping 3 expansions in backtrace; use -fmacro-backtrace-limit=0 to see all) include/linux/compiler_types.h:510:22: note: expanded from macro 'compiletime_assert' 510 | _compiletime_assert(condition, msg, __compiletime_assert_, __COUNTER__) | ^~~~~~~~~ include/linux/compiler_types.h:498:23: note: expanded from macro '_compiletime_assert' 498 | __compiletime_assert(condition, msg, prefix, suffix) | ^~~~~~~~~ include/linux/compiler_types.h:490:9: note: expanded from macro '__compiletime_assert' 490 | if (!(condition)) \ | ^~~~~~~~~ >> fs/fcntl.c:250:43: error: member reference type 'struct fown_struct' is not a pointer; did you mean to use '.'? 250 | fown_cred = rcu_dereference(filp->f_owner->cred); | ~~~~~~~~~~~~~^~ | . include/linux/rcupdate.h:747:50: note: expanded from macro 'rcu_dereference' 747 | #define rcu_dereference(p) rcu_dereference_check(p, 0) | ^ include/linux/rcupdate.h:675:27: note: expanded from macro 'rcu_dereference_check' 675 | __rcu_dereference_check((p), __UNIQUE_ID(rcu), \ | ^ include/linux/rcupdate.h:527:53: note: expanded from macro '__rcu_dereference_check' 527 | typeof(*p) *local = (typeof(*p) *__force)READ_ONCE(p); \ | ^ note: (skipping 3 expansions in backtrace; use -fmacro-backtrace-limit=0 to see all) include/linux/compiler_types.h:510:22: note: expanded from macro 'compiletime_assert' 510 | _compiletime_assert(condition, msg, __compiletime_assert_, __COUNTER__) | ^~~~~~~~~ include/linux/compiler_types.h:498:23: note: expanded from macro '_compiletime_assert' 498 | __compiletime_assert(condition, msg, prefix, suffix) | ^~~~~~~~~ include/linux/compiler_types.h:490:9: note: expanded from macro '__compiletime_assert' 490 | if (!(condition)) \ | ^~~~~~~~~ >> fs/fcntl.c:250:43: error: member reference type 'struct fown_struct' is not a pointer; did you mean to use '.'? 250 | fown_cred = rcu_dereference(filp->f_owner->cred); | ~~~~~~~~~~~~~^~ | . include/linux/rcupdate.h:747:50: note: expanded from macro 'rcu_dereference' 747 | #define rcu_dereference(p) rcu_dereference_check(p, 0) | ^ include/linux/rcupdate.h:675:27: note: expanded from macro 'rcu_dereference_check' 675 | __rcu_dereference_check((p), __UNIQUE_ID(rcu), \ | ^ include/linux/rcupdate.h:527:53: note: expanded from macro '__rcu_dereference_check' 527 | typeof(*p) *local = (typeof(*p) *__force)READ_ONCE(p); \ | ^ note: (skipping 3 expansions in backtrace; use -fmacro-backtrace-limit=0 to see all) include/linux/compiler_types.h:510:22: note: expanded from macro 'compiletime_assert' 510 | _compiletime_assert(condition, msg, __compiletime_assert_, __COUNTER__) | ^~~~~~~~~ include/linux/compiler_types.h:498:23: note: expanded from macro '_compiletime_assert' 498 | __compiletime_assert(condition, msg, prefix, suffix) | ^~~~~~~~~ include/linux/compiler_types.h:490:9: note: expanded from macro '__compiletime_assert' 490 | if (!(condition)) \ | ^~~~~~~~~ >> fs/fcntl.c:250:43: error: member reference type 'struct fown_struct' is not a pointer; did you mean to use '.'? 250 | fown_cred = rcu_dereference(filp->f_owner->cred); | ~~~~~~~~~~~~~^~ | . include/linux/rcupdate.h:747:50: note: expanded from macro 'rcu_dereference' 747 | #define rcu_dereference(p) rcu_dereference_check(p, 0) | ^ include/linux/rcupdate.h:675:27: note: expanded from macro 'rcu_dereference_check' 675 | __rcu_dereference_check((p), __UNIQUE_ID(rcu), \ | ^ include/linux/rcupdate.h:527:53: note: expanded from macro '__rcu_dereference_check' 527 | typeof(*p) *local = (typeof(*p) *__force)READ_ONCE(p); \ | ^ note: (skipping 3 expansions in backtrace; use -fmacro-backtrace-limit=0 to see all) include/linux/compiler_types.h:510:22: note: expanded from macro 'compiletime_assert' 510 | _compiletime_assert(condition, msg, __compiletime_assert_, __COUNTER__) | ^~~~~~~~~ include/linux/compiler_types.h:498:23: note: expanded from macro '_compiletime_assert' 498 | __compiletime_assert(condition, msg, prefix, suffix) | ^~~~~~~~~ include/linux/compiler_types.h:490:9: note: expanded from macro '__compiletime_assert' 490 | if (!(condition)) \ | ^~~~~~~~~ >> fs/fcntl.c:250:43: error: member reference type 'struct fown_struct' is not a pointer; did you mean to use '.'? 250 | fown_cred = rcu_dereference(filp->f_owner->cred); | ~~~~~~~~~~~~~^~ | . include/linux/rcupdate.h:747:50: note: expanded from macro 'rcu_dereference' 747 | #define rcu_dereference(p) rcu_dereference_check(p, 0) | ^ include/linux/rcupdate.h:675:27: note: expanded from macro 'rcu_dereference_check' 675 | __rcu_dereference_check((p), __UNIQUE_ID(rcu), \ | ^ include/linux/rcupdate.h:527:53: note: expanded from macro '__rcu_dereference_check' 527 | typeof(*p) *local = (typeof(*p) *__force)READ_ONCE(p); \ | ^ note: (skipping 2 expansions in backtrace; use -fmacro-backtrace-limit=0 to see all) include/linux/compiler_types.h:510:22: note: expanded from macro 'compiletime_assert' 510 | _compiletime_assert(condition, msg, __compiletime_assert_, __COUNTER__) | ^~~~~~~~~ include/linux/compiler_types.h:498:23: note: expanded from macro '_compiletime_assert' 498 | __compiletime_assert(condition, msg, prefix, suffix) | ^~~~~~~~~ include/linux/compiler_types.h:490:9: note: expanded from macro '__compiletime_assert' 490 | if (!(condition)) \ | ^~~~~~~~~ >> fs/fcntl.c:250:43: error: member reference type 'struct fown_struct' is not a pointer; did you mean to use '.'? 250 | fown_cred = rcu_dereference(filp->f_owner->cred); | ~~~~~~~~~~~~~^~ | . include/linux/rcupdate.h:747:50: note: expanded from macro 'rcu_dereference' 747 | #define rcu_dereference(p) rcu_dereference_check(p, 0) | ^ include/linux/rcupdate.h:675:27: note: expanded from macro 'rcu_dereference_check' 675 | __rcu_dereference_check((p), __UNIQUE_ID(rcu), \ | ^ include/linux/rcupdate.h:527:53: note: expanded from macro '__rcu_dereference_check' 527 | typeof(*p) *local = (typeof(*p) *__force)READ_ONCE(p); \ | ^ include/asm-generic/rwonce.h:50:14: note: expanded from macro 'READ_ONCE' 50 | __READ_ONCE(x); \ | ^ include/asm-generic/rwonce.h:44:65: note: expanded from macro '__READ_ONCE' 44 | #define __READ_ONCE(x) (*(const volatile __unqual_scalar_typeof(x) *)&(x)) | ^ include/linux/compiler_types.h:466:13: note: expanded from macro '__unqual_scalar_typeof' 466 | _Generic((x), \ | ^ >> fs/fcntl.c:250:43: error: member reference type 'struct fown_struct' is not a pointer; did you mean to use '.'? 250 | fown_cred = rcu_dereference(filp->f_owner->cred); | ~~~~~~~~~~~~~^~ | . include/linux/rcupdate.h:747:50: note: expanded from macro 'rcu_dereference' 747 | #define rcu_dereference(p) rcu_dereference_check(p, 0) | ^ include/linux/rcupdate.h:675:27: note: expanded from macro 'rcu_dereference_check' 675 | __rcu_dereference_check((p), __UNIQUE_ID(rcu), \ | ^ include/linux/rcupdate.h:527:53: note: expanded from macro '__rcu_dereference_check' 527 | typeof(*p) *local = (typeof(*p) *__force)READ_ONCE(p); \ | ^ include/asm-generic/rwonce.h:50:14: note: expanded from macro 'READ_ONCE' 50 | __READ_ONCE(x); \ | ^ include/asm-generic/rwonce.h:44:65: note: expanded from macro '__READ_ONCE' 44 | #define __READ_ONCE(x) (*(const volatile __unqual_scalar_typeof(x) *)&(x)) | ^ include/linux/compiler_types.h:473:15: note: expanded from macro '__unqual_scalar_typeof' 473 | default: (x))) | ^ >> fs/fcntl.c:250:43: error: member reference type 'struct fown_struct' is not a pointer; did you mean to use '.'? 250 | fown_cred = rcu_dereference(filp->f_owner->cred); | ~~~~~~~~~~~~~^~ | . include/linux/rcupdate.h:747:50: note: expanded from macro 'rcu_dereference' 747 | #define rcu_dereference(p) rcu_dereference_check(p, 0) | ^ include/linux/rcupdate.h:675:27: note: expanded from macro 'rcu_dereference_check' 675 | __rcu_dereference_check((p), __UNIQUE_ID(rcu), \ | ^ include/linux/rcupdate.h:527:53: note: expanded from macro '__rcu_dereference_check' 527 | typeof(*p) *local = (typeof(*p) *__force)READ_ONCE(p); \ | ^ include/asm-generic/rwonce.h:50:14: note: expanded from macro 'READ_ONCE' 50 | __READ_ONCE(x); \ | ^ include/asm-generic/rwonce.h:44:72: note: expanded from macro '__READ_ONCE' 44 | #define __READ_ONCE(x) (*(const volatile __unqual_scalar_typeof(x) *)&(x)) | ^ >> fs/fcntl.c:250:14: error: cannot take the address of an rvalue of type 'const struct cred *' 250 | fown_cred = rcu_dereference(filp->f_owner->cred); | ^ ~~~~~~~~~~~~~~~~~~~ include/linux/rcupdate.h:747:28: note: expanded from macro 'rcu_dereference' 747 | #define rcu_dereference(p) rcu_dereference_check(p, 0) | ^ ~ include/linux/rcupdate.h:675:2: note: expanded from macro 'rcu_dereference_check' 675 | __rcu_dereference_check((p), __UNIQUE_ID(rcu), \ | ^ ~ include/linux/rcupdate.h:527:43: note: expanded from macro '__rcu_dereference_check' 527 | typeof(*p) *local = (typeof(*p) *__force)READ_ONCE(p); \ | ^ ~ include/asm-generic/rwonce.h:50:2: note: expanded from macro 'READ_ONCE' 50 | __READ_ONCE(x); \ | ^ ~ include/asm-generic/rwonce.h:44:70: note: expanded from macro '__READ_ONCE' 44 | #define __READ_ONCE(x) (*(const volatile __unqual_scalar_typeof(x) *)&(x)) | ^ ~ >> fs/fcntl.c:250:43: error: member reference type 'struct fown_struct' is not a pointer; did you mean to use '.'? 250 | fown_cred = rcu_dereference(filp->f_owner->cred); | ~~~~~~~~~~~~~^~ | . include/linux/rcupdate.h:747:50: note: expanded from macro 'rcu_dereference' 747 | #define rcu_dereference(p) rcu_dereference_check(p, 0) | ^ include/linux/rcupdate.h:675:27: note: expanded from macro 'rcu_dereference_check' 675 | __rcu_dereference_check((p), __UNIQUE_ID(rcu), \ | ^ include/linux/rcupdate.h:530:12: note: expanded from macro '__rcu_dereference_check' 530 | ((typeof(*p) __force __kernel *)(local)); \ | ^ 12 errors generated. vim +250 fs/fcntl.c 239 240 #ifdef CONFIG_CHECKPOINT_RESTORE 241 static int f_getowner_uids(struct file *filp, unsigned long arg) 242 { 243 struct user_namespace *user_ns = current_user_ns(); 244 uid_t __user *dst = (void __user *)arg; 245 const struct cred *fown_cred; 246 uid_t src[2]; 247 int err; 248 249 rcu_read_lock(); > 250 fown_cred = rcu_dereference(filp->f_owner->cred); 251 src[0] = from_kuid(user_ns, fown_cred->uid); 252 src[1] = from_kuid(user_ns, fown_cred->euid); 253 rcu_read_unlock(); 254 255 err = put_user(src[0], &dst[0]); 256 err |= put_user(src[1], &dst[1]); 257 258 return err; 259 } 260 #else 261 static int f_getowner_uids(struct file *filp, unsigned long arg) 262 { 263 return -EINVAL; 264 } 265 #endif 266
Hi Mickaël,
kernel test robot noticed the following build warnings:
[auto build test WARNING on pcmoore-selinux/next]
[also build test WARNING on linus/master v6.11-rc3 next-20240812]
[cannot apply to brauner-vfs/vfs.all]
[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/Micka-l-Sala-n/fs-security-Fix-file_set_fowner-LSM-hook-inconsistencies/20240813-004648
base: https://git.kernel.org/pub/scm/linux/kernel/git/pcmoore/selinux.git next
patch link: https://lore.kernel.org/r/20240812144936.1616628-1-mic%40digikod.net
patch subject: [PATCH] fs,security: Fix file_set_fowner LSM hook inconsistencies
config: m68k-allmodconfig (https://download.01.org/0day-ci/archive/20240813/202408130946.6oMWnayg-lkp@intel.com/config)
compiler: m68k-linux-gcc (GCC) 14.1.0
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20240813/202408130946.6oMWnayg-lkp@intel.com/reproduce)
If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202408130946.6oMWnayg-lkp@intel.com/
All warnings (new ones prefixed by >>):
security/smack/smack_lsm.c: In function 'smack_file_send_sigiotask':
>> security/smack/smack_lsm.c:1913:22: warning: variable 'file' set but not used [-Wunused-but-set-variable]
1913 | struct file *file;
| ^~~~
vim +/file +1913 security/smack/smack_lsm.c
7898e1f8e9eb1b Casey Schaufler 2011-01-17 1895
e114e473771c84 Casey Schaufler 2008-02-04 1896 /**
e114e473771c84 Casey Schaufler 2008-02-04 1897 * smack_file_send_sigiotask - Smack on sigio
e114e473771c84 Casey Schaufler 2008-02-04 1898 * @tsk: The target task
e114e473771c84 Casey Schaufler 2008-02-04 1899 * @fown: the object the signal come from
e114e473771c84 Casey Schaufler 2008-02-04 1900 * @signum: unused
e114e473771c84 Casey Schaufler 2008-02-04 1901 *
e114e473771c84 Casey Schaufler 2008-02-04 1902 * Allow a privileged task to get signals even if it shouldn't
e114e473771c84 Casey Schaufler 2008-02-04 1903 *
e114e473771c84 Casey Schaufler 2008-02-04 1904 * Returns 0 if a subject with the object's smack could
e114e473771c84 Casey Schaufler 2008-02-04 1905 * write to the task, an error code otherwise.
e114e473771c84 Casey Schaufler 2008-02-04 1906 */
e114e473771c84 Casey Schaufler 2008-02-04 1907 static int smack_file_send_sigiotask(struct task_struct *tsk,
e114e473771c84 Casey Schaufler 2008-02-04 1908 struct fown_struct *fown, int signum)
e114e473771c84 Casey Schaufler 2008-02-04 1909 {
2f823ff8bec03a Casey Schaufler 2013-05-22 1910 struct smack_known *skp;
b17103a8b8ae9c Casey Schaufler 2018-11-09 1911 struct smack_known *tkp = smk_of_task(smack_cred(tsk->cred));
dcb569cf6ac99c Casey Schaufler 2018-09-18 1912 const struct cred *tcred;
e114e473771c84 Casey Schaufler 2008-02-04 @1913 struct file *file;
e114e473771c84 Casey Schaufler 2008-02-04 1914 int rc;
ecfcc53fef3c35 Etienne Basset 2009-04-08 1915 struct smk_audit_info ad;
e114e473771c84 Casey Schaufler 2008-02-04 1916
e114e473771c84 Casey Schaufler 2008-02-04 1917 /*
e114e473771c84 Casey Schaufler 2008-02-04 1918 * struct fown_struct is never outside the context of a struct file
e114e473771c84 Casey Schaufler 2008-02-04 1919 */
e114e473771c84 Casey Schaufler 2008-02-04 1920 file = container_of(fown, struct file, f_owner);
7898e1f8e9eb1b Casey Schaufler 2011-01-17 1921
ecfcc53fef3c35 Etienne Basset 2009-04-08 1922 /* we don't log here as rc can be overriden */
32192c8e14ea34 Mickaël Salaün 2024-08-12 1923 skp = smk_of_task(smack_cred(rcu_dereference(fown->cred)));
c60b906673eebb Casey Schaufler 2016-08-30 1924 rc = smk_access(skp, tkp, MAY_DELIVER, NULL);
c60b906673eebb Casey Schaufler 2016-08-30 1925 rc = smk_bu_note("sigiotask", skp, tkp, MAY_DELIVER, rc);
dcb569cf6ac99c Casey Schaufler 2018-09-18 1926
dcb569cf6ac99c Casey Schaufler 2018-09-18 1927 rcu_read_lock();
dcb569cf6ac99c Casey Schaufler 2018-09-18 1928 tcred = __task_cred(tsk);
dcb569cf6ac99c Casey Schaufler 2018-09-18 1929 if (rc != 0 && smack_privileged_cred(CAP_MAC_OVERRIDE, tcred))
ecfcc53fef3c35 Etienne Basset 2009-04-08 1930 rc = 0;
dcb569cf6ac99c Casey Schaufler 2018-09-18 1931 rcu_read_unlock();
ecfcc53fef3c35 Etienne Basset 2009-04-08 1932
ecfcc53fef3c35 Etienne Basset 2009-04-08 1933 smk_ad_init(&ad, __func__, LSM_AUDIT_DATA_TASK);
ecfcc53fef3c35 Etienne Basset 2009-04-08 1934 smk_ad_setfield_u_tsk(&ad, tsk);
c60b906673eebb Casey Schaufler 2016-08-30 1935 smack_log(skp->smk_known, tkp->smk_known, MAY_DELIVER, rc, &ad);
e114e473771c84 Casey Schaufler 2008-02-04 1936 return rc;
e114e473771c84 Casey Schaufler 2008-02-04 1937 }
e114e473771c84 Casey Schaufler 2008-02-04 1938
Hi Mickaël, kernel test robot noticed the following build warnings: [auto build test WARNING on pcmoore-selinux/next] [also build test WARNING on linus/master v6.11-rc3] [cannot apply to brauner-vfs/vfs.all next-20240813] [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/Micka-l-Sala-n/fs-security-Fix-file_set_fowner-LSM-hook-inconsistencies/20240813-004648 base: https://git.kernel.org/pub/scm/linux/kernel/git/pcmoore/selinux.git next patch link: https://lore.kernel.org/r/20240812144936.1616628-1-mic%40digikod.net patch subject: [PATCH] fs,security: Fix file_set_fowner LSM hook inconsistencies config: x86_64-randconfig-122-20240813 (https://download.01.org/0day-ci/archive/20240813/202408131900.xhbYFHR4-lkp@intel.com/config) compiler: gcc-11 (Debian 11.3.0-12) 11.3.0 reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20240813/202408131900.xhbYFHR4-lkp@intel.com/reproduce) If you fix the issue in a separate patch/commit (i.e. not just a new version of the same patch/commit), kindly add following tags | Reported-by: kernel test robot <lkp@intel.com> | Closes: https://lore.kernel.org/oe-kbuild-all/202408131900.xhbYFHR4-lkp@intel.com/ sparse warnings: (new ones prefixed by >>) >> fs/file_table.c:153:25: sparse: sparse: incorrect type in assignment (different address spaces) @@ expected struct cred const [noderef] __rcu *cred @@ got struct cred const * @@ fs/file_table.c:153:25: sparse: expected struct cred const [noderef] __rcu *cred fs/file_table.c:153:25: sparse: got struct cred const * >> fs/file_table.c:157:36: sparse: sparse: incorrect type in argument 1 (different address spaces) @@ expected struct cred const *cred @@ got struct cred const [noderef] __rcu *cred @@ fs/file_table.c:157:36: sparse: expected struct cred const *cred fs/file_table.c:157:36: sparse: got struct cred const [noderef] __rcu *cred fs/file_table.c:69:28: sparse: sparse: incorrect type in argument 1 (different address spaces) @@ expected struct cred const *cred @@ got struct cred const [noderef] __rcu *cred @@ fs/file_table.c:69:28: sparse: expected struct cred const *cred fs/file_table.c:69:28: sparse: got struct cred const [noderef] __rcu *cred fs/file_table.c:69:28: sparse: sparse: incorrect type in argument 1 (different address spaces) @@ expected struct cred const *cred @@ got struct cred const [noderef] __rcu *cred @@ fs/file_table.c:69:28: sparse: expected struct cred const *cred fs/file_table.c:69:28: sparse: got struct cred const [noderef] __rcu *cred vim +153 fs/file_table.c 147 148 static int init_file(struct file *f, int flags, const struct cred *cred) 149 { 150 int error; 151 152 f->f_cred = get_cred(cred); > 153 f->f_owner.cred = get_cred(cred); 154 error = security_file_alloc(f); 155 if (unlikely(error)) { 156 put_cred(f->f_cred); > 157 put_cred(f->f_owner.cred); 158 return error; 159 } 160 161 rwlock_init(&f->f_owner.lock); 162 spin_lock_init(&f->f_lock); 163 mutex_init(&f->f_pos_lock); 164 f->f_flags = flags; 165 f->f_mode = OPEN_FMODE(flags); 166 /* f->f_version: 0 */ 167 168 /* 169 * We're SLAB_TYPESAFE_BY_RCU so initialize f_count last. While 170 * fget-rcu pattern users need to be able to handle spurious 171 * refcount bumps we should reinitialize the reused file first. 172 */ 173 atomic_long_set(&f->f_count, 1); 174 return 0; 175 } 176
diff --git a/fs/fcntl.c b/fs/fcntl.c index 300e5d9ad913..4832d6b6759c 100644 --- a/fs/fcntl.c +++ b/fs/fcntl.c @@ -87,8 +87,8 @@ static int setfl(int fd, struct file * filp, unsigned int arg) return error; } -static void f_modown(struct file *filp, struct pid *pid, enum pid_type type, - int force) +void __f_setown(struct file *filp, struct pid *pid, enum pid_type type, + int force) { write_lock_irq(&filp->f_owner.lock); if (force || !filp->f_owner.pid) { @@ -96,21 +96,14 @@ static void f_modown(struct file *filp, struct pid *pid, enum pid_type type, filp->f_owner.pid = get_pid(pid); filp->f_owner.pid_type = type; - if (pid) { - const struct cred *cred = current_cred(); - filp->f_owner.uid = cred->uid; - filp->f_owner.euid = cred->euid; - } + if (pid) + put_cred(rcu_replace_pointer( + filp->f_owner.cred, + get_cred_rcu(current_cred()), + lockdep_is_held(&filp->f_owner.lock))); } write_unlock_irq(&filp->f_owner.lock); } - -void __f_setown(struct file *filp, struct pid *pid, enum pid_type type, - int force) -{ - security_file_set_fowner(filp); - f_modown(filp, pid, type, force); -} EXPORT_SYMBOL(__f_setown); int f_setown(struct file *filp, int who, int force) @@ -146,7 +139,7 @@ EXPORT_SYMBOL(f_setown); void f_delown(struct file *filp) { - f_modown(filp, NULL, PIDTYPE_TGID, 1); + __f_setown(filp, NULL, PIDTYPE_TGID, 1); } pid_t f_getown(struct file *filp) @@ -249,13 +242,15 @@ static int f_getowner_uids(struct file *filp, unsigned long arg) { struct user_namespace *user_ns = current_user_ns(); uid_t __user *dst = (void __user *)arg; + const struct cred *fown_cred; uid_t src[2]; int err; - read_lock_irq(&filp->f_owner.lock); - src[0] = from_kuid(user_ns, filp->f_owner.uid); - src[1] = from_kuid(user_ns, filp->f_owner.euid); - read_unlock_irq(&filp->f_owner.lock); + rcu_read_lock(); + fown_cred = rcu_dereference(filp->f_owner->cred); + src[0] = from_kuid(user_ns, fown_cred->uid); + src[1] = from_kuid(user_ns, fown_cred->euid); + rcu_read_unlock(); err = put_user(src[0], &dst[0]); err |= put_user(src[1], &dst[1]); @@ -737,14 +732,17 @@ static const __poll_t band_table[NSIGPOLL] = { static inline int sigio_perm(struct task_struct *p, struct fown_struct *fown, int sig) { - const struct cred *cred; + const struct cred *cred, *fown_cred; int ret; rcu_read_lock(); + fown_cred = rcu_dereference(fown->cred); cred = __task_cred(p); - ret = ((uid_eq(fown->euid, GLOBAL_ROOT_UID) || - uid_eq(fown->euid, cred->suid) || uid_eq(fown->euid, cred->uid) || - uid_eq(fown->uid, cred->suid) || uid_eq(fown->uid, cred->uid)) && + ret = ((uid_eq(fown_cred->euid, GLOBAL_ROOT_UID) || + uid_eq(fown_cred->euid, cred->suid) || + uid_eq(fown_cred->euid, cred->uid) || + uid_eq(fown_cred->uid, cred->suid) || + uid_eq(fown_cred->uid, cred->uid)) && !security_file_send_sigiotask(p, fown, sig)); rcu_read_unlock(); return ret; diff --git a/fs/file_table.c b/fs/file_table.c index 4f03beed4737..d28b76aef4f3 100644 --- a/fs/file_table.c +++ b/fs/file_table.c @@ -66,6 +66,7 @@ static inline void file_free(struct file *f) if (likely(!(f->f_mode & FMODE_NOACCOUNT))) percpu_counter_dec(&nr_files); put_cred(f->f_cred); + put_cred(f->f_owner.cred); if (unlikely(f->f_mode & FMODE_BACKING)) { path_put(backing_file_user_path(f)); kfree(backing_file(f)); @@ -149,9 +150,11 @@ static int init_file(struct file *f, int flags, const struct cred *cred) int error; f->f_cred = get_cred(cred); + f->f_owner.cred = get_cred(cred); error = security_file_alloc(f); if (unlikely(error)) { put_cred(f->f_cred); + put_cred(f->f_owner.cred); return error; } diff --git a/include/linux/fs.h b/include/linux/fs.h index 0283cf366c2a..345e8ff6d49a 100644 --- a/include/linux/fs.h +++ b/include/linux/fs.h @@ -942,7 +942,7 @@ struct fown_struct { rwlock_t lock; /* protects pid, uid, euid fields */ struct pid *pid; /* pid or -pgrp where SIGIO should be sent */ enum pid_type pid_type; /* Kind of process group SIGIO should be sent to */ - kuid_t uid, euid; /* uid/euid of process setting the owner */ + const struct cred __rcu *cred;/* cred of process setting the owner */ int signum; /* posix.1b rt signal to be delivered on IO */ }; diff --git a/include/linux/lsm_hook_defs.h b/include/linux/lsm_hook_defs.h index 44488b1ab9a9..974bcc1c8f8f 100644 --- a/include/linux/lsm_hook_defs.h +++ b/include/linux/lsm_hook_defs.h @@ -196,7 +196,6 @@ LSM_HOOK(int, 0, file_mprotect, struct vm_area_struct *vma, LSM_HOOK(int, 0, file_lock, struct file *file, unsigned int cmd) LSM_HOOK(int, 0, file_fcntl, struct file *file, unsigned int cmd, unsigned long arg) -LSM_HOOK(void, LSM_RET_VOID, file_set_fowner, struct file *file) LSM_HOOK(int, 0, file_send_sigiotask, struct task_struct *tsk, struct fown_struct *fown, int sig) LSM_HOOK(int, 0, file_receive, struct file *file) diff --git a/include/linux/security.h b/include/linux/security.h index de3af33e6ff5..20357efa4a77 100644 --- a/include/linux/security.h +++ b/include/linux/security.h @@ -415,7 +415,6 @@ int security_file_mprotect(struct vm_area_struct *vma, unsigned long reqprot, unsigned long prot); int security_file_lock(struct file *file, unsigned int cmd); int security_file_fcntl(struct file *file, unsigned int cmd, unsigned long arg); -void security_file_set_fowner(struct file *file); int security_file_send_sigiotask(struct task_struct *tsk, struct fown_struct *fown, int sig); int security_file_receive(struct file *file); diff --git a/security/security.c b/security/security.c index e5ca08789f74..34e7f5c86af5 100644 --- a/security/security.c +++ b/security/security.c @@ -2898,20 +2898,6 @@ int security_file_fcntl(struct file *file, unsigned int cmd, unsigned long arg) return call_int_hook(file_fcntl, file, cmd, arg); } -/** - * security_file_set_fowner() - Set the file owner info in the LSM blob - * @file: the file - * - * Save owner security information (typically from current->security) in - * file->f_security for later use by the send_sigiotask hook. - * - * Return: Returns 0 on success. - */ -void security_file_set_fowner(struct file *file) -{ - call_void_hook(file_set_fowner, file); -} - /** * security_file_send_sigiotask() - Check if sending SIGIO/SIGURG is allowed * @tsk: target task diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c index 7eed331e90f0..a8f5ed66808d 100644 --- a/security/selinux/hooks.c +++ b/security/selinux/hooks.c @@ -3644,8 +3644,6 @@ static int selinux_file_alloc_security(struct file *file) u32 sid = current_sid(); fsec->sid = sid; - fsec->fown_sid = sid; - return 0; } @@ -3918,33 +3916,20 @@ static int selinux_file_fcntl(struct file *file, unsigned int cmd, return err; } -static void selinux_file_set_fowner(struct file *file) -{ - struct file_security_struct *fsec; - - fsec = selinux_file(file); - fsec->fown_sid = current_sid(); -} - static int selinux_file_send_sigiotask(struct task_struct *tsk, struct fown_struct *fown, int signum) { - struct file *file; u32 sid = task_sid_obj(tsk); u32 perm; - struct file_security_struct *fsec; - - /* struct fown_struct is never outside the context of a struct file */ - file = container_of(fown, struct file, f_owner); - - fsec = selinux_file(file); + const struct task_security_struct *tsec = + selinux_cred(rcu_dereference(fown->cred)); if (!signum) perm = signal_to_av(SIGIO); /* as per send_sigio_to_task */ else perm = signal_to_av(signum); - return avc_has_perm(fsec->fown_sid, sid, + return avc_has_perm(tsec->sid, sid, SECCLASS_PROCESS, perm, NULL); } @@ -7202,7 +7187,6 @@ static struct security_hook_list selinux_hooks[] __ro_after_init = { LSM_HOOK_INIT(file_mprotect, selinux_file_mprotect), LSM_HOOK_INIT(file_lock, selinux_file_lock), LSM_HOOK_INIT(file_fcntl, selinux_file_fcntl), - LSM_HOOK_INIT(file_set_fowner, selinux_file_set_fowner), LSM_HOOK_INIT(file_send_sigiotask, selinux_file_send_sigiotask), LSM_HOOK_INIT(file_receive, selinux_file_receive), diff --git a/security/selinux/include/objsec.h b/security/selinux/include/objsec.h index dea1d6f3ed2d..d55b7f8d3a3d 100644 --- a/security/selinux/include/objsec.h +++ b/security/selinux/include/objsec.h @@ -56,7 +56,6 @@ struct inode_security_struct { struct file_security_struct { u32 sid; /* SID of open file description */ - u32 fown_sid; /* SID of file owner (for SIGIO) */ u32 isid; /* SID of inode at the time of file open */ u32 pseqno; /* Policy seqno at the time of file open */ }; diff --git a/security/smack/smack.h b/security/smack/smack.h index 041688e5a77a..06bac00cc796 100644 --- a/security/smack/smack.h +++ b/security/smack/smack.h @@ -328,12 +328,6 @@ static inline struct task_smack *smack_cred(const struct cred *cred) return cred->security + smack_blob_sizes.lbs_cred; } -static inline struct smack_known **smack_file(const struct file *file) -{ - return (struct smack_known **)(file->f_security + - smack_blob_sizes.lbs_file); -} - static inline struct inode_smack *smack_inode(const struct inode *inode) { return inode->i_security + smack_blob_sizes.lbs_inode; diff --git a/security/smack/smack_lsm.c b/security/smack/smack_lsm.c index f5cbec1e6a92..280a3da4c232 100644 --- a/security/smack/smack_lsm.c +++ b/security/smack/smack_lsm.c @@ -1650,26 +1650,6 @@ static void smack_inode_getsecid(struct inode *inode, u32 *secid) * label changing that SELinux does. */ -/** - * smack_file_alloc_security - assign a file security blob - * @file: the object - * - * The security blob for a file is a pointer to the master - * label list, so no allocation is done. - * - * f_security is the owner security information. It - * isn't used on file access checks, it's for send_sigio. - * - * Returns 0 - */ -static int smack_file_alloc_security(struct file *file) -{ - struct smack_known **blob = smack_file(file); - - *blob = smk_of_current(); - return 0; -} - /** * smack_file_ioctl - Smack check on ioctls * @file: the object @@ -1888,18 +1868,6 @@ static int smack_mmap_file(struct file *file, return rc; } -/** - * smack_file_set_fowner - set the file security blob value - * @file: object in question - * - */ -static void smack_file_set_fowner(struct file *file) -{ - struct smack_known **blob = smack_file(file); - - *blob = smk_of_current(); -} - /** * smack_file_send_sigiotask - Smack on sigio * @tsk: The target task @@ -1914,7 +1882,6 @@ static void smack_file_set_fowner(struct file *file) static int smack_file_send_sigiotask(struct task_struct *tsk, struct fown_struct *fown, int signum) { - struct smack_known **blob; struct smack_known *skp; struct smack_known *tkp = smk_of_task(smack_cred(tsk->cred)); const struct cred *tcred; @@ -1928,8 +1895,7 @@ static int smack_file_send_sigiotask(struct task_struct *tsk, file = container_of(fown, struct file, f_owner); /* we don't log here as rc can be overriden */ - blob = smack_file(file); - skp = *blob; + skp = smk_of_task(smack_cred(rcu_dereference(fown->cred))); rc = smk_access(skp, tkp, MAY_DELIVER, NULL); rc = smk_bu_note("sigiotask", skp, tkp, MAY_DELIVER, rc); @@ -5014,7 +4980,6 @@ static int smack_uring_cmd(struct io_uring_cmd *ioucmd) struct lsm_blob_sizes smack_blob_sizes __ro_after_init = { .lbs_cred = sizeof(struct task_smack), - .lbs_file = sizeof(struct smack_known *), .lbs_inode = sizeof(struct inode_smack), .lbs_ipc = sizeof(struct smack_known *), .lbs_msg_msg = sizeof(struct smack_known *), @@ -5065,14 +5030,12 @@ static struct security_hook_list smack_hooks[] __ro_after_init = { LSM_HOOK_INIT(inode_listsecurity, smack_inode_listsecurity), LSM_HOOK_INIT(inode_getsecid, smack_inode_getsecid), - LSM_HOOK_INIT(file_alloc_security, smack_file_alloc_security), LSM_HOOK_INIT(file_ioctl, smack_file_ioctl), LSM_HOOK_INIT(file_ioctl_compat, smack_file_ioctl), LSM_HOOK_INIT(file_lock, smack_file_lock), LSM_HOOK_INIT(file_fcntl, smack_file_fcntl), LSM_HOOK_INIT(mmap_file, smack_mmap_file), LSM_HOOK_INIT(mmap_addr, cap_mmap_addr), - LSM_HOOK_INIT(file_set_fowner, smack_file_set_fowner), LSM_HOOK_INIT(file_send_sigiotask, smack_file_send_sigiotask), LSM_HOOK_INIT(file_receive, smack_file_receive),
The fcntl's F_SETOWN command sets the process that handle SIGIO/SIGURG for the related file descriptor. Before this change, the file_set_fowner LSM hook was used to store this information. However, there are three issues with this approach: - Because security_file_set_fowner() only get one argument, all hook implementations ignore the VFS logic which may not actually change the process that handles SIGIO (e.g. TUN, TTY, dnotify). - Because security_file_set_fowner() is called before f_modown() without lock (e.g. f_owner.lock), concurrent F_SETOWN commands could result to a race condition and inconsistent LSM states (e.g. SELinux's fown_sid) compared to struct fown_struct's UID/EUID. - Because the current hook implementations does not use explicit atomic operations, they may create inconsistencies. It would help to completely remove this constraint, as well as the requirements of the RCU read-side critical section for the hook. Fix these issues by replacing f_owner.uid and f_owner.euid with a new f_owner.cred [1]. This also saves memory by removing dedicated LSM blobs, and simplifies code by removing the file_set_fowner LSM hook. This changes enables to remove the smack_file_alloc_security implementation, Smack's file blob, and SELinux's file_security_struct->fown_sid field. As for the UID/EUID, f_owner.cred is not always updated. Because the file_set_fowner hook is removed, the fowner credentials now have the same semantic as what is used by the VFS. Before this change, f_owner's UID/EUID were initialized to zero (i.e. GLOBAL_ROOT_UID), but to simplify code, f_owner's cred is now initialized with the file descriptor creator's credentials (i.e. file->f_cred), which is more consistent and simplifies LSMs logic. The sigio_perm()'s semantic does not need any change because SIGIO/SIGURG are only sent when a process is explicitly set with __f_setown(). Rename f_modown() to __f_setown() to simplify code. Cc: Al Viro <viro@zeniv.linux.org.uk> Cc: Casey Schaufler <casey@schaufler-ca.com> Cc: Christian Brauner <brauner@kernel.org> Cc: James Morris <jmorris@namei.org> Cc: Jann Horn <jannh@google.com> Cc: Ondrej Mosnacek <omosnace@redhat.com> Cc: Paul Moore <paul@paul-moore.com> Cc: Serge E. Hallyn <serge@hallyn.com> Cc: Stephen Smalley <stephen.smalley.work@gmail.com> Link: https://lore.kernel.org/r/20240809-explosionsartig-ablesen-b039dbc6ce82@brauner [1] Signed-off-by: Mickaël Salaün <mic@digikod.net> --- fs/fcntl.c | 44 +++++++++++++++---------------- fs/file_table.c | 3 +++ include/linux/fs.h | 2 +- include/linux/lsm_hook_defs.h | 1 - include/linux/security.h | 1 - security/security.c | 14 ---------- security/selinux/hooks.c | 22 +++------------- security/selinux/include/objsec.h | 1 - security/smack/smack.h | 6 ----- security/smack/smack_lsm.c | 39 +-------------------------- 10 files changed, 29 insertions(+), 104 deletions(-)