| Message ID | 20240826124709.23530-1-stephen.smalley.work@gmail.com (mailing list archive) |
|---|---|
| State | Accepted |
| Delegated to: | Paul Moore |
| Headers | show |
| Series | selinux: annotate false positive data race to avoid KCSAN warnings | expand |
On Mon, Aug 26, 2024 at 8:47 AM Stephen Smalley <stephen.smalley.work@gmail.com> wrote: > > KCSAN flags the check of isec->initialized by > __inode_security_revalidate() as a data race. This is indeed a racy > check, but inode_doinit_with_dentry() will recheck with isec->lock held. > Annotate the check with the data_race() macro to silence the KCSAN false > positive. > > Reported-by: syzbot+319ed1769c0078257262@syzkaller.appspotmail.com > Signed-off-by: Stephen Smalley <stephen.smalley.work@gmail.com> With this patch applied and the following KCSAN-related configuration, I don't see any data-race warnings in SELinux functions during the selinux-testsuite (lots of them elsewhere though). CONFIG_KCSAN=y # CONFIG_KCSAN_VERBOSE is not set CONFIG_KCSAN_SELFTEST=y CONFIG_KCSAN_EARLY_ENABLE=y CONFIG_KCSAN_NUM_WATCHPOINTS=64 CONFIG_KCSAN_UDELAY_TASK=80 CONFIG_KCSAN_UDELAY_INTERRUPT=20 CONFIG_KCSAN_DELAY_RANDOMIZE=y CONFIG_KCSAN_SKIP_WATCH=4000 CONFIG_KCSAN_SKIP_WATCH_RANDOMIZE=y # CONFIG_KCSAN_INTERRUPT_WATCHER is not set CONFIG_KCSAN_REPORT_ONCE_IN_MS=3000 CONFIG_KCSAN_REPORT_RACE_UNKNOWN_ORIGIN=y # CONFIG_KCSAN_STRICT is not set CONFIG_KCSAN_REPORT_VALUE_CHANGE_ONLY=y CONFIG_KCSAN_ASSUME_PLAIN_WRITES_ATOMIC=y # CONFIG_KCSAN_IGNORE_ATOMICS is not set # CONFIG_KCSAN_PERMISSIVE is not set
On Aug 26, 2024 Stephen Smalley <stephen.smalley.work@gmail.com> wrote: > > KCSAN flags the check of isec->initialized by > __inode_security_revalidate() as a data race. This is indeed a racy > check, but inode_doinit_with_dentry() will recheck with isec->lock held. > Annotate the check with the data_race() macro to silence the KCSAN false > positive. > > Reported-by: syzbot+319ed1769c0078257262@syzkaller.appspotmail.com > Signed-off-by: Stephen Smalley <stephen.smalley.work@gmail.com> > --- > security/selinux/hooks.c | 7 ++++++- > 1 file changed, 6 insertions(+), 1 deletion(-) Merged into selinux/dev, thanks Stephen. -- paul-moore.com
diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c index 55c78c318ccd..70c335846336 100644 --- a/security/selinux/hooks.c +++ b/security/selinux/hooks.c @@ -282,8 +282,13 @@ static int __inode_security_revalidate(struct inode *inode, might_sleep_if(may_sleep); + /* + * The check of isec->initialized below is racy but + * inode_doinit_with_dentry() will recheck with + * isec->lock held. + */ if (selinux_initialized() && - isec->initialized != LABEL_INITIALIZED) { + data_race(isec->initialized != LABEL_INITIALIZED)) { if (!may_sleep) return -ECHILD;
KCSAN flags the check of isec->initialized by __inode_security_revalidate() as a data race. This is indeed a racy check, but inode_doinit_with_dentry() will recheck with isec->lock held. Annotate the check with the data_race() macro to silence the KCSAN false positive. Reported-by: syzbot+319ed1769c0078257262@syzkaller.appspotmail.com Signed-off-by: Stephen Smalley <stephen.smalley.work@gmail.com> --- security/selinux/hooks.c | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-)