Received-SPF: pass (google.com: domain of linux-ext4-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
MIME-Version: 1.0
References: <20211026184239.151156-1-krisman@collabora.com> <20211026184239.151156-5-krisman@collabora.com>
In-Reply-To: <20211026184239.151156-5-krisman@collabora.com>
From:   Amir Goldstein <amir73il@gmail.com>
Date:   Wed, 27 Oct 2021 09:43:43 +0300
Message-ID: <CAOQ4uxgtkV7kF-YoWH4mau-p2U6bwLb1ajHXmVV1hwoDVpEDTQ@mail.gmail.com>
Subject: Re: [PATCH v2 04/10] syscalls/fanotify20: Validate the generic error info
To:     Gabriel Krisman Bertazi <krisman@collabora.com>
Cc:     LTP List <ltp@lists.linux.it>, Jan Kara <jack@suse.com>,
        Khazhismel Kumykov <khazhy@google.com>, kernel@collabora.com,
        Ext4 <linux-ext4@vger.kernel.org>,
        Matthew Bobrowski <repnop@google.com>
Content-Type: text/plain; charset="UTF-8"
Precedence: bulk

On Tue, Oct 26, 2021 at 9:43 PM Gabriel Krisman Bertazi
<krisman@collabora.com> wrote:
>
> Implement some validation for the generic error info record emitted by
> the kernel.  The error number is fs-specific but, well, we only support
> ext4 for now anyway.
>
> Signed-off-by: Gabriel Krisman Bertazi <krisman@collabora.com>
>

After fixing and testing configure.ac you may add:

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

> ---
> Changes since v1:
>   - Move defines to header file.
> ---
>  testcases/kernel/syscalls/fanotify/fanotify.h | 32 +++++++++++++++++
>  .../kernel/syscalls/fanotify/fanotify20.c     | 35 ++++++++++++++++++-
>  2 files changed, 66 insertions(+), 1 deletion(-)
>
> diff --git a/testcases/kernel/syscalls/fanotify/fanotify.h b/testcases/kernel/syscalls/fanotify/fanotify.h
> index 8828b53532a2..58e30aaf00bc 100644
> --- a/testcases/kernel/syscalls/fanotify/fanotify.h
> +++ b/testcases/kernel/syscalls/fanotify/fanotify.h
> @@ -167,6 +167,9 @@ typedef struct {
>  #ifndef FAN_EVENT_INFO_TYPE_DFID
>  #define FAN_EVENT_INFO_TYPE_DFID       3
>  #endif
> +#ifndef FAN_EVENT_INFO_TYPE_ERROR
> +#define FAN_EVENT_INFO_TYPE_ERROR      5
> +#endif
>
>  #ifndef HAVE_STRUCT_FANOTIFY_EVENT_INFO_HEADER
>  struct fanotify_event_info_header {
> @@ -184,6 +187,14 @@ struct fanotify_event_info_fid {
>  };
>  #endif /* HAVE_STRUCT_FANOTIFY_EVENT_INFO_FID */
>
> +#ifndef HAVE_STRUCT_FANOTIFY_EVENT_INFO_ERROR
> +struct fanotify_event_info_error {
> +       struct fanotify_event_info_header hdr;
> +       __s32 error;
> +       __u32 error_count;
> +};
> +#endif /* HAVE_STRUCT_FANOTIFY_EVENT_INFO_ERROR */

Need to add in configure.ac:

AC_CHECK_TYPES([struct fanotify_event_info_error, struct
fanotify_event_info_header],,,[#include <sys/fanotify.h>])

(not tested)

> +
>  /* NOTE: only for struct fanotify_event_info_fid */
>  #ifdef HAVE_STRUCT_FANOTIFY_EVENT_INFO_FID_FSID___VAL
>  # define FSID_VAL_MEMBER(fsid, i) (fsid.__val[i])
> @@ -403,4 +414,25 @@ static inline int fanotify_mark_supported_by_kernel(uint64_t flag)
>                 fanotify_events_supported_by_kernel(mask, init_flags, mark_type)); \
>  } while (0)
>
> +struct fanotify_event_info_header *get_event_info(
> +                                       struct fanotify_event_metadata *event,
> +                                       int info_type)
> +{
> +       struct fanotify_event_info_header *hdr = NULL;
> +       char *start = (char *) event;
> +       int off;
> +
> +       for (off = event->metadata_len; (off+sizeof(*hdr)) < event->event_len;
> +            off += hdr->len) {
> +               hdr = (struct fanotify_event_info_header *) &(start[off]);
> +               if (hdr->info_type == info_type)
> +                       return hdr;
> +       }
> +       return NULL;
> +}
> +
> +#define get_event_info_error(event)                                    \
> +       ((struct fanotify_event_info_error *)                           \
> +        get_event_info((event), FAN_EVENT_INFO_TYPE_ERROR))
> +
>  #endif /* __FANOTIFY_H__ */
> diff --git a/testcases/kernel/syscalls/fanotify/fanotify20.c b/testcases/kernel/syscalls/fanotify/fanotify20.c
> index 7a522aad4386..6074d449ae63 100644
> --- a/testcases/kernel/syscalls/fanotify/fanotify20.c
> +++ b/testcases/kernel/syscalls/fanotify/fanotify20.c
> @@ -42,10 +42,32 @@ int fd_notify;
>
>  static struct test_case {
>         char *name;
> +       int error;
> +       unsigned int error_count;
>         void (*trigger_error)(void);
>  } testcases[] = {
>  };
>
> +int check_error_event_info_error(struct fanotify_event_info_error *info_error,
> +                                const struct test_case *ex)
> +{
> +       int fail = 0;
> +
> +       if (info_error->error_count != ex->error_count) {
> +               tst_res(TFAIL, "%s: Unexpected error_count (%d!=%d)",
> +                       ex->name, info_error->error_count, ex->error_count);
> +               fail++;
> +       }
> +
> +       if (info_error->error != ex->error) {
> +               tst_res(TFAIL, "%s: Unexpected error code value (%d!=%d)",
> +                       ex->name, info_error->error, ex->error);
> +               fail++;
> +       }
> +
> +       return fail;
> +}
> +
>  int check_error_event_metadata(struct fanotify_event_metadata *event)
>  {
>         int fail = 0;
> @@ -68,6 +90,8 @@ 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;
> +       int fail = 0;
>
>         if (len < FAN_EVENT_METADATA_LEN) {
>                 tst_res(TFAIL, "No event metadata found");
> @@ -77,7 +101,16 @@ void check_event(char *buf, size_t len, const struct test_case *ex)
>         if (check_error_event_metadata(event))
>                 return;
>
> -       tst_res(TPASS, "Successfully received: %s", ex->name);
> +       info_error = get_event_info_error(event);
> +       if (info_error)
> +               fail += check_error_event_info_error(info_error, ex);
> +       else {
> +               tst_res(TFAIL, "Generic error record not found");
> +               fail++;
> +       }
> +
> +       if (!fail)
> +               tst_res(TPASS, "Successfully received: %s", ex->name);
>  }
>
>  static void do_test(unsigned int i)
> --
> 2.33.0
>