| Message ID | 1546336466-11039-1-git-send-email-penguin-kernel@I-love.SAKURA.ne.jp (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | inotify: Fix fd refcount leak in inotify_add_watch(). | expand |
On Tue, Jan 1, 2019 at 11:54 AM Tetsuo Handa <penguin-kernel@i-love.sakura.ne.jp> wrote: > > Commit 4d97f7d53da7dc83 ("inotify: Add flag IN_MASK_CREATE for > inotify_add_watch()") forgot to call fdput() before bailing out. > > Signed-off-by: Tetsuo Handa <penguin-kernel@I-love.SAKURA.ne.jp> > --- > fs/notify/inotify/inotify_user.c | 6 ++++-- > 1 file changed, 4 insertions(+), 2 deletions(-) > > diff --git a/fs/notify/inotify/inotify_user.c b/fs/notify/inotify/inotify_user.c > index 105576d..798f125 100644 > --- a/fs/notify/inotify/inotify_user.c > +++ b/fs/notify/inotify/inotify_user.c > @@ -724,8 +724,10 @@ static int do_inotify_init(int flags) > return -EBADF; > > /* IN_MASK_ADD and IN_MASK_CREATE don't make sense together */ > - if (unlikely((mask & IN_MASK_ADD) && (mask & IN_MASK_CREATE))) > - return -EINVAL; > + if (unlikely((mask & IN_MASK_ADD) && (mask & IN_MASK_CREATE))) { > + ret = -EINVAL; > + goto fput_and_out; > + } > Thanks for the fix. A matter of personal taste, but for brevity, I would prefer initializing ret = -EINVAL once and one liner goto here and in the case below. Either way, you may add: Reviewed-by: Amir Goldstein <amir73il@gmail.com> Thanks, Amir.
On Tue 01-01-19 12:50:32, Amir Goldstein wrote: > On Tue, Jan 1, 2019 at 11:54 AM Tetsuo Handa > <penguin-kernel@i-love.sakura.ne.jp> wrote: > > > > Commit 4d97f7d53da7dc83 ("inotify: Add flag IN_MASK_CREATE for > > inotify_add_watch()") forgot to call fdput() before bailing out. > > > > Signed-off-by: Tetsuo Handa <penguin-kernel@I-love.SAKURA.ne.jp> > > --- > > fs/notify/inotify/inotify_user.c | 6 ++++-- > > 1 file changed, 4 insertions(+), 2 deletions(-) > > > > diff --git a/fs/notify/inotify/inotify_user.c b/fs/notify/inotify/inotify_user.c > > index 105576d..798f125 100644 > > --- a/fs/notify/inotify/inotify_user.c > > +++ b/fs/notify/inotify/inotify_user.c > > @@ -724,8 +724,10 @@ static int do_inotify_init(int flags) > > return -EBADF; > > > > /* IN_MASK_ADD and IN_MASK_CREATE don't make sense together */ > > - if (unlikely((mask & IN_MASK_ADD) && (mask & IN_MASK_CREATE))) > > - return -EINVAL; > > + if (unlikely((mask & IN_MASK_ADD) && (mask & IN_MASK_CREATE))) { > > + ret = -EINVAL; > > + goto fput_and_out; > > + } > > > > Thanks for the fix. > A matter of personal taste, but for brevity, I would prefer > initializing ret = -EINVAL > once and one liner goto here and in the case below. > > Either way, you may add: > Reviewed-by: Amir Goldstein <amir73il@gmail.com> Thanks for the patch Tetsuo and for the review Amir. I've added the patch to my tree (added CC to stable) and will push it to Linus for rc2. Honza
Jan, will this fix be sent to linux.git shortly? I still can't find this fix. On 2019/01/03 2:48, Jan Kara wrote: > On Tue 01-01-19 12:50:32, Amir Goldstein wrote: >> On Tue, Jan 1, 2019 at 11:54 AM Tetsuo Handa >> <penguin-kernel@i-love.sakura.ne.jp> wrote: >>> >>> Commit 4d97f7d53da7dc83 ("inotify: Add flag IN_MASK_CREATE for >>> inotify_add_watch()") forgot to call fdput() before bailing out. >>> >>> Signed-off-by: Tetsuo Handa <penguin-kernel@I-love.SAKURA.ne.jp> >>> --- >>> fs/notify/inotify/inotify_user.c | 6 ++++-- >>> 1 file changed, 4 insertions(+), 2 deletions(-) >>> >>> diff --git a/fs/notify/inotify/inotify_user.c b/fs/notify/inotify/inotify_user.c >>> index 105576d..798f125 100644 >>> --- a/fs/notify/inotify/inotify_user.c >>> +++ b/fs/notify/inotify/inotify_user.c >>> @@ -724,8 +724,10 @@ static int do_inotify_init(int flags) >>> return -EBADF; >>> >>> /* IN_MASK_ADD and IN_MASK_CREATE don't make sense together */ >>> - if (unlikely((mask & IN_MASK_ADD) && (mask & IN_MASK_CREATE))) >>> - return -EINVAL; >>> + if (unlikely((mask & IN_MASK_ADD) && (mask & IN_MASK_CREATE))) { >>> + ret = -EINVAL; >>> + goto fput_and_out; >>> + } >>> >> >> Thanks for the fix. >> A matter of personal taste, but for brevity, I would prefer >> initializing ret = -EINVAL >> once and one liner goto here and in the case below. >> >> Either way, you may add: >> Reviewed-by: Amir Goldstein <amir73il@gmail.com> > > Thanks for the patch Tetsuo and for the review Amir. I've added the patch > to my tree (added CC to stable) and will push it to Linus for rc2. > > Honza >
On Sun 20-01-19 11:56:27, Tetsuo Handa wrote: > Jan, will this fix be sent to linux.git shortly? > I still can't find this fix. Yes, I'll send it this week. But thanks for pinging as I've forgotten to push it out to my for_next branch. Honza > On 2019/01/03 2:48, Jan Kara wrote: > > On Tue 01-01-19 12:50:32, Amir Goldstein wrote: > >> On Tue, Jan 1, 2019 at 11:54 AM Tetsuo Handa > >> <penguin-kernel@i-love.sakura.ne.jp> wrote: > >>> > >>> Commit 4d97f7d53da7dc83 ("inotify: Add flag IN_MASK_CREATE for > >>> inotify_add_watch()") forgot to call fdput() before bailing out. > >>> > >>> Signed-off-by: Tetsuo Handa <penguin-kernel@I-love.SAKURA.ne.jp> > >>> --- > >>> fs/notify/inotify/inotify_user.c | 6 ++++-- > >>> 1 file changed, 4 insertions(+), 2 deletions(-) > >>> > >>> diff --git a/fs/notify/inotify/inotify_user.c b/fs/notify/inotify/inotify_user.c > >>> index 105576d..798f125 100644 > >>> --- a/fs/notify/inotify/inotify_user.c > >>> +++ b/fs/notify/inotify/inotify_user.c > >>> @@ -724,8 +724,10 @@ static int do_inotify_init(int flags) > >>> return -EBADF; > >>> > >>> /* IN_MASK_ADD and IN_MASK_CREATE don't make sense together */ > >>> - if (unlikely((mask & IN_MASK_ADD) && (mask & IN_MASK_CREATE))) > >>> - return -EINVAL; > >>> + if (unlikely((mask & IN_MASK_ADD) && (mask & IN_MASK_CREATE))) { > >>> + ret = -EINVAL; > >>> + goto fput_and_out; > >>> + } > >>> > >> > >> Thanks for the fix. > >> A matter of personal taste, but for brevity, I would prefer > >> initializing ret = -EINVAL > >> once and one liner goto here and in the case below. > >> > >> Either way, you may add: > >> Reviewed-by: Amir Goldstein <amir73il@gmail.com> > > > > Thanks for the patch Tetsuo and for the review Amir. I've added the patch > > to my tree (added CC to stable) and will push it to Linus for rc2. > > > > Honza > >
diff --git a/fs/notify/inotify/inotify_user.c b/fs/notify/inotify/inotify_user.c index 105576d..798f125 100644 --- a/fs/notify/inotify/inotify_user.c +++ b/fs/notify/inotify/inotify_user.c @@ -724,8 +724,10 @@ static int do_inotify_init(int flags) return -EBADF; /* IN_MASK_ADD and IN_MASK_CREATE don't make sense together */ - if (unlikely((mask & IN_MASK_ADD) && (mask & IN_MASK_CREATE))) - return -EINVAL; + if (unlikely((mask & IN_MASK_ADD) && (mask & IN_MASK_CREATE))) { + ret = -EINVAL; + goto fput_and_out; + } /* verify that this is indeed an inotify instance */ if (unlikely(f.file->f_op != &inotify_fops)) {
Commit 4d97f7d53da7dc83 ("inotify: Add flag IN_MASK_CREATE for inotify_add_watch()") forgot to call fdput() before bailing out. Signed-off-by: Tetsuo Handa <penguin-kernel@I-love.SAKURA.ne.jp> --- fs/notify/inotify/inotify_user.c | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-)