From ae181b0ff1226cf38be8a7f03ff19bf869c87f54 Mon Sep 17 00:00:00 2001 From: Takashi Yano Date: Sun, 20 Oct 2024 01:54:00 +0900 Subject: [PATCH] Cygwin: lockf: Make lockf() return ENOLCK when too many locks Previously, lockf() printed a warning message when the number of locks per file exceeds the limit (MAX_LOCKF_CNT). This patch makes lockf() return ENOLCK in that case rather than printing the warning message. Addresses: https://cygwin.com/pipermail/cygwin/2024-October/256528.html Fixes: 31390e4ca643 ("(inode_t::get_all_locks_list): Use pre-allocated buffer in i_all_lf instead of allocating every lock. Return pointer to start of linked list of locks.") Reported-by: Christian Franke Reviewed-by: Corinna Vinschen Signed-off-by: Takashi Yano --- winsup/cygwin/flock.cc | 82 +++++++++++++++++++++++++++++++++---- winsup/cygwin/release/3.5.5 | 4 ++ 2 files changed, 78 insertions(+), 8 deletions(-) diff --git a/winsup/cygwin/flock.cc b/winsup/cygwin/flock.cc index 5550b3a5b..3821bddd6 100644 --- a/winsup/cygwin/flock.cc +++ b/winsup/cygwin/flock.cc @@ -297,6 +297,7 @@ class inode_t HANDLE i_dir; HANDLE i_mtx; uint32_t i_cnt; /* # of threads referencing this instance. */ + uint32_t i_lock_cnt; /* # of locks for this file */ public: inode_t (dev_t dev, ino_t ino); @@ -321,6 +322,8 @@ class inode_t void unlock_and_remove_if_unused (); lockf_t *get_all_locks_list (); + uint32_t get_lock_count () /* needs get_all_locks_list() */ + { return i_lock_cnt; } bool del_my_locks (long long id, HANDLE fhdl); }; @@ -503,7 +506,8 @@ inode_t::get (dev_t dev, ino_t ino, bool create_if_missing, bool lock) } inode_t::inode_t (dev_t dev, ino_t ino) -: i_lockf (NULL), i_all_lf (NULL), i_dev (dev), i_ino (ino), i_cnt (0L) +: i_lockf (NULL), i_all_lf (NULL), i_dev (dev), i_ino (ino), i_cnt (0L), + i_lock_cnt (0) { HANDLE parent_dir; WCHAR name[48]; @@ -610,17 +614,16 @@ inode_t::get_all_locks_list () dbi->ObjectName.Buffer[LOCK_OBJ_NAME_LEN] = L'\0'; if (!newlock.from_obj_name (this, &i_all_lf, dbi->ObjectName.Buffer)) continue; - if (lock - i_all_lf >= MAX_LOCKF_CNT) - { - system_printf ("Warning, can't handle more than %d locks per file.", - MAX_LOCKF_CNT); - break; - } + /* This should not be happen. The number of locks is limitted + in lf_setlock() and lf_clearlock() so that it does not + exceed MAX_LOCKF_CNT. */ + assert (lock - i_all_lf < MAX_LOCKF_CNT); if (lock > i_all_lf) lock[-1].lf_next = lock; new (lock++) lockf_t (newlock); } } + i_lock_cnt = lock - i_all_lf; /* If no lock has been found, return NULL. */ if (lock == i_all_lf) return NULL; @@ -1190,6 +1193,12 @@ restart: /* Entry point after a restartable signal came in. */ return -1; } +/* The total number of locks shall not exceed MAX_LOCKF_CNT. + If once it exceeds, lf_fildoverlap() cannot work correctly. + Therefore, lf_setlock() and lf_clearlock() control the + total number of locks not to exceed MAX_LOCKF_CNT. When + they detect that the operation will cause excess, they + return ENOCLK. */ /* * Set a byte-range lock. */ @@ -1346,14 +1355,31 @@ lf_setlock (lockf_t *lock, inode_t *node, lockf_t **clean, HANDLE fhdl) * * Handle any locks that overlap. */ + node->get_all_locks_list (); /* Update lock count */ + uint32_t lock_cnt = node->get_lock_count (); + /* lf_clearlock() sometimes increases the number of locks. Without + this room, the unlocking will never succeed in some situation. */ + const uint32_t room_for_clearlock = 2; + const int incr[] = {1, 1, 2, 2, 3, 2}; + int decr = 0; + prev = head; block = *head; needtolink = 1; for (;;) { ovcase = lf_findoverlap (block, lock, SELF, &prev, &overlap); + /* Estimate the maximum increase in number of the locks that + can occur here. If this possibly exceeds the MAX_LOCKF_CNT, + return ENOLCK. */ if (ovcase) - block = overlap->lf_next; + { + block = overlap->lf_next; + HANDLE ov_obj = overlap->lf_obj; + decr = (ov_obj && get_obj_handle_count (ov_obj) == 1) ? 1 : 0; + } + if (needtolink) + lock_cnt += incr[ovcase] - decr; /* * Six cases: * 0) no overlap @@ -1368,6 +1394,8 @@ lf_setlock (lockf_t *lock, inode_t *node, lockf_t **clean, HANDLE fhdl) case 0: /* no overlap */ if (needtolink) { + if (lock_cnt > MAX_LOCKF_CNT - room_for_clearlock) + return ENOLCK; *prev = lock; lock->lf_next = overlap; lock->create_lock_obj (); @@ -1380,6 +1408,8 @@ lf_setlock (lockf_t *lock, inode_t *node, lockf_t **clean, HANDLE fhdl) * able to acquire it. * Cygwin: Always wake lock. */ + if (lock_cnt > MAX_LOCKF_CNT - room_for_clearlock) + return ENOLCK; lf_wakelock (overlap, fhdl); overlap->lf_type = lock->lf_type; overlap->create_lock_obj (); @@ -1397,6 +1427,11 @@ lf_setlock (lockf_t *lock, inode_t *node, lockf_t **clean, HANDLE fhdl) *clean = lock; break; } + if (overlap->lf_start < lock->lf_start + && overlap->lf_end > lock->lf_end) + lock_cnt++; + if (lock_cnt > MAX_LOCKF_CNT - room_for_clearlock) + return ENOLCK; if (overlap->lf_start == lock->lf_start) { *prev = lock; @@ -1413,6 +1448,8 @@ lf_setlock (lockf_t *lock, inode_t *node, lockf_t **clean, HANDLE fhdl) break; case 3: /* lock contains overlap */ + if (needtolink && lock_cnt > MAX_LOCKF_CNT - room_for_clearlock) + return ENOLCK; /* * If downgrading lock, others may be able to * acquire it, otherwise take the list. @@ -1440,6 +1477,8 @@ lf_setlock (lockf_t *lock, inode_t *node, lockf_t **clean, HANDLE fhdl) /* * Add lock after overlap on the list. */ + if (lock_cnt > MAX_LOCKF_CNT - room_for_clearlock) + return ENOLCK; lock->lf_next = overlap->lf_next; overlap->lf_next = lock; overlap->lf_end = lock->lf_start - 1; @@ -1456,6 +1495,8 @@ lf_setlock (lockf_t *lock, inode_t *node, lockf_t **clean, HANDLE fhdl) */ if (needtolink) { + if (lock_cnt > MAX_LOCKF_CNT - room_for_clearlock) + return ENOLCK; *prev = lock; lock->lf_next = overlap; lock->create_lock_obj (); @@ -1483,12 +1524,35 @@ lf_clearlock (lockf_t *unlock, lockf_t **clean, HANDLE fhdl) lockf_t *lf = *head; lockf_t *overlap, **prev; int ovcase; + inode_t *node = lf->lf_inode; + tmp_pathbuf tp; + node->i_all_lf = (lockf_t *) tp.w_get (); + node->get_all_locks_list (); /* Update lock count */ + uint32_t lock_cnt = node->get_lock_count (); + bool first_loop = true; if (lf == NOLOCKF) return 0; prev = head; while ((ovcase = lf_findoverlap (lf, unlock, SELF, &prev, &overlap))) { + /* Estimate the maximum increase in number of the locks that + can occur here. If this possibly exceeds the MAX_LOCKF_CNT, + return ENOLCK. */ + HANDLE ov_obj = overlap->lf_obj; + if (first_loop) + { + const int incr[] = {0, 0, 1, 1, 2, 1}; + int decr = (ov_obj && get_obj_handle_count (ov_obj) == 1) ? 1 : 0; + lock_cnt += incr[ovcase] - decr; + if (ovcase == 2 + && overlap->lf_start < unlock->lf_start + && overlap->lf_end > unlock->lf_end) + lock_cnt++; + if (lock_cnt > MAX_LOCKF_CNT) + return ENOLCK; + } + /* * Wakeup the list of locks to be retried. */ @@ -1521,6 +1585,7 @@ lf_clearlock (lockf_t *unlock, lockf_t **clean, HANDLE fhdl) lf = overlap->lf_next; overlap->lf_next = *clean; *clean = overlap; + first_loop = false; continue; case 4: /* overlap starts before lock */ @@ -1528,6 +1593,7 @@ lf_clearlock (lockf_t *unlock, lockf_t **clean, HANDLE fhdl) prev = &overlap->lf_next; lf = overlap->lf_next; overlap->create_lock_obj (); + first_loop = false; continue; case 5: /* overlap ends after lock */ diff --git a/winsup/cygwin/release/3.5.5 b/winsup/cygwin/release/3.5.5 index ca96edf04..2dba4aa8f 100644 --- a/winsup/cygwin/release/3.5.5 +++ b/winsup/cygwin/release/3.5.5 @@ -16,3 +16,7 @@ Fixes: - Fix lockf() error which occurs when adding a new lock over multiple locks. Addresses: https://cygwin.com/pipermail/cygwin/2024-October/256528.html + +- Make lockf() return ENOLCK when the number of locks exceeds + MAX_LOCKF_CNT rather than printing a warning message. + Addresses: https://cygwin.com/pipermail/cygwin/2024-October/256528.html