linux-ext4 - Re: [PATCH 3/7] syscalls/fanotify20: Validate incoming FID in 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: <CAOQ4uxj_WwDPxZv0nr9+Hh+pim6+2onaBdFq_BR-qK=xz+8yUg@mail.gmail.com>
Date:   Wed, 4 Aug 2021 08:39:55 +0300
From:   Amir Goldstein <amir73il@...il.com>
To:     Gabriel Krisman Bertazi <krisman@...labora.com>
Cc:     LTP List <ltp@...ts.linux.it>, Jan Kara <jack@...e.com>,
        Ext4 <linux-ext4@...r.kernel.org>,
        Khazhismel Kumykov <khazhy@...gle.com>, kernel@...labora.com,
        Matthew Bobrowski <repnop@...gle.com>
Subject: Re: [PATCH 3/7] syscalls/fanotify20: Validate incoming FID in FAN_FS_ERROR

On Wed, Aug 4, 2021 at 7:54 AM Gabriel Krisman Bertazi
<krisman@...labora.com> wrote:
>
> Amir Goldstein <amir73il@...il.com> writes:
>
> > On Tue, Aug 3, 2021 at 12:47 AM Gabriel Krisman Bertazi
> > <krisman@...labora.com> wrote:
> >>
> >> Verify the FID provided in the event.  If the testcase has a null inode,
> >> this is assumed to be a superblock error (i.e. null FH).
> >>
> >> Signed-off-by: Gabriel Krisman Bertazi <krisman@...labora.com>
> >> ---
> >>  .../kernel/syscalls/fanotify/fanotify20.c     | 51 +++++++++++++++++++
> >>  1 file changed, 51 insertions(+)
> >>
> >> diff --git a/testcases/kernel/syscalls/fanotify/fanotify20.c b/testcases/kernel/syscalls/fanotify/fanotify20.c
> >> index fd5cfb8744f1..d8d788ae685f 100644
> >> --- a/testcases/kernel/syscalls/fanotify/fanotify20.c
> >> +++ b/testcases/kernel/syscalls/fanotify/fanotify20.c
> >> @@ -40,6 +40,14 @@
> >>
> >>  #define FAN_EVENT_INFO_TYPE_ERROR      4
> >>
> >> +#ifndef FILEID_INVALID
> >> +#define        FILEID_INVALID          0xff
> >> +#endif
> >> +
> >> +#ifndef FILEID_INO32_GEN
> >> +#define FILEID_INO32_GEN       1
> >> +#endif
> >> +
> >>  struct fanotify_event_info_error {
> >>         struct fanotify_event_info_header hdr;
> >>         __s32 error;
> >> @@ -57,6 +65,9 @@ static const struct test_case {
> >>         char *name;
> >>         int error;
> >>         unsigned int error_count;
> >> +
> >> +       /* inode can be null for superblock errors */
> >> +       unsigned int *inode;
> >
> > Any reason not to use fanotify_fid_t * like fanotify16.c?
>
> No reason other than I didn't notice they existed. Sorry. I will get
> this fixed.

No problem. That's what review is for ;-)

BTW, unless anyone is specifically interested I don't think there
is a reason to re post the test patches before the submission request.
Certainly not for the small fixes that I requested.

I do request that you post a link to a branch with the fixed test
so that we can experiment with the kernel patches.

I've also CC'ed Matthew who may want to help with review of the test
and man page that you posted in the cover letter [1].

Thanks,
Amir.

[1] https://lore.kernel.org/linux-ext4/20210802214645.2633028-1-krisman@collabora.com/T/#m9cf637c6aca94e28390f61deac5a53afbc9e88ae