linux-ext4 - Re: [PATCH v6 20/21] samples: Add fs error monitoring example

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: <20210818130251.GD28119@quack2.suse.cz>
Date:   Wed, 18 Aug 2021 15:02:51 +0200
From:   Jan Kara <jack@...e.cz>
To:     Gabriel Krisman Bertazi <krisman@...labora.com>
Cc:     amir73il@...il.com, jack@...e.com, linux-api@...r.kernel.org,
        linux-ext4@...r.kernel.org, linux-fsdevel@...r.kernel.org,
        khazhy@...gle.com, dhowells@...hat.com, david@...morbit.com,
        tytso@....edu, djwong@...nel.org, repnop@...gle.com,
        kernel@...labora.com
Subject: Re: [PATCH v6 20/21] samples: Add fs error monitoring example

On Thu 12-08-21 17:40:09, Gabriel Krisman Bertazi wrote:
> Introduce an example of a FAN_FS_ERROR fanotify user to track filesystem
> errors.
> 
> Reviewed-by: Amir Goldstein <amir73il@...il.com>
> Signed-off-by: Gabriel Krisman Bertazi <krisman@...labora.com>

<snip>

> diff --git a/samples/fanotify/fs-monitor.c b/samples/fanotify/fs-monitor.c
> new file mode 100644
> index 000000000000..e115053382be
> --- /dev/null
> +++ b/samples/fanotify/fs-monitor.c
> @@ -0,0 +1,138 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Copyright 2021, Collabora Ltd.
> + */
> +
> +#define _GNU_SOURCE
> +#include <errno.h>
> +#include <err.h>
> +#include <stdlib.h>
> +#include <stdio.h>
> +#include <fcntl.h>
> +#include <sys/fanotify.h>
> +#include <sys/types.h>
> +#include <unistd.h>
> +#include <sys/types.h>
> +
> +#ifndef FAN_FS_ERROR
> +#define FAN_FS_ERROR		0x00008000
> +#define FAN_EVENT_INFO_TYPE_ERROR	4
> +
> +struct fanotify_event_info_error {
> +	struct fanotify_event_info_header hdr;
> +	__s32 error;
> +	__u32 error_count;
> +};
> +#endif

Shouldn't we get these from uapi headers? But I guess the problem is that
you want this sample to work before glibc picks up the new headers? Is this
meant as a sample code for userspace to copy from or more as a testcase?

> +#ifndef FILEID_INO32_GEN
> +#define FILEID_INO32_GEN	1
> +#endif
> +
> +#ifndef FILEID_INVALID
> +#define	FILEID_INVALID		0xff
> +#endif
> +
> +static void print_fh(struct file_handle *fh)
> +{
> +	int i;
> +	uint32_t *h = (uint32_t *) fh->f_handle;
> +
> +	printf("\tfh: ");
> +	for (i = 0; i < fh->handle_bytes; i++)
> +		printf("%hhx", fh->f_handle[i]);
> +	printf("\n");
> +
> +	printf("\tdecoded fh: ");
> +	if (fh->handle_type == FILEID_INO32_GEN)
> +		printf("inode=%u gen=%u\n", h[0], h[1]);
> +	else if (fh->handle_type == FILEID_INVALID && !fh->handle_bytes)
> +		printf("Type %d (Superblock error)\n", fh->handle_type);
> +	else
> +		printf("Type %d (Unknown)\n", fh->handle_type);
> +
> +}
> +
> +static void handle_notifications(char *buffer, int len)
> +{
> +	struct fanotify_event_metadata *metadata;
> +	struct fanotify_event_info_error *error;
> +	struct fanotify_event_info_fid *fid;
> +	char *next;
> +
> +	for (metadata = (struct fanotify_event_metadata *) buffer;
> +	     FAN_EVENT_OK(metadata, len);
> +	     metadata = FAN_EVENT_NEXT(metadata, len)) {
> +		next = (char *)metadata + metadata->event_len;
> +		if (metadata->mask != FAN_FS_ERROR) {
> +			printf("unexpected FAN MARK: %llx\n", metadata->mask);
> +			goto next_event;
> +		} else if (metadata->fd != FAN_NOFD) {
> +			printf("Unexpected fd (!= FAN_NOFD)\n");
> +			goto next_event;
> +		}
> +
> +		printf("FAN_FS_ERROR found len=%d\n", metadata->event_len);
> +
> +		error = (struct fanotify_event_info_error *) (metadata+1);
> +		if (error->hdr.info_type != FAN_EVENT_INFO_TYPE_ERROR) {
> +			printf("unknown record: %d (Expecting TYPE_ERROR)\n",
> +			       error->hdr.info_type);
> +			goto next_event;
> +		}

The ordering of additional infos is undefined. Your code must not rely on
the fact that FAN_EVENT_INFO_TYPE_ERROR comes first and
FAN_EVENT_INFO_TYPE_FID second. Also you should ignore (maybe just print
type and len in this sample code) when you see unexpected info types as
later additions to the API may add additional info records 

> +
> +		printf("\tGeneric Error Record: len=%d\n", error->hdr.len);
> +		printf("\terror: %d\n", error->error);
> +		printf("\terror_count: %d\n", error->error_count);
> +
> +		fid = (struct fanotify_event_info_fid *) (error + 1);
> +		if ((char *) fid >= next) {
> +			printf("Event doesn't have FID\n");
> +			goto next_event;
> +		}
> +		printf("FID record found\n");
> +
> +		if (fid->hdr.info_type != FAN_EVENT_INFO_TYPE_FID) {
> +			printf("unknown record: %d (Expecting TYPE_FID)\n",
> +			       fid->hdr.info_type);
> +			goto next_event;
> +		}
> +		printf("\tfsid: %x%x\n", fid->fsid.val[0], fid->fsid.val[1]);
> +		print_fh((struct file_handle *) &fid->handle);
> +
> +next_event:
> +		printf("---\n\n");
> +	}

								Honza
-- 
Jan Kara <jack@...e.com>
SUSE Labs, CR