Github/emacs
2
0
Fork 1
mirror of git://git.sv.gnu.org/emacs.git synced 2026-02-17 18:37:33 +00:00
Code Issues Projects Releases Packages Wiki Activity

Some inotify cleanup

Browse source 
This catches some problems with integer overflow and races
that I noticed in inotify.c after reviewing the changes
installed to fix Bug#26126.
* src/fns.c, src/lisp.h (equal_no_quit): Now extern.
* src/inotify.c (aspect_to_inotifymask):
Check for cycles and for improper lists.
(make_lispy_mask, lispy_mask_match_p): Remove.
All callers changed to use INTEGER_TO_CONS and CONS_TO_INTEGER.
(inotifyevent_to_event, add_watch):
Don’t assume watch descriptors and cookies fit in fixnums.
(add_watch): Use assoc_no_quit, not Fassoc.
Avoid integer overflow in (very!) long-running processes where
the Emacs watch ID could overflow.  Avoid some duplicate code.
(find_descriptor): New function.
(remove_descriptor): First arg is now the returned value from
find_descriptor, rather than the descriptor.  This way, the
value can be removed without calling Fdelete, which might quit.
Wait until the end (when watch_list is consistent) before signaling
any errors.
(remove_watch, inotify_callback):
Use find_descriptor to avoid the need for Fdelete.
(inotify_callback): Use simpler tests for ioctl failure.
Free temporary buffer if signaled, and put it on the stack if small.
Use ssize_t to index through read results, to avoid a cast.
(valid_watch_descriptor): New function, with a tighter check.
(Finotify_rm_watch, Finotify_valid_p): Use it.
(Finotify_valid_p): Use assoc_no_quit and ass_no_quit instead
of Fassoc.  Do not assume the first assoc succeeds.
* test/src/inotify-tests.el (inotify-valid-p-simple):
Add inotify-valid-p tests, some of which dump core without
the fixes noted above.
This commit is contained in:
Paul Eggert 2017-03-30 11:08:23 -07:00
parent 1be3330b31
commit 6ff870218d
4 changed files with 150 additions and 115 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

3
src/fns.c
View file

@ -38,7 +38,6 @@ along with GNU Emacs. If not, see <http://www.gnu.org/licenses/>. */
static void sort_vector_copy (Lisp_Object, ptrdiff_t,
Lisp_Object *restrict, Lisp_Object *restrict);
static bool equal_no_quit (Lisp_Object, Lisp_Object);
enum equal_kind { EQUAL_NO_QUIT, EQUAL_PLAIN, EQUAL_INCLUDING_PROPERTIES };
static bool internal_equal (Lisp_Object, Lisp_Object,
enum equal_kind, int, Lisp_Object);
@ -2121,7 +2120,7 @@ of strings. (`equal' ignores text properties.) */)
Use this only on arguments that are cycle-free and not too large and
are not window configurations. */
static bool
bool
equal_no_quit (Lisp_Object o1, Lisp_Object o2)
{
return internal_equal (o1, o2, EQUAL_NO_QUIT, 0, Qnil);

252
src/inotify.c
View file

@ -41,16 +41,16 @@ along with GNU Emacs. If not, see <http://www.gnu.org/licenses/>. */
#ifndef IN_ONLYDIR
# define IN_ONLYDIR 0
#endif
#define INOTIFY_DEFAULT_MASK (IN_ALL_EVENTS|IN_EXCL_UNLINK)
#define INOTIFY_DEFAULT_MASK (IN_ALL_EVENTS | IN_EXCL_UNLINK)
/* File handle for inotify. */
static int inotifyfd = -1;
/* Alist of files being watched. We want the returned descriptor to
be unique for every watch, but inotify returns the same descriptor
for multiple calls to inotify_add_watch with the same file. In
order to solve this problem, we add a ID, uniquely identifying a
watch/file combination.
WD for multiple calls to inotify_add_watch with the same file.
Supply a nonnegative integer ID, so that WD and ID together
uniquely identify a watch/file combination.
For the same reason, we also need to store the watch's mask and we
can't allow the following flags to be used.
@ -60,12 +60,21 @@ static int inotifyfd = -1;
IN_ONESHOT
IN_ONLYDIR
Format: (descriptor . ((id filename callback mask) ...))
*/
Each element of this list is of the form (DESCRIPTOR . WATCHES)
where no two DESCRIPTOR values are the same. DESCRIPTOR represents
the inotify watch descriptor and WATCHES is a list with elements of
the form (ID FILENAME CALLBACK MASK), where ID is the integer
described above, FILENAME names the file being watched, CALLBACK is
invoked when the event occurs, and MASK represents the aspects
being watched. The WATCHES list is sorted by ID. Although
DESCRIPTOR and MASK are ordinarily integers, they are conses when
representing integers outside of fixnum range. */
static Lisp_Object watch_list;
static Lisp_Object
mask_to_aspects (uint32_t mask) {
mask_to_aspects (uint32_t mask)
{
Lisp_Object aspects = Qnil;
if (mask & IN_ACCESS)
aspects = Fcons (Qaccess, aspects);
@ -149,41 +158,27 @@ symbol_to_inotifymask (Lisp_Object symb)
static uint32_t
aspect_to_inotifymask (Lisp_Object aspect)
{
if (CONSP (aspect))
if (CONSP (aspect) || NILP (aspect))
{
Lisp_Object x = aspect;
uint32_t mask = 0;
while (CONSP (x))
{
mask |= symbol_to_inotifymask (XCAR (x));
x = XCDR (x);
}
FOR_EACH_TAIL (x)
mask |= symbol_to_inotifymask (XCAR (x));
CHECK_LIST_END (x, aspect);
return mask;
}
else
return symbol_to_inotifymask (aspect);
}
static Lisp_Object
make_lispy_mask (uint32_t mask)
{
return Fcons (make_number (mask & 0xffff),
make_number (mask >> 16));
}
static bool
lispy_mask_match_p (Lisp_Object mask, uint32_t other)
{
return (XINT (XCAR (mask)) & other)
|| ((XINT (XCDR (mask)) << 16) & other);
}
static Lisp_Object
inotifyevent_to_event (Lisp_Object watch, struct inotify_event const *ev)
{
Lisp_Object name = Qnil;
Lisp_Object name;
uint32_t mask;
CONS_TO_INTEGER (Fnth (make_number (3), watch), uint32_t, mask);
if (! lispy_mask_match_p (Fnth (make_number (3), watch), ev->mask))
if (! (mask & ev->mask))
return Qnil;
if (ev->len > 0)
@ -195,10 +190,10 @@ inotifyevent_to_event (Lisp_Object watch, struct inotify_event const *ev)
else
name = XCAR (XCDR (watch));
return list2 (list4 (Fcons (make_number (ev->wd), XCAR (watch)),
return list2 (list4 (Fcons (INTEGER_TO_CONS (ev->wd), XCAR (watch)),
mask_to_aspects (ev->mask),
name,
make_number (ev->cookie)),
INTEGER_TO_CONS (ev->cookie)),
Fnth (make_number (2), watch));
}
@ -209,55 +204,88 @@ static Lisp_Object
add_watch (int wd, Lisp_Object filename,
Lisp_Object aspect, Lisp_Object callback)
{
Lisp_Object descriptor = make_number (wd);
Lisp_Object elt = Fassoc (descriptor, watch_list);
Lisp_Object watches = Fcdr (elt);
Lisp_Object descriptor = INTEGER_TO_CONS (wd);
Lisp_Object tail = assoc_no_quit (descriptor, watch_list);
Lisp_Object watch, watch_id;
Lisp_Object mask = make_lispy_mask (aspect_to_inotifymask (aspect));
uint32_t imask = aspect_to_inotifymask (aspect);
Lisp_Object mask = INTEGER_TO_CONS (imask);
int id = 0;
while (! NILP (watches))
EMACS_INT id = 0;
if (NILP (tail))
{
id = max (id, 1 + XINT (XCAR (XCAR (watches))));
watches = XCDR (watches);
tail = list1 (descriptor);
watch_list = Fcons (tail, watch_list);
}
else
{
/* Assign a watch ID that is not already in use, by looking
for a gap in the existing sorted list. */
for (; ! NILP (XCDR (tail)); tail = XCDR (tail), id++)
if (!EQ (XCAR (XCAR (XCDR (tail))), make_number (id)))
break;
if (MOST_POSITIVE_FIXNUM < id)
emacs_abort ();
}
watch_id = make_number (id);
watch = list4 (watch_id, filename, callback, mask);
if (NILP (elt))
watch_list = Fcons (Fcons (descriptor, Fcons (watch, Qnil)),
watch_list);
else
XSETCDR (elt, Fcons (watch, XCDR (elt)));
XSETCDR (tail, Fcons (watch, XCDR (tail)));
return Fcons (descriptor, watch_id);
}
/* Remove all watches associated with descriptor. If INVALID_P is
true, the descriptor is already invalid, i.e. it received a
IN_IGNORED event. In this case skip calling inotify_rm_watch. */
static void
remove_descriptor (Lisp_Object descriptor, bool invalid_p)
/* Find the watch list element (if any) matching DESCRIPTOR. Return
nil if not found. If found, return t if the first element matches
DESCRIPTOR; otherwise, return the cons whose cdr matches
DESCRIPTOR. This lets the caller easily remove the element
matching DESCRIPTOR without having to search for it again, and
without calling Fdelete (which might quit). */
static Lisp_Object
find_descriptor (Lisp_Object descriptor)
{
Lisp_Object elt = Fassoc (descriptor, watch_list);
Lisp_Object tail, prevtail = Qt;
for (tail = watch_list; !NILP (tail); prevtail = tail, tail = XCDR (tail))
if (equal_no_quit (XCAR (XCAR (tail)), descriptor))
return prevtail;
return Qnil;
}
if (! NILP (elt))
/* Remove all watches associated with the watch list element after
PREVTAIL, or after the first element if PREVTAIL is t. If INVALID_P
is true, the descriptor is already invalid, i.e., it received a
IN_IGNORED event. In this case skip calling inotify_rm_watch. */
static void
remove_descriptor (Lisp_Object prevtail, bool invalid_p)
{
Lisp_Object tail = CONSP (prevtail) ? XCDR (prevtail) : watch_list;
int inotify_errno = 0;
if (! invalid_p)
{
int wd = XINT (descriptor);
watch_list = Fdelete (elt, watch_list);
if (! invalid_p)
if (inotify_rm_watch (inotifyfd, wd) == -1)
report_file_notify_error ("Could not rm watch", descriptor);
int wd;
CONS_TO_INTEGER (XCAR (XCAR (tail)), int, wd);
if (inotify_rm_watch (inotifyfd, wd) != 0)
inotify_errno = errno;
}
/* Cleanup if no more files are watched. */
if (NILP (watch_list))
if (CONSP (prevtail))
XSETCDR (prevtail, XCDR (tail));
else
{
emacs_close (inotifyfd);
delete_read_fd (inotifyfd);
inotifyfd = -1;
watch_list = XCDR (tail);
if (NILP (watch_list))
{
delete_read_fd (inotifyfd);
emacs_close (inotifyfd);
inotifyfd = -1;
}
}
if (inotify_errno != 0)
{
errno = inotify_errno;
report_file_notify_error ("Could not rm watch", XCAR (tail));
}
}
@ -265,19 +293,19 @@ remove_descriptor (Lisp_Object descriptor, bool invalid_p)
static void
remove_watch (Lisp_Object descriptor, Lisp_Object id)
{
Lisp_Object elt = Fassoc (descriptor, watch_list);
Lisp_Object prevtail = find_descriptor (descriptor);
if (NILP (prevtail))
return;
if (! NILP (elt))
{
Lisp_Object watch = Fassoc (id, XCDR (elt));
if (! NILP (watch))
XSETCDR (elt, Fdelete (watch, XCDR (elt)));
/* Remove the descriptor if noone is watching it. */
if (NILP (XCDR (elt)))
remove_descriptor (descriptor, false);
}
Lisp_Object elt = XCAR (CONSP (prevtail) ? XCDR (prevtail) : watch_list);
for (Lisp_Object prev = elt; !NILP (XCDR (prev)); prev = XCDR (prev))
if (EQ (id, XCAR (XCAR (XCDR (prev)))))
{
XSETCDR (prev, XCDR (XCDR (prev)));
if (NILP (XCDR (elt)))
remove_descriptor (prevtail, false);
break;
}
}
/* This callback is called when the FD is available for read. The inotify
@ -285,52 +313,44 @@ remove_watch (Lisp_Object descriptor, Lisp_Object id)
static void
inotify_callback (int fd, void *_)
{
struct input_event event;
int to_read;
char *buffer;
ssize_t n;
size_t i;
to_read = 0;
if (ioctl (fd, FIONREAD, &to_read) == -1)
if (ioctl (fd, FIONREAD, &to_read) < 0)
report_file_notify_error ("Error while retrieving file system events",
Qnil);
buffer = xmalloc (to_read);
n = read (fd, buffer, to_read);
USE_SAFE_ALLOCA;
char *buffer = SAFE_ALLOCA (to_read);
ssize_t n = read (fd, buffer, to_read);
if (n < 0)
{
xfree (buffer);
report_file_notify_error ("Error while reading file system events", Qnil);
}
report_file_notify_error ("Error while reading file system events", Qnil);
struct input_event event;
EVENT_INIT (event);
event.kind = FILE_NOTIFY_EVENT;
i = 0;
while (i < (size_t)n)
for (ssize_t i = 0; i < n; )
{
struct inotify_event *ev = (struct inotify_event *) &buffer[i];
Lisp_Object descriptor = make_number (ev->wd);
Lisp_Object elt = Fassoc (descriptor, watch_list);
Lisp_Object descriptor = INTEGER_TO_CONS (ev->wd);
Lisp_Object prevtail = find_descriptor (descriptor);
if (! NILP (elt))
if (! NILP (prevtail))
{
Lisp_Object watches = XCDR (elt);
while (! NILP (watches))
Lisp_Object tail = CONSP (prevtail) ? XCDR (prevtail) : watch_list;
for (Lisp_Object watches = XCDR (XCAR (tail)); ! NILP (watches);
watches = XCDR (watches))
{
event.arg = inotifyevent_to_event (XCAR (watches), ev);
if (!NILP (event.arg))
kbd_buffer_store_event (&event);
watches = XCDR (watches);
}
/* If event was removed automatically: Drop it from watch list. */
if (ev->mask & IN_IGNORED)
remove_descriptor (descriptor, true);
remove_descriptor (prevtail, true);
}
i += sizeof (*ev) + ev->len;
}
xfree (buffer);
SAFE_FREE ();
}
DEFUN ("inotify-add-watch", Finotify_add_watch, Sinotify_add_watch, 3, 3, 0,
@ -407,7 +427,7 @@ IN_ONLYDIR */)
if (inotifyfd < 0)
{
inotifyfd = inotify_init1 (IN_NONBLOCK|IN_CLOEXEC);
inotifyfd = inotify_init1 (IN_NONBLOCK | IN_CLOEXEC);
if (inotifyfd < 0)
report_file_notify_error ("File watching is not available", Qnil);
watch_list = Qnil;
@ -416,12 +436,24 @@ IN_ONLYDIR */)
encoded_file_name = ENCODE_FILE (filename);
wd = inotify_add_watch (inotifyfd, SSDATA (encoded_file_name), mask);
if (wd == -1)
if (wd < 0)
report_file_notify_error ("Could not add watch for file", filename);
return add_watch (wd, filename, aspect, callback);
}
static bool
valid_watch_descriptor (Lisp_Object wd)
{
return (CONSP (wd)
&& (RANGED_INTEGERP (0, XCAR (wd), INT_MAX)
|| (CONSP (XCAR (wd))
&& RANGED_INTEGERP ((MOST_POSITIVE_FIXNUM >> 16) + 1,
XCAR (XCAR (wd)), INT_MAX >> 16)
&& RANGED_INTEGERP (0, XCDR (XCAR (wd)), (1 << 16) - 1)))
&& NATNUMP (XCDR (wd)));
}
DEFUN ("inotify-rm-watch", Finotify_rm_watch, Sinotify_rm_watch, 1, 1, 0,
doc: /* Remove an existing WATCH-DESCRIPTOR.
@ -433,9 +465,7 @@ See inotify_rm_watch(2) for more information. */)
Lisp_Object descriptor, id;
if (! (CONSP (watch_descriptor)
&& INTEGERP (XCAR (watch_descriptor))
&& INTEGERP (XCDR (watch_descriptor))))
if (! valid_watch_descriptor (watch_descriptor))
report_file_notify_error ("Invalid descriptor ", watch_descriptor);
descriptor = XCAR (watch_descriptor);
@ -456,16 +486,12 @@ reason. Removing the watch by calling `inotify-rm-watch' also makes
it invalid. */)
(Lisp_Object watch_descriptor)
{
Lisp_Object elt, watch;
if (! (CONSP (watch_descriptor)
&& INTEGERP (XCAR (watch_descriptor))
&& INTEGERP (XCDR (watch_descriptor))))
if (! valid_watch_descriptor (watch_descriptor))
return Qnil;
elt = Fassoc (XCAR (watch_descriptor), watch_list);
watch = Fassoc (XCDR (watch_descriptor), XCDR (elt));
Lisp_Object tail = assoc_no_quit (XCAR (watch_descriptor), watch_list);
if (NILP (tail))
return Qnil;
Lisp_Object watch = assq_no_quit (XCDR (watch_descriptor), XCDR (tail));
return ! NILP (watch) ? Qt : Qnil;
}

1
src/lisp.h
View file

@ -3376,6 +3376,7 @@ extern Lisp_Object merge (Lisp_Object, Lisp_Object, Lisp_Object);
extern Lisp_Object do_yes_or_no_p (Lisp_Object);
extern Lisp_Object concat2 (Lisp_Object, Lisp_Object);
extern Lisp_Object concat3 (Lisp_Object, Lisp_Object, Lisp_Object);
extern bool equal_no_quit (Lisp_Object, Lisp_Object);
extern Lisp_Object nconc2 (Lisp_Object, Lisp_Object);
extern Lisp_Object assq_no_quit (Lisp_Object, Lisp_Object);
extern Lisp_Object assoc_no_quit (Lisp_Object, Lisp_Object);

9
test/src/inotify-tests.el
View file

@ -28,6 +28,13 @@
(declare-function inotify-add-watch "inotify.c" (file-name aspect callback))
(declare-function inotify-rm-watch "inotify.c" (watch-descriptor))
(ert-deftest inotify-valid-p-simple ()
"Simple tests for `inotify-valid-p'."
(skip-unless (featurep 'inotify))
(should-not (inotify-valid-p 0))
(should-not (inotify-valid-p nil))
(should-not (inotify-valid-p '(0 . 0))))
;; (ert-deftest filewatch-file-watch-aspects-check ()
;; "Test whether `file-watch' properly checks the aspects."
;; (let ((temp-file (make-temp-file "filewatch-aspects")))
@ -56,7 +63,9 @@
(insert "Foo\n"))
(read-event nil nil 5)
(should (> events 0)))
(should (inotify-valid-p wd))
(inotify-rm-watch wd)
(should-not (inotify-valid-p wd))
(delete-file temp-file)))))
(provide 'inotify-tests)