linux-kernel - Re: [PATCH 4/4] fanotify: Expose the file changes to the user

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: <201011291711.57876.tvrtko.ursulin@sophos.com>
Date:	Mon, 29 Nov 2010 17:11:57 +0000
From:	Tvrtko Ursulin <tvrtko.ursulin@...hos.com>
To:	Eric Paris <eparis@...hat.com>
CC:	Alexey Zaytsev <alexey.zaytsev@...il.com>,
	Scott Hassan <hassan@...funk.com>, Jan Kara <jack@...e.cz>,
	"agruen@...bit.com" <agruen@...bit.com>,
	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
	"stefan@...ttcher.org" <stefan@...ttcher.org>,
	Al Viro <viro@...iv.linux.org.uk>,
	"linux-fsdevel@...r.kernel.org" <linux-fsdevel@...r.kernel.org>
Subject: Re: [PATCH 4/4] fanotify: Expose the file changes to the user

On Monday 29 Nov 2010 16:14:54 Eric Paris wrote:
> On Fri, 2010-11-26 at 10:11 +0000, Tvrtko Ursulin wrote:
> > On Monday 22 Nov 2010 00:37:21 Alexey Zaytsev wrote:
> > >  struct fanotify_event_metadata {
> > >
> > > -       __u16 event_len;
> > > +       __u16 event_len; /* Including the options */
> > >
> > >         __u8 vers;
> > >
> > > -       __u8 reserved;
> > > +       __u8 options_offset; /* Aka header length */
> > >
> > >         __s32 fd;
> > >         __aligned_u64 mask;
> > >         __s32 pid;
> > >
> > > +       /* Options go here. */
> > >
> > >  };
> >
> > Also, options_offset is, if I understood it correctly, basically the
> > lenght of fanotify_event_metadata. As such, isn't that field redundant
> > since the lenght is implied from the protocol version?
>
> Ok, the way I envision the interface is the fanotify_event_metadata is
> going to be information that is common for all event types.  If a
> particular event type (in the case being brought forth MODIFY and
> CLOSE_WRITE) needs more information it will use the optional headers.
> I'd still like to be able to add generic info, ex: the uid of the
> requesting process, to the generic event metadata.  We might also
> someday want to use the optional headers for things like the inotify
> move cookie.  Why waste space in every event when only the move type
> events cared about this cookie?
>
> This necessitates options_offset.  Lets say I compile my userspace app
> as version 3.  I know that fanotify_event_metadata is 14 bytes long in
> version 3.  Thus I start looking at byte 15 for the first optional
> header.  But the kernel is version 4 and fanotify_event_metadata is
> actually 18 bytes long.  Whoops, userspace should be looking at byte 19
> for the header but accidentally looked at byte 15.  options_offset will
> allow a version 3 userspace to run on a version 4 kernel while skipping
> bytes 15-18 which it doesn't understand.

I see now what you want to do here. I assumed metadata is fixed for ever and
all expansions will come via options. That is why I said offset field is
redundant. If you want to leave option for changing metadata open then yes, I
agree it is needed.

> At the end of all of this discussion what did people finally agree on?
> Something like this?
>
> struct fanotify_opt_hdr {
>         __u16 type;
>         __u16 len;
>         /* Payload goes here. */
> };
>
> struct fanotify_event_metadata {
>         __u32 event_len; /* Including the options */
>         __u16 vers;
>         __u16 options_offset; /* Aka header length */
>         __s32 fd;
>         __aligned_u64 mask;
>         __s32 pid;
>         /* Options go here. */
> };
>
> We could be ABI compatible if we let vers as a __u32 and added
> options_offset at the end of the structure...   Do we have strong
> feelings on either side about maintaining compatibility or about
> structure lengths?

No strong feelings here on either. I do not really think maintaining
compatibility with an unreleased protocol is worth it. And I would not like to
limit any field to 8-bits if possible.

Tvrtko

Sophos Limited, The Pentagon, Abingdon Science Park, Abingdon, OX14 3YP, United Kingdom.
Company Reg No 2096520. VAT Reg No GB 991 2418 08.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/