diff mbox series

[19/20] fanotify: move event name into fanotify_fh

Message ID 20200612093343.5669-20-amir73il@gmail.com (mailing list archive)
State New, archived
Headers show
Series Prep work for fanotify named events | expand

Commit Message

Amir Goldstein June 12, 2020, 9:33 a.m. UTC
An fanotify event name is always recorded relative to a dir fh.
Move the name_len members of fanotify_name_event into unused space
in struct fanotify_fh.

We add a name_offset member to allow packing a binary blob before
the name string in the variable size buffer. We are going to use
that space to store the child fid.

It also fixes a bug in fanotify_alloc_name_event() which used an
allocation size 7 bytes bigger than required size, because it used
sizeof(struct fanotify_name_event) without deducting that 7 bytes
alignment padding.

Signed-off-by: Amir Goldstein <amir73il@gmail.com>
---
 fs/notify/fanotify/fanotify.c      | 27 ++++++++-----
 fs/notify/fanotify/fanotify.h      | 62 +++++++++++++++++++++++-------
 fs/notify/fanotify/fanotify_user.c | 23 +++++------
 3 files changed, 75 insertions(+), 37 deletions(-)

Comments

Jan Kara July 3, 2020, 4:02 p.m. UTC | #1
On Fri 12-06-20 12:33:42, Amir Goldstein wrote:
> An fanotify event name is always recorded relative to a dir fh.
> Move the name_len members of fanotify_name_event into unused space
> in struct fanotify_fh.
> 
> We add a name_offset member to allow packing a binary blob before
> the name string in the variable size buffer. We are going to use
> that space to store the child fid.

So how much is this packing going to save us? Currently it is 1 byte for
name events (modulo that fanotify_alloc_name_event_bug() you mention
below). With the additional fanotify_fh in the event, we'll save two more
bytes by the packing. So that doesn't really seem to be worth it to me.
Am I missing some other benefit?

Maybe your main motivation (which is not mentioned in the changelog at all
BTW) is that the whole game of inline vs out of line file handles is
pointless when we kmalloc() the event anyway because of the name? And it's
actively wasteful in case handles don't fit in the inline space. I agree
with that and it's good observation. But I'd rather leave fanotify_fh
struct alone for the cases where we want to bother with inline vs out of line
file handles and define new way of partitioning space at the end of the
event among one or two file handles and name. Something like:

struct fanotify_dynamic_info {
	u8 dirfh_len;
	u8 filefh_len;
	u8 name_len;
	unsigned char buf[];
};

And at appropriate offsets (0, dirfh_len, dirfh_len + filefh_len) there
would be additional info (e.g. type + fh for file handles). Maybe this
format will require some tweaking so that processing of both storage types
of file handles can be reasonably uniform but at this point it seems
cleaner than what you try to do fanotify_fh with combining lenghts and
offsets and some blobs in the middle...

> It also fixes a bug in fanotify_alloc_name_event() which used an
> allocation size 7 bytes bigger than required size, because it used
> sizeof(struct fanotify_name_event) without deducting that 7 bytes
> alignment padding.

								Honza
> 
> Signed-off-by: Amir Goldstein <amir73il@gmail.com>
> ---
>  fs/notify/fanotify/fanotify.c      | 27 ++++++++-----
>  fs/notify/fanotify/fanotify.h      | 62 +++++++++++++++++++++++-------
>  fs/notify/fanotify/fanotify_user.c | 23 +++++------
>  3 files changed, 75 insertions(+), 37 deletions(-)
> 
> diff --git a/fs/notify/fanotify/fanotify.c b/fs/notify/fanotify/fanotify.c
> index 3885bf63976b..3a2d48edaddd 100644
> --- a/fs/notify/fanotify/fanotify.c
> +++ b/fs/notify/fanotify/fanotify.c
> @@ -52,15 +52,20 @@ static bool fanotify_fid_event_equal(struct fanotify_fid_event *ffe1,
>  static bool fanotify_name_event_equal(struct fanotify_name_event *fne1,
>  				      struct fanotify_name_event *fne2)
>  {
> +	struct fanotify_fh *dfh1 = &fne1->dir_fh;
> +	struct fanotify_fh *dfh2 = &fne2->dir_fh;
> +
>  	/* Do not merge name events without dir fh */
> -	if (!fne1->dir_fh.len)
> +	if (!dfh1->len)
>  		return false;
>  
> -	if (fne1->name_len != fne2->name_len ||
> -	    !fanotify_fh_equal(&fne1->dir_fh, &fne2->dir_fh))
> +	if (dfh1->name_len != dfh2->name_len ||
> +	    dfh1->name_offset != dfh2->name_offset ||
> +	    !fanotify_fh_equal(dfh1, dfh2))
>  		return false;
>  
> -	return !memcmp(fne1->name, fne2->name, fne1->name_len);
> +	return !memcmp(fanotify_fh_name(dfh1), fanotify_fh_name(dfh2),
> +		       dfh1->name_len);
>  }
>  
>  static bool fanotify_should_merge(struct fsnotify_event *old_fsn,
> @@ -284,8 +289,7 @@ static void fanotify_encode_fh(struct fanotify_fh *fh, struct inode *inode,
>  	void *buf = fh->buf;
>  	int err;
>  
> -	fh->type = FILEID_ROOT;
> -	fh->len = 0;
> +	fanotify_fh_init(fh);
>  	if (!inode)
>  		return;
>  
> @@ -314,6 +318,10 @@ static void fanotify_encode_fh(struct fanotify_fh *fh, struct inode *inode,
>  
>  	fh->type = type;
>  	fh->len = bytes;
> +	if (fh->len > FANOTIFY_INLINE_FH_LEN)
> +		fh->name_offset = FANOTIFY_INLINE_FH_LEN;
> +	else
> +		fh->name_offset = fh->len;
>  
>  	return;
>  
> @@ -401,6 +409,7 @@ struct fanotify_event *fanotify_alloc_name_event(struct inode *id,
>  						 gfp_t gfp)
>  {
>  	struct fanotify_name_event *fne;
> +	struct fanotify_fh *dfh;
>  
>  	fne = kmalloc(sizeof(*fne) + file_name->len + 1, gfp);
>  	if (!fne)
> @@ -408,9 +417,9 @@ struct fanotify_event *fanotify_alloc_name_event(struct inode *id,
>  
>  	fne->fae.type = FANOTIFY_EVENT_TYPE_FID_NAME;
>  	fne->fsid = *fsid;
> -	fanotify_encode_fh(&fne->dir_fh, id, gfp);
> -	fne->name_len = file_name->len;
> -	strcpy(fne->name, file_name->name);
> +	dfh = &fne->dir_fh;
> +	fanotify_encode_fh(dfh, id, gfp);
> +	fanotify_fh_copy_name(dfh, file_name);
>  
>  	return &fne->fae;
>  }
> diff --git a/fs/notify/fanotify/fanotify.h b/fs/notify/fanotify/fanotify.h
> index 1b2a3bbe6008..8cb062eefd3e 100644
> --- a/fs/notify/fanotify/fanotify.h
> +++ b/fs/notify/fanotify/fanotify.h
> @@ -23,13 +23,24 @@ enum {
>   * stored in either the first or last 2 dwords.
>   */
>  #define FANOTIFY_INLINE_FH_LEN	(3 << 2)
> +#define FANOTIFY_FH_HDR_LEN	offsetof(struct fanotify_fh, buf)
>  
>  struct fanotify_fh {
> -	unsigned char buf[FANOTIFY_INLINE_FH_LEN];
>  	u8 type;
>  	u8 len;
> +	u8 name_offset;
> +	u8 name_len;
> +	unsigned char buf[FANOTIFY_INLINE_FH_LEN];
>  } __aligned(4);
>  
> +static inline void fanotify_fh_init(struct fanotify_fh *fh)
> +{
> +	fh->type = FILEID_ROOT;
> +	fh->len = 0;
> +	fh->name_offset = 0;
> +	fh->name_len = 0;
> +}
> +
>  static inline bool fanotify_fh_has_ext_buf(struct fanotify_fh *fh)
>  {
>  	return fh->len > FANOTIFY_INLINE_FH_LEN;
> @@ -37,6 +48,7 @@ static inline bool fanotify_fh_has_ext_buf(struct fanotify_fh *fh)
>  
>  static inline char **fanotify_fh_ext_buf_ptr(struct fanotify_fh *fh)
>  {
> +	BUILD_BUG_ON(FANOTIFY_FH_HDR_LEN % 4);
>  	BUILD_BUG_ON(__alignof__(char *) - 4 + sizeof(char *) >
>  		     FANOTIFY_INLINE_FH_LEN);
>  	return (char **)ALIGN((unsigned long)(fh->buf), __alignof__(char *));
> @@ -52,6 +64,35 @@ static inline void *fanotify_fh_buf(struct fanotify_fh *fh)
>  	return fanotify_fh_has_ext_buf(fh) ? fanotify_fh_ext_buf(fh) : fh->buf;
>  }
>  
> +static inline int fanotify_fh_blob_len(struct fanotify_fh *fh)
> +{
> +	if (fh->name_offset <= fh->len)
> +		return 0;
> +
> +	/* Is there a space between end of fh_buf and start of name? */
> +	return fh->name_offset - fh->len;
> +}
> +
> +static inline void *fanotify_fh_blob(struct fanotify_fh *fh)
> +{
> +	if (fh->name_offset <= fh->len)
> +		return NULL;
> +
> +	return fh->buf + fh->len;
> +}
> +
> +static inline const char *fanotify_fh_name(struct fanotify_fh *fh)
> +{
> +	return fh->name_len ? fh->buf + fh->name_offset : NULL;
> +}
> +
> +static inline void fanotify_fh_copy_name(struct fanotify_fh *fh,
> +					 const struct qstr *name)
> +{
> +	fh->name_len = name->len;
> +	strcpy(fh->buf + fh->name_offset, name->name);
> +}
> +
>  /*
>   * Common structure for fanotify events. Concrete structs are allocated in
>   * fanotify_handle_event() and freed when the information is retrieved by
> @@ -93,12 +134,16 @@ FANOTIFY_FE(struct fanotify_event *event)
>  	return container_of(event, struct fanotify_fid_event, fae);
>  }
>  
> +/*
> + * This is identical to struct fanotify_fid_event, but allocated with variable
> + * size kmalloc and should have positive value of dir_fh.name_len.
> + * Keeping the separate struct definition for semantics and type safety -
> + * an event should be cast to this type IFF it was allocated using kmalloc.
> + */
>  struct fanotify_name_event {
>  	struct fanotify_event fae;
>  	__kernel_fsid_t fsid;
>  	struct fanotify_fh dir_fh;
> -	u8 name_len;
> -	char name[];
>  };
>  
>  static inline struct fanotify_name_event *
> @@ -142,17 +187,6 @@ static inline int fanotify_event_object_fh_len(struct fanotify_event *event)
>  	return fh ? fh->len : 0;
>  }
>  
> -static inline bool fanotify_event_has_name(struct fanotify_event *event)
> -{
> -	return event->type == FANOTIFY_EVENT_TYPE_FID_NAME;
> -}
> -
> -static inline int fanotify_event_name_len(struct fanotify_event *event)
> -{
> -	return fanotify_event_has_name(event) ?
> -		FANOTIFY_NE(event)->name_len : 0;
> -}
> -
>  struct fanotify_path_event {
>  	struct fanotify_event fae;
>  	struct path path;
> diff --git a/fs/notify/fanotify/fanotify_user.c b/fs/notify/fanotify/fanotify_user.c
> index 42b8cc51cb3f..af8268b44c68 100644
> --- a/fs/notify/fanotify/fanotify_user.c
> +++ b/fs/notify/fanotify/fanotify_user.c
> @@ -68,17 +68,14 @@ static int fanotify_event_info_len(struct fanotify_event *event)
>  {
>  	int info_len = 0;
>  	int fh_len = fanotify_event_object_fh_len(event);
> +	struct fanotify_fh *dfh = fanotify_event_dir_fh(event);
> +
> +	if (dfh)
> +		info_len += fanotify_fid_info_len(dfh->len, dfh->name_len);
>  
>  	if (fh_len)
>  		info_len += fanotify_fid_info_len(fh_len, 0);
>  
> -	if (fanotify_event_name_len(event)) {
> -		struct fanotify_name_event *fne = FANOTIFY_NE(event);
> -
> -		info_len += fanotify_fid_info_len(fne->dir_fh.len,
> -						  fne->name_len);
> -	}
> -
>  	return info_len;
>  }
>  
> @@ -305,6 +302,7 @@ static ssize_t copy_event_to_user(struct fsnotify_group *group,
>  {
>  	struct fanotify_event_metadata metadata;
>  	struct path *path = fanotify_event_path(event);
> +	struct fanotify_fh *dfh = fanotify_event_dir_fh(event);
>  	struct file *f = NULL;
>  	int ret, fd = FAN_NOFD;
>  
> @@ -346,13 +344,10 @@ static ssize_t copy_event_to_user(struct fsnotify_group *group,
>  		fd_install(fd, f);
>  
>  	/* Event info records order is: dir fid + name, child fid */
> -	if (fanotify_event_name_len(event)) {
> -		struct fanotify_name_event *fne = FANOTIFY_NE(event);
> -
> -		ret = copy_info_to_user(fanotify_event_fsid(event),
> -					fanotify_event_dir_fh(event),
> -					fne->name, fne->name_len,
> -					buf, count);
> +	if (dfh) {
> +		ret = copy_info_to_user(fanotify_event_fsid(event), dfh,
> +					fanotify_fh_name(dfh),
> +					dfh->name_len, buf, count);
>  		if (ret < 0)
>  			return ret;
>  
> -- 
> 2.17.1
>
Amir Goldstein July 6, 2020, 8:21 a.m. UTC | #2
On Fri, Jul 3, 2020 at 7:02 PM Jan Kara <jack@suse.cz> wrote:
>
> On Fri 12-06-20 12:33:42, Amir Goldstein wrote:
> > An fanotify event name is always recorded relative to a dir fh.
> > Move the name_len members of fanotify_name_event into unused space
> > in struct fanotify_fh.
> >
> > We add a name_offset member to allow packing a binary blob before
> > the name string in the variable size buffer. We are going to use
> > that space to store the child fid.
>
> So how much is this packing going to save us? Currently it is 1 byte for
> name events (modulo that fanotify_alloc_name_event_bug() you mention
> below). With the additional fanotify_fh in the event, we'll save two more
> bytes by the packing. So that doesn't really seem to be worth it to me.
> Am I missing some other benefit?
>
> Maybe your main motivation (which is not mentioned in the changelog at all
> BTW) is that the whole game of inline vs out of line file handles is
> pointless when we kmalloc() the event anyway because of the name?

The only motivation, which is written in the commit message is to make
space to store the child file handle. Saving space is just a by product.
In fact, the new parceling code looses this space back to alignment
and I am perfectly fine with that.

> And it's
> actively wasteful in case handles don't fit in the inline space. I agree
> with that and it's good observation. But I'd rather leave fanotify_fh
> struct alone for the cases where we want to bother with inline vs out of line
> file handles and define new way of partitioning space at the end of the
> event among one or two file handles and name. Something like:
>
> struct fanotify_dynamic_info {

I called this fanotify_info. There is no ambiguity that justifies _dynamic_.
The encapsulations are:

fanotify_fid_event { ..., fanotify_fid { ..,buf[INLINE_BUF] } }
fanotify_name_event { ..., fanotify_info { fanotify_fid { ..,buf[..]
}+, name[..] }


>         u8 dirfh_len;
>         u8 filefh_len;

I called these {dir,file}_fh_totlen to distinguish from fh->len, which does not
include the size of the fanotify_fh header fields.

>         u8 name_len;
>         unsigned char buf[];

This had to be 4 bytes aligned to contain fanotify_fh.

> };
>
> And at appropriate offsets (0, dirfh_len, dirfh_len + filefh_len) there
> would be additional info (e.g. type + fh for file handles). Maybe this
> format will require some tweaking so that processing of both storage types
> of file handles can be reasonably uniform but at this point it seems
> cleaner than what you try to do fanotify_fh with combining lenghts and
> offsets and some blobs in the middle...
>

I tried your suggestion (with the minor modifications above) and I
like the result.
Pushed prep series with 2 last patches changed to branch fanotify_prep.
Old prep series is at fanotify_prep-v2.
Pushed tested full series adapted to this change to fanotify_name_fid.
Old full series is at fanotify_name_fid-v4.

There was almost no changes to the fanotify_name_fid patches besides
adapting the accessors, e.g.:
-               fanotify_fh_blob(&FANOTIFY_NE(event)->dir_fh);
+              fanotify_info_file_fh(&FANOTIFY_NE(event)->info);

Please let me know if you want me to post fanotify_name_fid-v5 with these
changes.

Thanks,
Amir.
Jan Kara July 6, 2020, 3:24 p.m. UTC | #3
On Mon 06-07-20 11:21:24, Amir Goldstein wrote:
> On Fri, Jul 3, 2020 at 7:02 PM Jan Kara <jack@suse.cz> wrote:
> >
> > On Fri 12-06-20 12:33:42, Amir Goldstein wrote:
> > > An fanotify event name is always recorded relative to a dir fh.
> > > Move the name_len members of fanotify_name_event into unused space
> > > in struct fanotify_fh.
> > >
> > > We add a name_offset member to allow packing a binary blob before
> > > the name string in the variable size buffer. We are going to use
> > > that space to store the child fid.
> >
> > So how much is this packing going to save us? Currently it is 1 byte for
> > name events (modulo that fanotify_alloc_name_event_bug() you mention
> > below). With the additional fanotify_fh in the event, we'll save two more
> > bytes by the packing. So that doesn't really seem to be worth it to me.
> > Am I missing some other benefit?
> >
> > Maybe your main motivation (which is not mentioned in the changelog at all
> > BTW) is that the whole game of inline vs out of line file handles is
> > pointless when we kmalloc() the event anyway because of the name?
> 
> The only motivation, which is written in the commit message is to make
> space to store the child file handle. Saving space is just a by product.
> In fact, the new parceling code looses this space back to alignment
> and I am perfectly fine with that.

Yeah, I think the loss is acceptable.

> I tried your suggestion (with the minor modifications above) and I
> like the result.
> Pushed prep series with 2 last patches changed to branch fanotify_prep.
> Old prep series is at fanotify_prep-v2.

Yeah, I like the result as well. I've left some minor comments on github.
Please repost the preparatory series once you address the comments so that
we have something for final review and picking up into my tree.

> Pushed tested full series adapted to this change to fanotify_name_fid.
> Old full series is at fanotify_name_fid-v4.
> 
> There was almost no changes to the fanotify_name_fid patches besides
> adapting the accessors, e.g.:
> -               fanotify_fh_blob(&FANOTIFY_NE(event)->dir_fh);
> +              fanotify_info_file_fh(&FANOTIFY_NE(event)->info);
> 
> Please let me know if you want me to post fanotify_name_fid-v5 with these
> changes.

No need to repost at this point I guess. I can do a high-level check with
what I have...

								Honza
diff mbox series

Patch

diff --git a/fs/notify/fanotify/fanotify.c b/fs/notify/fanotify/fanotify.c
index 3885bf63976b..3a2d48edaddd 100644
--- a/fs/notify/fanotify/fanotify.c
+++ b/fs/notify/fanotify/fanotify.c
@@ -52,15 +52,20 @@  static bool fanotify_fid_event_equal(struct fanotify_fid_event *ffe1,
 static bool fanotify_name_event_equal(struct fanotify_name_event *fne1,
 				      struct fanotify_name_event *fne2)
 {
+	struct fanotify_fh *dfh1 = &fne1->dir_fh;
+	struct fanotify_fh *dfh2 = &fne2->dir_fh;
+
 	/* Do not merge name events without dir fh */
-	if (!fne1->dir_fh.len)
+	if (!dfh1->len)
 		return false;
 
-	if (fne1->name_len != fne2->name_len ||
-	    !fanotify_fh_equal(&fne1->dir_fh, &fne2->dir_fh))
+	if (dfh1->name_len != dfh2->name_len ||
+	    dfh1->name_offset != dfh2->name_offset ||
+	    !fanotify_fh_equal(dfh1, dfh2))
 		return false;
 
-	return !memcmp(fne1->name, fne2->name, fne1->name_len);
+	return !memcmp(fanotify_fh_name(dfh1), fanotify_fh_name(dfh2),
+		       dfh1->name_len);
 }
 
 static bool fanotify_should_merge(struct fsnotify_event *old_fsn,
@@ -284,8 +289,7 @@  static void fanotify_encode_fh(struct fanotify_fh *fh, struct inode *inode,
 	void *buf = fh->buf;
 	int err;
 
-	fh->type = FILEID_ROOT;
-	fh->len = 0;
+	fanotify_fh_init(fh);
 	if (!inode)
 		return;
 
@@ -314,6 +318,10 @@  static void fanotify_encode_fh(struct fanotify_fh *fh, struct inode *inode,
 
 	fh->type = type;
 	fh->len = bytes;
+	if (fh->len > FANOTIFY_INLINE_FH_LEN)
+		fh->name_offset = FANOTIFY_INLINE_FH_LEN;
+	else
+		fh->name_offset = fh->len;
 
 	return;
 
@@ -401,6 +409,7 @@  struct fanotify_event *fanotify_alloc_name_event(struct inode *id,
 						 gfp_t gfp)
 {
 	struct fanotify_name_event *fne;
+	struct fanotify_fh *dfh;
 
 	fne = kmalloc(sizeof(*fne) + file_name->len + 1, gfp);
 	if (!fne)
@@ -408,9 +417,9 @@  struct fanotify_event *fanotify_alloc_name_event(struct inode *id,
 
 	fne->fae.type = FANOTIFY_EVENT_TYPE_FID_NAME;
 	fne->fsid = *fsid;
-	fanotify_encode_fh(&fne->dir_fh, id, gfp);
-	fne->name_len = file_name->len;
-	strcpy(fne->name, file_name->name);
+	dfh = &fne->dir_fh;
+	fanotify_encode_fh(dfh, id, gfp);
+	fanotify_fh_copy_name(dfh, file_name);
 
 	return &fne->fae;
 }
diff --git a/fs/notify/fanotify/fanotify.h b/fs/notify/fanotify/fanotify.h
index 1b2a3bbe6008..8cb062eefd3e 100644
--- a/fs/notify/fanotify/fanotify.h
+++ b/fs/notify/fanotify/fanotify.h
@@ -23,13 +23,24 @@  enum {
  * stored in either the first or last 2 dwords.
  */
 #define FANOTIFY_INLINE_FH_LEN	(3 << 2)
+#define FANOTIFY_FH_HDR_LEN	offsetof(struct fanotify_fh, buf)
 
 struct fanotify_fh {
-	unsigned char buf[FANOTIFY_INLINE_FH_LEN];
 	u8 type;
 	u8 len;
+	u8 name_offset;
+	u8 name_len;
+	unsigned char buf[FANOTIFY_INLINE_FH_LEN];
 } __aligned(4);
 
+static inline void fanotify_fh_init(struct fanotify_fh *fh)
+{
+	fh->type = FILEID_ROOT;
+	fh->len = 0;
+	fh->name_offset = 0;
+	fh->name_len = 0;
+}
+
 static inline bool fanotify_fh_has_ext_buf(struct fanotify_fh *fh)
 {
 	return fh->len > FANOTIFY_INLINE_FH_LEN;
@@ -37,6 +48,7 @@  static inline bool fanotify_fh_has_ext_buf(struct fanotify_fh *fh)
 
 static inline char **fanotify_fh_ext_buf_ptr(struct fanotify_fh *fh)
 {
+	BUILD_BUG_ON(FANOTIFY_FH_HDR_LEN % 4);
 	BUILD_BUG_ON(__alignof__(char *) - 4 + sizeof(char *) >
 		     FANOTIFY_INLINE_FH_LEN);
 	return (char **)ALIGN((unsigned long)(fh->buf), __alignof__(char *));
@@ -52,6 +64,35 @@  static inline void *fanotify_fh_buf(struct fanotify_fh *fh)
 	return fanotify_fh_has_ext_buf(fh) ? fanotify_fh_ext_buf(fh) : fh->buf;
 }
 
+static inline int fanotify_fh_blob_len(struct fanotify_fh *fh)
+{
+	if (fh->name_offset <= fh->len)
+		return 0;
+
+	/* Is there a space between end of fh_buf and start of name? */
+	return fh->name_offset - fh->len;
+}
+
+static inline void *fanotify_fh_blob(struct fanotify_fh *fh)
+{
+	if (fh->name_offset <= fh->len)
+		return NULL;
+
+	return fh->buf + fh->len;
+}
+
+static inline const char *fanotify_fh_name(struct fanotify_fh *fh)
+{
+	return fh->name_len ? fh->buf + fh->name_offset : NULL;
+}
+
+static inline void fanotify_fh_copy_name(struct fanotify_fh *fh,
+					 const struct qstr *name)
+{
+	fh->name_len = name->len;
+	strcpy(fh->buf + fh->name_offset, name->name);
+}
+
 /*
  * Common structure for fanotify events. Concrete structs are allocated in
  * fanotify_handle_event() and freed when the information is retrieved by
@@ -93,12 +134,16 @@  FANOTIFY_FE(struct fanotify_event *event)
 	return container_of(event, struct fanotify_fid_event, fae);
 }
 
+/*
+ * This is identical to struct fanotify_fid_event, but allocated with variable
+ * size kmalloc and should have positive value of dir_fh.name_len.
+ * Keeping the separate struct definition for semantics and type safety -
+ * an event should be cast to this type IFF it was allocated using kmalloc.
+ */
 struct fanotify_name_event {
 	struct fanotify_event fae;
 	__kernel_fsid_t fsid;
 	struct fanotify_fh dir_fh;
-	u8 name_len;
-	char name[];
 };
 
 static inline struct fanotify_name_event *
@@ -142,17 +187,6 @@  static inline int fanotify_event_object_fh_len(struct fanotify_event *event)
 	return fh ? fh->len : 0;
 }
 
-static inline bool fanotify_event_has_name(struct fanotify_event *event)
-{
-	return event->type == FANOTIFY_EVENT_TYPE_FID_NAME;
-}
-
-static inline int fanotify_event_name_len(struct fanotify_event *event)
-{
-	return fanotify_event_has_name(event) ?
-		FANOTIFY_NE(event)->name_len : 0;
-}
-
 struct fanotify_path_event {
 	struct fanotify_event fae;
 	struct path path;
diff --git a/fs/notify/fanotify/fanotify_user.c b/fs/notify/fanotify/fanotify_user.c
index 42b8cc51cb3f..af8268b44c68 100644
--- a/fs/notify/fanotify/fanotify_user.c
+++ b/fs/notify/fanotify/fanotify_user.c
@@ -68,17 +68,14 @@  static int fanotify_event_info_len(struct fanotify_event *event)
 {
 	int info_len = 0;
 	int fh_len = fanotify_event_object_fh_len(event);
+	struct fanotify_fh *dfh = fanotify_event_dir_fh(event);
+
+	if (dfh)
+		info_len += fanotify_fid_info_len(dfh->len, dfh->name_len);
 
 	if (fh_len)
 		info_len += fanotify_fid_info_len(fh_len, 0);
 
-	if (fanotify_event_name_len(event)) {
-		struct fanotify_name_event *fne = FANOTIFY_NE(event);
-
-		info_len += fanotify_fid_info_len(fne->dir_fh.len,
-						  fne->name_len);
-	}
-
 	return info_len;
 }
 
@@ -305,6 +302,7 @@  static ssize_t copy_event_to_user(struct fsnotify_group *group,
 {
 	struct fanotify_event_metadata metadata;
 	struct path *path = fanotify_event_path(event);
+	struct fanotify_fh *dfh = fanotify_event_dir_fh(event);
 	struct file *f = NULL;
 	int ret, fd = FAN_NOFD;
 
@@ -346,13 +344,10 @@  static ssize_t copy_event_to_user(struct fsnotify_group *group,
 		fd_install(fd, f);
 
 	/* Event info records order is: dir fid + name, child fid */
-	if (fanotify_event_name_len(event)) {
-		struct fanotify_name_event *fne = FANOTIFY_NE(event);
-
-		ret = copy_info_to_user(fanotify_event_fsid(event),
-					fanotify_event_dir_fh(event),
-					fne->name, fne->name_len,
-					buf, count);
+	if (dfh) {
+		ret = copy_info_to_user(fanotify_event_fsid(event), dfh,
+					fanotify_fh_name(dfh),
+					dfh->name_len, buf, count);
 		if (ret < 0)
 			return ret;