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]
Date:   Wed, 27 Oct 2021 09:48:41 +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>,
        Khazhismel Kumykov <khazhy@...gle.com>, kernel@...labora.com,
        Ext4 <linux-ext4@...r.kernel.org>,
        Matthew Bobrowski <repnop@...gle.com>
Subject: Re: [PATCH v2 05/10] syscalls/fanotify20: Validate incoming FID in FAN_FS_ERROR

On Tue, Oct 26, 2021 at 9:43 PM Gabriel Krisman Bertazi
<krisman@...labora.com> wrote:
>
> Verify the FID provided in the event.  If the FH has size 0, this is
> assumed to be a superblock error (i.e. null FH).
>
> Signed-off-by: Gabriel Krisman Bertazi <krisman@...labora.com>

Reviewed-by: Amir Goldstein <amir73il@...il.com>

Except maybe move define of FILEID_INVALID to header.

>
> ---
> Changes since v1:
>   - Move defines to header file.
>   - Use 0-len FH for sb error
> ---
>  testcases/kernel/syscalls/fanotify/fanotify.h |  4 ++
>  .../kernel/syscalls/fanotify/fanotify20.c     | 63 +++++++++++++++++++
>  2 files changed, 67 insertions(+)
>
> diff --git a/testcases/kernel/syscalls/fanotify/fanotify.h b/testcases/kernel/syscalls/fanotify/fanotify.h
> index 58e30aaf00bc..9bff3cf1a3fe 100644
> --- a/testcases/kernel/syscalls/fanotify/fanotify.h
> +++ b/testcases/kernel/syscalls/fanotify/fanotify.h
> @@ -435,4 +435,8 @@ struct fanotify_event_info_header *get_event_info(
>         ((struct fanotify_event_info_error *)                           \
>          get_event_info((event), FAN_EVENT_INFO_TYPE_ERROR))
>
> +#define get_event_info_fid(event)                                      \
> +       ((struct fanotify_event_info_fid *)                             \
> +        get_event_info((event), FAN_EVENT_INFO_TYPE_FID))
> +
>  #endif /* __FANOTIFY_H__ */
> diff --git a/testcases/kernel/syscalls/fanotify/fanotify20.c b/testcases/kernel/syscalls/fanotify/fanotify20.c
> index 6074d449ae63..220cd51419e8 100644
> --- a/testcases/kernel/syscalls/fanotify/fanotify20.c
> +++ b/testcases/kernel/syscalls/fanotify/fanotify20.c
> @@ -34,20 +34,61 @@
>  #ifdef HAVE_SYS_FANOTIFY_H
>  #include "fanotify.h"
>
> +#ifndef FILEID_INVALID
> +#define        FILEID_INVALID          0xff
> +#endif
> +
>  #define BUF_SIZE 256
>  static char event_buf[BUF_SIZE];
>  int fd_notify;
>
>  #define MOUNT_PATH "test_mnt"
>
> +/* These expected FIDs are common to multiple tests */
> +static struct fanotify_fid_t null_fid;
> +
>  static struct test_case {
>         char *name;
>         int error;
>         unsigned int error_count;
> +       struct fanotify_fid_t *fid;
>         void (*trigger_error)(void);
>  } testcases[] = {
>  };
>
> +int check_error_event_info_fid(struct fanotify_event_info_fid *fid,
> +                                const struct test_case *ex)
> +{
> +       struct file_handle *fh = (struct file_handle *) &fid->handle;
> +
> +       if (memcmp(&fid->fsid, &ex->fid->fsid, sizeof(fid->fsid))) {
> +               tst_res(TFAIL, "%s: Received bad FSID type (%x...!=%x...)",
> +                       ex->name, FSID_VAL_MEMBER(fid->fsid, 0),
> +                       FSID_VAL_MEMBER(ex->fid->fsid, 0));
> +
> +               return 1;
> +       }
> +       if (fh->handle_type != ex->fid->handle.handle_type) {
> +               tst_res(TFAIL, "%s: Received bad file_handle type (%d!=%d)",
> +                       ex->name, fh->handle_type, ex->fid->handle.handle_type);
> +               return 1;
> +       }
> +
> +       if (fh->handle_bytes != ex->fid->handle.handle_bytes) {
> +               tst_res(TFAIL, "%s: Received bad file_handle len (%d!=%d)",
> +                       ex->name, fh->handle_bytes, ex->fid->handle.handle_bytes);
> +               return 1;
> +       }
> +
> +       if (memcmp(fh->f_handle, ex->fid->handle.f_handle, fh->handle_bytes)) {
> +               tst_res(TFAIL, "%s: Received wrong handle. "
> +                       "Expected (%x...) got (%x...) ", ex->name,
> +                       *(int*)ex->fid->handle.f_handle, *(int*)fh->f_handle);
> +               return 1;
> +       }
> +       return 0;
> +}
> +
>  int check_error_event_info_error(struct fanotify_event_info_error *info_error,
>                                  const struct test_case *ex)
>  {
> @@ -91,6 +132,7 @@ void check_event(char *buf, size_t len, const struct test_case *ex)
>         struct fanotify_event_metadata *event =
>                 (struct fanotify_event_metadata *) buf;
>         struct fanotify_event_info_error *info_error;
> +       struct fanotify_event_info_fid *info_fid;
>         int fail = 0;
>
>         if (len < FAN_EVENT_METADATA_LEN) {
> @@ -109,6 +151,14 @@ void check_event(char *buf, size_t len, const struct test_case *ex)
>                 fail++;
>         }
>
> +       info_fid = get_event_info_fid(event);
> +       if (info_fid)
> +               fail += check_error_event_info_fid(info_fid, ex);
> +       else {
> +               tst_res(TFAIL, "FID record not found");
> +               fail++;
> +       }
> +
>         if (!fail)
>                 tst_res(TPASS, "Successfully received: %s", ex->name);
>  }
> @@ -125,12 +175,25 @@ static void do_test(unsigned int i)
>         check_event(event_buf, read_len, tcase);
>  }
>
> +static void init_null_fid(void)
> +{
> +       /* Use fanotify_save_fid to fill the fsid and overwrite the
> +        * file_handler to create a null_fid
> +        */
> +       fanotify_save_fid(MOUNT_PATH, &null_fid);
> +
> +       null_fid.handle.handle_type = FILEID_INVALID;
> +       null_fid.handle.handle_bytes = 0;
> +}
> +
>  static void setup(void)
>  {
>         REQUIRE_FANOTIFY_EVENTS_SUPPORTED_ON_FS(FAN_CLASS_NOTIF|FAN_REPORT_FID,
>                                                 FAN_MARK_FILESYSTEM,
>                                                 FAN_FS_ERROR, ".");
>
> +       init_null_fid();
> +
>         fd_notify = SAFE_FANOTIFY_INIT(FAN_CLASS_NOTIF|FAN_REPORT_FID,
>                                        O_RDONLY);
>
> --
> 2.33.0
>

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ