Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

BUG: selinuxfs class directory not updated atomically on reload #51

Closed
stephensmalley opened this issue Jun 22, 2020 · 5 comments
Closed

Comments

@stephensmalley
Copy link
Member

As reported in https://lore.kernel.org/selinux/[email protected]/, selinuxfs does not atomically update its class subdirectory upon a policy reload, thereby creating a window during which userspace lookups of classes/permissions will fail. This can break userspace object managers like systemd or dbusd especially after more recent userspace changes to flush the class/perm cache upon a policy load notification from the kernel. If handle_unknown=deny, this can yield extraneous denials during the race window. Instead of deleting the old class subdirectory and then creating the new one in place, selinuxfs should create an unattached class directory tree from the new policy and then atomically exchange the old and new directories (ala RENAME_EXCHANGE). This is part of a broader set of issues around policy reload.

@stephensmalley
Copy link
Member Author

Aside from the manner in which we re-create the tree, wondering if we ought to optimize for the case where there are no changes to the existing classes/permissions (hence no need to delete or re-create any nodes under "class"). Ditto for "booleans".

@pcmoore
Copy link
Member

pcmoore commented Jun 24, 2020

Adding @dburgener as he originally brought the issue up on-list.

@pcmoore
Copy link
Member

pcmoore commented Jun 24, 2020

Related to #52.

@dburgener
Copy link
Member

Thanks for the mention. I've been working off and on on trying to get a patch set ready for submission for this, but unfortunately I haven't had as much time to put towards it as I'd hoped. I'm still making progress though, so if no one else gets there first, I hope to have a fix for this eventually.

@stephensmalley
Copy link
Member Author

Fixed via 0eea609

pcmoore pushed a commit that referenced this issue Sep 16, 2021
Commit 3df98d7 ("lsm,selinux: pass flowi_common instead of flowi
to the LSM hooks") introduced flowi{4,6}_to_flowi_common() functions which
cause UBSAN warning when building with LLVM 11.0.1 on Ubuntu 21.04.

 ================================================================================
 UBSAN: object-size-mismatch in ./include/net/flow.h:197:33
 member access within address ffffc9000109fbd8 with insufficient space
 for an object of type 'struct flowi'
 CPU: 2 PID: 7410 Comm: systemd-resolve Not tainted 5.14.0 #51
 Hardware name: VMware, Inc. VMware Virtual Platform/440BX Desktop Reference Platform, BIOS 6.00 02/27/2020
 Call Trace:
  dump_stack_lvl+0x103/0x171
  ubsan_type_mismatch_common+0x1de/0x390
  __ubsan_handle_type_mismatch_v1+0x41/0x50
  udp_sendmsg+0xda2/0x1300
  ? ip_skb_dst_mtu+0x1f0/0x1f0
  ? sock_rps_record_flow+0xe/0x200
  ? inet_send_prepare+0x2d/0x90
  sock_sendmsg+0x49/0x80
  ____sys_sendmsg+0x269/0x370
  __sys_sendmsg+0x15e/0x1d0
  ? syscall_enter_from_user_mode+0xf0/0x1b0
  do_syscall_64+0x3d/0xb0
  entry_SYSCALL_64_after_hwframe+0x44/0xae
 RIP: 0033:0x7f7081a50497
 Code: 0c 00 f7 d8 64 89 02 48 c7 c0 ff ff ff ff eb b7 0f 1f 00 f3 0f 1e fa 64 8b 04 25 18 00 00 00 85 c0 75 10 b8 2e 00 00 00 0f 05 <48> 3d 00 f0 ff ff 77 51 c3 48 83 ec 28 89 54 24 1c 48 89 74 24 10
 RSP: 002b:00007ffc153870f8 EFLAGS: 00000246 ORIG_RAX: 000000000000002e
 RAX: ffffffffffffffda RBX: 000000000000000c RCX: 00007f7081a50497
 RDX: 0000000000000000 RSI: 00007ffc15387140 RDI: 000000000000000c
 RBP: 00007ffc15387140 R08: 0000563f29a5e4fc R09: 000000000000cd28
 R10: 0000563f29a68a30 R11: 0000000000000246 R12: 000000000000000c
 R13: 0000000000000001 R14: 0000563f29a68a30 R15: 0000563f29a5e50c
 ================================================================================

I don't think we need to call flowi{4,6}_to_flowi() from these functions
because the first member of "struct flowi4" and "struct flowi6" is

  struct flowi_common __fl_common;

while the first member of "struct flowi" is

  union {
    struct flowi_common __fl_common;
    struct flowi4       ip4;
    struct flowi6       ip6;
    struct flowidn      dn;
  } u;

which should point to the same address without access to "struct flowi".

Signed-off-by: Tetsuo Handa <[email protected]>
Signed-off-by: David S. Miller <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants