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: <CAHk-=wjFKgs-to95Op3p19Shy+EqW2ttSOwk2OadVN-e=eV73g@mail.gmail.com> Date: Tue, 12 Nov 2024 11:45:55 -0800 From: Linus Torvalds <torvalds@...ux-foundation.org> To: Josef Bacik <josef@...icpanda.com> Cc: kernel-team@...com, linux-fsdevel@...r.kernel.org, jack@...e.cz, amir73il@...il.com, brauner@...nel.org, linux-xfs@...r.kernel.org, linux-btrfs@...r.kernel.org, linux-mm@...ck.org, linux-ext4@...r.kernel.org Subject: Re: [PATCH v7 01/18] fsnotify: opt-in for permission events at file_open_perm() time On Tue, 12 Nov 2024 at 09:56, Josef Bacik <josef@...icpanda.com> wrote: > > @@ -119,14 +118,37 @@ static inline int fsnotify_file(struct file *file, __u32 mask) > * handle creation / destruction events and not "real" file events. > */ > if (file->f_mode & (FMODE_NONOTIFY | FMODE_PATH)) > + return false; > + > + /* Permission events require that watches are set before FS_OPEN_PERM */ > + if (mask & ALL_FSNOTIFY_PERM_EVENTS & ~FS_OPEN_PERM && > + !(file->f_mode & FMODE_NOTIFY_PERM)) > + return false; This still all looks very strange. As far as I can tell, there is exactly one user of FS_OPEN_PERM in 'mask', and that's fsnotify_open_perm(). Which is called in exactly one place: security_file_open(), which is the wrong place to call it anyway and is the only place where fsnotify is called from the security layer. In fact, that looks like an active bug: if you enable FSNOTIFY, but you *don't* enable CONFIG_SECURITY, the whole fsnotify_open_perm() will never be called at all. And I just verified that yes, you can very much generate such a config. So the whole FS_OPEN_PERM thing looks like a special case, called from a (broken) special place, and now polluting this "fsnotify_file()" logic for no actual reason and making it all look unnecessarily messy. I'd suggest that the whole fsnotify_open_perm() simply be moved to where it *should* be - in the open path - and not make a bad and broken attempt at hiding inside the security layer, and not use this "fsnotify_file()" logic at all. The open-time logic is different. It shouldn't even attempt - badly - to look like it's the same thing as some regular file access. Linus
Powered by blists - more mailing lists