linux-kernel - Re: Things I wish I'd known about Inotify

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]

Date:	Fri, 4 Apr 2014 15:08:38 +0200
From:	David Herrmann <dh.herrmann@...il.com>
To:	Michael Kerrisk-manpages <mtk.manpages@...il.com>
Cc:	Robert Love <rlove@...ve.org>, Eric Paris <eparis@...hat.com>,
	"linux-fsdevel@...r.kernel.org" <linux-fsdevel@...r.kernel.org>,
	linux-man <linux-man@...r.kernel.org>, gamin-list@...me.org,
	lkml <linux-kernel@...r.kernel.org>,
	inotify-tools-general@...ts.sourceforge.net
Subject: Re: Things I wish I'd known about Inotify

Hi

On Fri, Apr 4, 2014 at 3:00 PM, David Herrmann <dh.herrmann@...il.com> wrote:
> 1)
> IN_IGNORED is async and _immediate_ in case a file got deleted. So if
> you use watch-descriptors as keys for your objects, an _already_ used
> key might be returned by inotify_add_watch() if an IN_IGNORED is
> queued for the old watch (which implicitly destroys the watch). Once
> you read the IN_IGNORED from the queue, there is usually no way to
> know whether it's generated by the old watch or by the new. The
> man-page mentions this in:
>   "IN_IGNORED:  Watch was removed explicitly (inotify_rm_watch(2)) or
>    automatically (file was deleted, or filesystem was unmounted)."
> I think we should add a note to BUGS that mentions this race (which is
> really not obvious from the description).
>
> This race could be fixed by requiring an explicit inotify_rm_watch()
> if an IN_IGNORED was generated asynchronously.
>
> 2)
> inotify_add_watch() is based on inodes. So if you call it on
> hardlinks, you will modify the existing watch instead of creating a
> new one. This is often really annoying and I think an IN_FORCE_NEW
> flag that disables this would be really nice. Imagine the following
> code:
>
> wd1 = inotify_add_watch(fd, A);
> wd2 = inotify_add_watch(fd, B);
> ...
> inotify_rm_watch(fd, wd2);
> wd3 = inotify_add_watch(fd, C);
> ...
> inotify_rm_watch(fd, wd1);
> ...
> inotify_rm_watch(fd, wd3);
>
> If A and B are hardlinks to the same file, then wd1==wd2. Therefore,
> after wd2 was removed, we _might_ end up with wd3==wd1 and thus remove
> wd3 early (which obviously is not intended). So simple code like this
> doesn't work. You have to verify whether returned handles are
> duplicates or new. An IN_FORCE_NEW flag would really help here.

Note that both of these races rely on watch-descriptors being reused
after they were freed. Turns out, that was "fixed" about exactly 1
year ago in:

commit a66c04b4534f9b25e1241dff9a9d94dff9fd66f8
Author: Jeff Layton <jlayton@...hat.com>
Date:   Mon Apr 29 16:21:21 2013 -0700

    inotify: convert inotify_add_to_idr() to use idr_alloc_cyclic()

So in case that was never backported, only older kernels are affected.
In newer kernels, wd reuse is quite unlikely. The races are still
there, though.

Thanks
David
--
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/