Re: [PATCH v5 2/3] fanotify: define struct members to hold response decision context
On 2023-01-03 13:42, Jan Kara wrote:
> On Thu 22-12-22 15:47:21, Richard Guy Briggs wrote:
> > On 2022-12-16 17:43, Jan Kara wrote:
> > > On Mon 12-12-22 09:06:10, Richard Guy Briggs wrote:
> > > > This patch adds a flag, FAN_INFO and an extensible buffer to provide
> > > > additional information about response decisions. The buffer contains
> > > > one or more headers defining the information type and the length of the
> > > > following information. The patch defines one additional information
> > > > type, FAN_RESPONSE_INFO_AUDIT_RULE, to audit a rule number. This will
> > > > allow for the creation of other information types in the future if other
> > > > users of the API identify different needs.
> > > >
> > > > Suggested-by: Steve Grubb
> > > > Link: https://lore.kernel.org/r/2745105.e9J7NaK4W3@x2
> > > > Suggested-by: Jan Kara
> > > > Link: https://lore.kernel.org/r/20201001101219.ge17...@quack2.suse.cz
> > > > Signed-off-by: Richard Guy Briggs
> > > > +{
> > > > + if (fd == FAN_NOFD)
> > > > + return -ENOENT;
> > >
> > > I would not test 'fd' in this function at all. After all it is not part of
> > > the response info structure and you do check it in
> > > process_access_response() anyway.
> >
> > I wrestled with that. I was even tempted to swallow the following fd
> > check too, but the flow would not have made as much sense for the
> > non-INFO case.
> >
> > My understanding from Amir was that FAN_NOFD was only to be sent in in
> > conjuction with FAN_INFO to test if a newer kernel was present.
>
> Yes, that is correct. But we not only want to check that FAN_INFO flag is
> understood (as you do in your patch) but also whether a particular response
> type is understood (which you don't verify for FAN_NOFD). Currently, there
> is only one response type (FAN_RESPONSE_INFO_AUDIT_RULE) but if there are
> more in the future we need old kernels to refuse new response types even
> for FAN_NOFD case.
Ok, I agree the NOFD check should be after.
> > I presumed that if FAN_NOFD was present without FAN_INFO that was an
> > invalid input to an old kernel.
>
> Yes, that is correct and I agree the conditions I've suggested below are
> wrong in that regard and need a bit of tweaking. Thanks for catching it.
>
> > > > +
> > > > + if (info_len != sizeof(*friar))
> > > > + return -EINVAL;
> > > > +
> > > > + if (copy_from_user(friar, info, sizeof(*friar)))
> > > > + return -EFAULT;
> > > > +
> > > > + if (friar->hdr.type != FAN_RESPONSE_INFO_AUDIT_RULE)
> > > > + return -EINVAL;
> > > > + if (friar->hdr.pad != 0)
> > > > + return -EINVAL;
> > > > + if (friar->hdr.len != sizeof(*friar))
> > > > + return -EINVAL;
> > > > +
> > > > + return info_len;
> > > > +}
> > > > +
> > >
> > > ...
> > >
> > > > @@ -327,10 +359,18 @@ static int process_access_response(struct
> > > > fsnotify_group *group,
> > > > return -EINVAL;
> > > > }
> > > >
> > > > - if (fd < 0)
> > > > + if ((response & FAN_AUDIT) && !FAN_GROUP_FLAG(group,
> > > > FAN_ENABLE_AUDIT))
> > > > return -EINVAL;
> > > >
> > > > - if ((response & FAN_AUDIT) && !FAN_GROUP_FLAG(group,
> > > > FAN_ENABLE_AUDIT))
> > > > + if (response & FAN_INFO) {
> > > > + ret = process_access_response_info(fd, info, info_len,
> > > > &friar);
> > > > + if (ret < 0)
> > > > + return ret;
> > > > + } else {
> > > > + ret = 0;
> > > > + }
> > > > +
> > > > + if (fd < 0)
> > > > return -EINVAL;
> > >
> > > And here I'd do:
> > >
> > > if (fd == FAN_NOFD)
> > > return 0;
> > > if (fd < 0)
> > > return -EINVAL;
> > >
> > > As we talked in previous revisions we'd specialcase FAN_NOFD to just
> > > verify
> > > extra info is understood by the kernel so that application writing
> > > fanotify
> > > responses has a way to check which information it can provide to the
> > > kernel.
> >
> > The reason for including it in process_access_response_info() is to make
> > sure that it is included in the FAN_INFO case to detect this extension.
> > If it were included here
>
> I see what you're getting at now. So the condition
>
> if (fd == FAN_NOFD)
> return 0;
>
> needs to be moved into
>
> if (response & FAN_INFO)
>
> branch after process_access_response_info(). I still prefer to keep it
> outside of the process_access_response_info() function itself as it looks
> more logical to me. Does it address your concerns?
Ok. Note that this does not return zero to userspace, since this
function's return value is added to the size of the struct
fanotify_response when there is no error.
For that reason, I think it makes more sense to return -ENOENT, or some
other unused error code that fits, unless you think it is acceptable to
return
Re: Difference between BITMAP_EXECUTABLE_PATH and BITMAP_EXCLUDE_EXTEND flags
On Monday, January 16, 2023 11:15:46 AM EST Avtansh Gupta wrote: > Hello All, > > Please could you help me understand the difference between the following > flags which are being used? > > AUDIT_FEATURE_BITMAP_EXECUTABLE_PATH This ^^^ means the kernel supports -F exe= in the rules. https://listman.redhat.com/archives/linux-audit/2015-August/010585.html > AUDIT_FEATURE_BITMAP_EXCLUDE_EXTEND This ^^^ means that the exclude filter supports many more kinds of fields than the original design allowed for. https://listman.redhat.com/archives/linux-audit/2016-June/011433.html For upstream kernels and ones derived after it was release, the second implies the first one is already included. -Steve -- Linux-audit mailing list Linux-audit@redhat.com https://listman.redhat.com/mailman/listinfo/linux-audit
Difference between BITMAP_EXECUTABLE_PATH and BITMAP_EXCLUDE_EXTEND flags
Hello All, Please could you help me understand the difference between the following flags which are being used? AUDIT_FEATURE_BITMAP_EXECUTABLE_PATH AUDIT_FEATURE_BITMAP_EXCLUDE_EXTEND -- *Regards,* *Avtansh Gupta* *+91 8743068185* -- Linux-audit mailing list Linux-audit@redhat.com https://listman.redhat.com/mailman/listinfo/linux-audit
Re: [RFC PATCH 00/25] Upstream kvx Linux port
On Mon, Jan 9, 2023 at 11:53 PM Jeff Xie wrote:
>
> On Mon, Jan 9, 2023 at 11:30 PM Yann Sionneau wrote:
> >
> > Hi Jeff,
> >
> > On 1/9/23 16:11, Jeff Xie wrote:
> > > On Mon, Jan 9, 2023 at 9:21 PM Yann Sionneau wrote:
> > >> Hi Jeff,
> > >>
> > >> On 1/7/23 07:25, Jeff Xie wrote:
> > >>> Hi,
> > >>>
> > >>> On Wed, Jan 4, 2023 at 1:01 AM Yann Sionneau
> > >>> wrote:
> > [snip]
> >
> > A kvx toolchain can be built using:
> > # install dependencies: texinfo bison flex libgmp-dev libmpc-dev
> > libmpfr-dev
> > $ git clone https://github.com/kalray/build-scripts
> > $ cd build-scripts
> > $ source last.refs
> > $ ./build-kvx-xgcc.sh output
> > >>> I would like to build the kvx-xgcc to compile and test the linux
> > >>> kernel, but it reported a compile error.
> > >>> I wonder what version of gcc you are using.
> > >>>
> > >>> My build environment:
> > >>> VERSION="20.04.2 LTS (Focal Fossa)"
> > >>> gcc version 9.3.0 (Ubuntu 9.3.0-17ubuntu1~20.04)
> > >>>
> > >>>
> > >>> Compile error:
> > >>> $ ./build-kvx-xgcc.sh output
> > >>>
> > >>> ../../binutils/libiberty/fibheap.c: In function
> > >>> ‘fibheap_replace_key_data’:
> > >>> ../../binutils/libiberty/fibheap.c:38:24: error: ‘LONG_MIN’ undeclared
> > >>> (first use in this function)
> > >>> 38 | #define FIBHEAPKEY_MIN LONG_MIN
> > >>> |^~~~
> > >>> [snip]
> > >> What SHA1 of https://github.com/kalray/build-scripts are you using?
> > > I have executed the "source last.refs"
> >
> > I was referring to the SHA1 of the repo itself (build-scripts).
> >
> > `last.refs` is a symbolic link which can point to several releases,
> > depending on "when" you did the clone.
> >
> > I am asking this because we recently published new toolchains.
> >
> > I want to make sure which one you are trying to build.
>
> Unfortunately I deleted this repo a few minutes before you asked me ;-(
> But I remember that I cloned this repo two days ago.
> it should be: last.refs -> refs/4.11.0.refs
It should be my own environmental problem.
I reinstalled the system once and it has been able to compile normally ;-)
In the past few days, I have reviewed almost all the codes,
which is very meaningful for me to learn, thank you team.
>
> > >> We are building our toolchain on Ubuntu 18.04 / 20.04 and 22.04 without
> > >> issues, I don't understand why it does not work for you, although indeed
> > >> the error log you are having pops out on my search engine and seems to
> > >> be some well known issue.
> > > Yes, there are many answers on the web, but none of them solve this
> > > problem.
> > >
> > >> If the build-script does not work for you, you can still use the
> > >> pre-built toolchains generated by the GitHub automated actions:
> > >> https://github.com/kalray/build-scripts/releases/tag/v4.11.1 ("latest"
> > >> means 22.04)
> > > Thanks, this is the final solution ;-)
> > Good to see it helped :)
> >
> > Regards,
> >
> > --
> >
> > Yann
> >
> >
> >
> >
> >
>
>
> --
> Thanks,
> JeffXie
--
Thanks,
JeffXie
--
Linux-audit mailing list
Linux-audit@redhat.com
https://listman.redhat.com/mailman/listinfo/linux-audit
