From: Greg KH <gregkh@linuxfoundation.org>
To: Amir Goldstein <amir73il@gmail.com>
Cc: Jan Kara <jack@suse.cz>, linux-fsdevel <linux-fsdevel@vger.kernel.org>
Subject: Re: [PATCH 2/2] fsnotify: Protect bail out path of fsnotify_add_mark_locked() properly
Date: Tue, 31 Oct 2017 14:40:24 +0100 [thread overview]
Message-ID: <20171031134024.GA2454@kroah.com> (raw)
In-Reply-To: <CAOQ4uxiTohOnJnhioFkFvE6qhywDxakC2QVfOJuUeFdFL2QibA@mail.gmail.com>
On Tue, Oct 31, 2017 at 02:57:49PM +0200, Amir Goldstein wrote:
> On Tue, Oct 31, 2017 at 2:50 PM, Greg KH <gregkh@linuxfoundation.org> wrote:
> > On Tue, Oct 31, 2017 at 02:26:35PM +0200, Amir Goldstein wrote:
> >> On Tue, Oct 31, 2017 at 11:33 AM, Jan Kara <jack@suse.cz> wrote:
> >> > When fsnotify_add_mark_locked() fails it cleans up the mark it was
> >> > adding. Since the mark is already visible in group's list, we should
> >> > protect update of mark->flags with mark->lock. I'm not aware of any real
> >> > issues this could cause (since we also hold group->mark_mutex) but
> >> > better be safe and obey locking rules properly.
> >> >
> >> > Signed-off-by: Jan Kara <jack@suse.cz>
> >>
> >> Reviewed-by: Amir Goldstein <amir73il@gmail.com>
> >>
> >> IMO, even though this does not fix a concrete bug, if it's worth
> >> fixing in upstream, it's worth fixing in stable.
> >> A future stable fix may either make this into a concrete bug
> >> or just be harder to apply.
> >>
> >> So I suggest to add the Fixes: and Cc: stable tags.
> >>
> >> Greg,
> >>
> >> Do you agree with this reasoning?
> >
> > If it doesn't fix an actual bug, how does that fit with the stable
> > kernel rules?
> >
>
> So this is the case of incorrect code w.r.t locking rules
> that either does not hit a bug because of an indirect protection
> (as Jan wrote in commit) or we did not find how to hit a bug.
>
> Not sure how you want to call this, but if you think it doesn't belong
> for stable we won't send it. That's why I called for your opinion.
How about we get the opinion of the developer and maintainer of the
subsystem, I will defer to them...
next prev parent reply other threads:[~2017-10-31 13:40 UTC|newest]
Thread overview: 12+ messages / expand[flat|nested] mbox.gz Atom feed top
2017-10-31 9:33 [PATCH 0/2] dnotify: Fix ENOMEM handling Jan Kara
2017-10-31 9:33 ` [PATCH 1/2] dnotify: Handle errors from fsnotify_add_mark_locked() in fcntl_dirnotify() Jan Kara
2017-10-31 12:11 ` Amir Goldstein
2017-10-31 15:45 ` Jan Kara
2017-10-31 16:15 ` Jan Kara
2017-10-31 16:28 ` Amir Goldstein
2017-10-31 9:33 ` [PATCH 2/2] fsnotify: Protect bail out path of fsnotify_add_mark_locked() properly Jan Kara
2017-10-31 12:26 ` Amir Goldstein
2017-10-31 12:50 ` Greg KH
2017-10-31 12:57 ` Amir Goldstein
2017-10-31 13:40 ` Greg KH [this message]
2017-10-31 16:10 ` Jan Kara
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20171031134024.GA2454@kroah.com \
--to=gregkh@linuxfoundation.org \
--cc=amir73il@gmail.com \
--cc=jack@suse.cz \
--cc=linux-fsdevel@vger.kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.