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
 
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <20241209122648.dpptugrol4p6ikmm@quack3>
Date: Mon, 9 Dec 2024 13:26:48 +0100
From: Jan Kara <jack@...e.cz>
To: Bert Karwatzki <spasswolf@....de>
Cc: Josef Bacik <josef@...icpanda.com>, linux-kernel@...r.kernel.org,
	Jan Kara <jack@...e.cz>, kernel-team@...com,
	linux-fsdevel@...r.kernel.org, amir73il@...il.com,
	brauner@...nel.org, torvalds@...ux-foundation.org,
	viro@...iv.linux.org.uk, linux-xfs@...r.kernel.org,
	linux-btrfs@...r.kernel.org, linux-mm@...ck.org,
	linux-ext4@...r.kernel.org
Subject: Re: commit 0790303ec869 leads to cpu stall without
 CONFIG_FANOTIFY_ACCESS_PERMISSIONS=y

On Mon 09-12-24 13:11:04, Jan Kara wrote:
> > Then I took a closer look at the function called in the problematic code
> > and noticed that fsnotify_file_area_perm(), is a NOOP when
> > CONFIG_FANOTIFY_ACCESS_PERMISSIONS is not set (which was the case in my
> > .config). This also explains why this was not found before, as
> > distributional .config file have this option enabled.  Setting the option
> > to y solves the issue, too
> 
> Well, I agree with you on all the points but the real question is, how come
> the test FMODE_FSNOTIFY_HSM(file->f_mode) was true on our kernel when you
> clearly don't run HSM software, even more so with
> CONFIG_FANOTIFY_ACCESS_PERMISSIONS disabled. That's the real cause of this
> problem. Something fishy is going on here... checking...
> 
> Ah, because I've botched out file_set_fsnotify_mode() in case
> CONFIG_FANOTIFY_ACCESS_PERMISSIONS is disabled. This should fix the
> problem:
> 
> index 1a9ef8f6784d..778a88fcfddc 100644
> --- a/include/linux/fsnotify.h
> +++ b/include/linux/fsnotify.h
> @@ -215,6 +215,7 @@ static inline int fsnotify_open_perm(struct file *file)
>  #else
>  static inline void file_set_fsnotify_mode(struct file *file)
>  {
> +       file->f_mode |= FMODE_NONOTIFY_PERM;
>  }
> 
> I'm going to test this with CONFIG_FANOTIFY_ACCESS_PERMISSIONS disabled and
> push out a fixed version. Thanks again for the report and analysis!

So this was not enough, What we need is:
index 1a9ef8f6784d..778a88fcfddc 100644
--- a/include/linux/fsnotify.h
+++ b/include/linux/fsnotify.h
@@ -215,6 +215,10 @@ static inline int fsnotify_open_perm(struct file *file)
 #else
 static inline void file_set_fsnotify_mode(struct file *file)
 {
+	/* Is it a file opened by fanotify? */
+	if (FMODE_FSNOTIFY_NONE(file->f_mode))
+		return;
+	file->f_mode |= FMODE_NONOTIFY_PERM;
 }

This passes testing for me so I've pushed it out and the next linux-next
build should have this fix.

								Honza
-- 
Jan Kara <jack@...e.com>
SUSE Labs, CR

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ