linux-ext4 - Re: [PATCH v5.15.y] Revert "fanotify: Allow users to request FAN_FS

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: <87jzhbsncw.fsf@mailhost.krisman.be>
Date: Tue, 23 Jul 2024 19:24:47 -0400
From: Gabriel Krisman Bertazi <gabriel@...sman.be>
To: cel@...nel.org
Cc: amir73il@...il.com,  gregkh@...uxfoundation.org,  jack@...e.cz,
  sashal@...nel.org,  stable@...r.kernel.org,  adilger.kernel@...ger.ca,
  linux-ext4@...r.kernel.org,  tytso@....edu,
  alexey.makhalov@...adcom.com,  vasavi.sirnapalli@...adcom.com,
  florian.fainelli@...adcom.com,  Chuck Lever <chuck.lever@...cle.com>,
  Gabriel Krisman Bertazi <gabriel@...sman.be>
Subject: Re: [PATCH v5.15.y] Revert "fanotify: Allow users to request
 FAN_FS_ERROR events"

cel@...nel.org writes:

> From: Chuck Lever <chuck.lever@...cle.com>
>
> Gabriel says:
>> 9709bd548f11 just enabled a new feature -
>> which seems against stable rules.  Considering that "anything is
>> a CVE", we really need to be cautious about this kind of stuff in
>> stable kernels.
>>
>> Is it possible to drop 9709bd548f11 from stable instead?
>
> The revert wasn't clean, but adjusting it to fit was straightforward.
> This passes NFSD CI, and adds no new failures to the fanotify ltp
> tests.
>
> Reported-by: Gabriel Krisman Bertazi <gabriel@...sman.be>
> Signed-off-by: Chuck Lever <chuck.lever@...cle.com>
> ---
>  fs/notify/fanotify/fanotify_user.c | 4 ----
>  include/linux/fanotify.h           | 6 +-----
>  2 files changed, 1 insertion(+), 9 deletions(-)
>
> Gabriel, is this what you were thinking?

Thanks Chuck.

This looks good to me as a way to disable it in stable and prevent
userspace from trying to use it. Up to fanotify maintainers to decide
whether to brign the rest of the series or merge this, but either way:

Reviewed-by: Gabriel Krisman Bertazi <gabriel@...sman.be>


>
> diff --git a/fs/notify/fanotify/fanotify_user.c b/fs/notify/fanotify/fanotify_user.c
> index d93418f21386..0d91db1c7249 100644
> --- a/fs/notify/fanotify/fanotify_user.c
> +++ b/fs/notify/fanotify/fanotify_user.c
> @@ -1701,10 +1701,6 @@ static int do_fanotify_mark(int fanotify_fd, unsigned int flags, __u64 mask,
>  	    group->priority == FS_PRIO_0)
>  		goto fput_and_out;
>  
> -	if (mask & FAN_FS_ERROR &&
> -	    mark_type != FAN_MARK_FILESYSTEM)
> -		goto fput_and_out;
> -
>  	/*
>  	 * Evictable is only relevant for inode marks, because only inode object
>  	 * can be evicted on memory pressure.
> diff --git a/include/linux/fanotify.h b/include/linux/fanotify.h
> index 558844c8d259..df60b46971c9 100644
> --- a/include/linux/fanotify.h
> +++ b/include/linux/fanotify.h
> @@ -97,13 +97,9 @@ extern struct ctl_table fanotify_table[]; /* for sysctl */
>  #define FANOTIFY_INODE_EVENTS	(FANOTIFY_DIRENT_EVENTS | \
>  				 FAN_ATTRIB | FAN_MOVE_SELF | FAN_DELETE_SELF)
>  
> -/* Events that can only be reported with data type FSNOTIFY_EVENT_ERROR */
> -#define FANOTIFY_ERROR_EVENTS	(FAN_FS_ERROR)
> -
>  /* Events that user can request to be notified on */
>  #define FANOTIFY_EVENTS		(FANOTIFY_PATH_EVENTS | \
> -				 FANOTIFY_INODE_EVENTS | \
> -				 FANOTIFY_ERROR_EVENTS)
> +				 FANOTIFY_INODE_EVENTS)
>  
>  /* Events that require a permission response from user */
>  #define FANOTIFY_PERM_EVENTS	(FAN_OPEN_PERM | FAN_ACCESS_PERM | \

-- 
Gabriel Krisman Bertazi