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-=wgNFNYinkWCUvT2UnH2E2K_qPexEPgrm-xgr68YXnEQ_g@mail.gmail.com> Date: Tue, 12 Nov 2024 11:54:16 -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 07/18] fsnotify: generate pre-content permission event on open On Tue, 12 Nov 2024 at 09:56, Josef Bacik <josef@...icpanda.com> wrote: > > + /* > + * This permission hook is different than fsnotify_open_perm() hook. > + * This is a pre-content hook that is called without sb_writers held > + * and after the file was truncated. > + */ > + return fsnotify_file_area_perm(file, MAY_OPEN, &file->f_pos, 0); > } I still object to this all. You can't say "permission denied" after you've already truncated the file. It's not a sane model. I complained about that earlier, it seems that complaint was missed in the other complaints. Also, this whole "This permission hook is different than fsnotify_open_perm() hook" statement is purely because fsnotify_open_perm() itself was broken and called from the wrong place as mentioned in the other email. Fix *THAT* first, then unify the two places that should *not* be different into one single "this is the fsnotify_open" code. And that place explicitly sets that FMODE_NOTIFY_PERM bit, and makes sure that it does *not* set it for FMODE_NONOTIFY or FMODE_PATH cases. And then please work on making sure that that isn't called unless actually required. The actual real "pre-content permission events" should then ONLY test the FMODE_NOTIFY_PERM bit. Nothing else. None of this "re-use the existing fsnotify_file() logic" stuff. Noe extra tests, no extra logic. Don't make me jump through filve layers of inline functions that all test different 'mask' bits, just to verify that the open / read / write paths don't do something stupid. IOW, make it straightforward and obvious what you are doing, and make it very clear that you're not pointlessly testing things like FMODE_NONOTIFY when the *ONLY* thing that should be tested is whether FMODE_NOTIFY_PERM is set. Please. Linus
Powered by blists - more mailing lists