On Fri, Jun 11, 2021 at 6:32 AM Matthew Bobrowski <repnop@xxxxxxxxxx> wrote:
>
> Ensure that clean up is performed on the allocated file descriptor and
> struct file object in the event that an error is encountered while copying
> fid info objects. Currently, we return directly to the caller when an error
> is experienced in the fid info copying helper, which isn't ideal given that
> the listener process could be left with a dangling file descriptor in their
> fdtable.
>
> Fixes: 44d705b0370b1 ("fanotify: report name info for FAN_DIR_MODIFY event")
> Fixes: 5e469c830fdb5 ("fanotify: copy event fid info to user")
> Link: https://lore.kernel.org/linux-fsdevel/YMKv1U7tNPK955ho@xxxxxxxxxx/T/#m15361cd6399dad4396aad650de25dbf6b312288e
>
This newline should not be here.
> Signed-off-by: Matthew Bobrowski <repnop@xxxxxxxxxx>
> ---
>
> Hey Amir/Jan,
>
> I wasn't 100% sure what specific commit hash I should be referencing in the
> fix tags, so please let me know if that needs to be changed.
Trick question.
There are two LTS kernels where those fixes are relevant 5.4.y and 5.10.y
(Patch would be picked up for latest stable anyway)
The first Fixes: suggests that the patch should be applied to 5.10+
and the second Fixes: suggests that the patch should be applied to 5.4+
In theory, you could have split this to two patches, one auto applied to 5.4+
and the other auto applied to +5.10.
In practice, this patch would not auto apply to 5.4.y cleanly even if you split
it and also, it's arguably not that critical to worth the effort, so I would
keep the first Fixes: tag and drop the second to avoid the noise of the
stable bots trying to apply the patch.
If you want to do a service to the 5.4.y downstream community,
you can send a backport patch directly to stable list *after* this patch
is applied to master.
>
> Should we also be CC'ing <stable@xxxxxxxxxxxxxxx> so this gets backported?
>
Yes and no.
Actually CC-ing the stable list is not needed, so don't do it.
Cc: tag in the commit message is somewhat redundant to Fixes: tag
these days, but it doesn't hurt to be explicit about intentions.
Specifying:
Cc: <stable@xxxxxxxxxxxxxxx> # v5.10+
Could help as a hint in case the Fixes: tags is for an old commit, but
you know that the patch would not apply before 5.10 and you think it
is not worth the trouble (as in this case).
But if you do specify stable kernel version hint, try not to get it wrong
like I did :-/
https://lore.kernel.org/linux-fsdevel/20210608122829.GI5562@xxxxxxxxxxxxxx/
CC-ing Greg in case my understanding of the stable kernel patch
candidate analysis process is wrong.
Thanks,
Amir.
> fs/notify/fanotify/fanotify_user.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/fs/notify/fanotify/fanotify_user.c b/fs/notify/fanotify/fanotify_user.c
> index be5b6d2c01e7..64864fb40b40 100644
> --- a/fs/notify/fanotify/fanotify_user.c
> +++ b/fs/notify/fanotify/fanotify_user.c
> @@ -471,7 +471,7 @@ static ssize_t copy_event_to_user(struct fsnotify_group *group,
> info_type, fanotify_info_name(info),
> info->name_len, buf, count);
> if (ret < 0)
> - return ret;
> + goto out_close_fd;
>
> buf += ret;
> count -= ret;
> @@ -519,7 +519,7 @@ static ssize_t copy_event_to_user(struct fsnotify_group *group,
> fanotify_event_object_fh(event),
> info_type, dot, dot_len, buf, count);
> if (ret < 0)
> - return ret;
> + goto out_close_fd;
>
> buf += ret;
> count -= ret;
> --
> 2.32.0.272.g935e593368-goog
>
> /M
