Re: [PATCH v1 1/2] audit: record fanotify event regardless of presence of rules

[Jan Kara]

On Fri 07-03-25 14:19:38, Richard Guy Briggs wrote:
> On 2025-03-07 15:52, Jan Kara wrote:
> > On Thu 06-03-25 20:12:23, Richard Guy Briggs wrote:
> > > On 2025-03-06 16:06, Jan Kara wrote:
> > > > On Wed 05-03-25 16:33:19, Richard Guy Briggs wrote:
> > > > > When no audit rules are in place, fanotify event results are
> > > > > unconditionally dropped due to an explicit check for the existence of
> > > > > any audit rules.  Given this is a report from another security
> > > > > sub-system, allow it to be recorded regardless of the existence of any
> > > > > audit rules.
> > > > > 
> > > > > To test, install and run the fapolicyd daemon with default config.  Then
> > > > > as an unprivileged user, create and run a very simple binary that should
> > > > > be denied.  Then check for an event with
> > > > > 	ausearch -m FANOTIFY -ts recent
> > > > > 
> > > > > Link: https://issues.redhat.com/browse/RHEL-1367
> > > > > Signed-off-by: Richard Guy Briggs <rgb@xxxxxxxxxx>
> > > > 
> > > > I don't know enough about security modules to tell whether this is what
> > > > admins want or not so that's up to you but:
> > > > 
> > > > > -static inline void audit_fanotify(u32 response, struct fanotify_response_info_audit_rule *friar)
> > > > > -{
> > > > > -	if (!audit_dummy_context())
> > > > > -		__audit_fanotify(response, friar);
> > > > > -}
> > > > > -
> > > > 
> > > > I think this is going to break compilation with !CONFIG_AUDITSYSCALL &&
> > > > CONFIG_FANOTIFY?
> > > 
> > > Why would that break it?  The part of the patch you (prematurely)
> > > deleted takes care of that.
> > 
> > So I'm failing to see how it takes care of that when with
> > !CONFIG_AUDITSYSCALL kernel/auditsc.c does not get compiled into the kernel.
> > So what does provide the implementation of audit_fanotify() in that case?
> > I think you need to provide empty audit_fanotify() inline wrapper for that
> > case...
> 
> I'm sorry, I responded too quickly without thinking about your question,
> my mistake.  It isn't the prototype that was changed in the
> CONFIG_SYSCALL case that is relevant in that case.
> 
> There was already in existance in the !CONFIG_AUDITSYSCALL case the
> inline wrapper to do that job:
> 
> 	static inline void audit_fanotify(u32 response, struct fanotify_response_info_audit_rule *friar)
> 	{ }
> 
> Did I understand correctly this time and does this answer your question?

Yes, thanks for explanation and sorry for not noticing the second
audit_fanotify() implementation. Somehow I've hasted to a conclusion (based
on customs of parts of kernel I maintain ;)) that you rely on
audit_dummy_context() being constant 0 for !CONFIG_AUDITSYSCALL and thus
__audit_fanotify() call getting compiled out (which would not be the case
after your changes).

Anyway, for the patch feel free to add:

Acked-by: Jan Kara <jack@xxxxxxx>

> But you do cause me to notice the case that these notifications will be
> dropped when CONFIG_AUDIT && !CONFIG_AUDITSYSCALL.

Glad my blindness helped something ;)

								Honza
-- 
Jan Kara <jack@xxxxxxxx>
SUSE Labs, CR