Skip site navigation (1)Skip section navigation (2)
Date:      Mon, 12 May 2025 14:08:47 GMT
From:      Mark Johnston <markj@FreeBSD.org>
To:        src-committers@FreeBSD.org, dev-commits-src-all@FreeBSD.org, dev-commits-src-main@FreeBSD.org
Subject:   git: eed3be47967f - main - linuxkpi: Fix up jiffies handling
Message-ID:  <202505121408.54CE8l8o024492@gitrepo.freebsd.org>

next in thread | raw e-mail | index | archive | help
The branch main has been updated by markj:

URL: https://cgit.FreeBSD.org/src/commit/?id=eed3be47967f0ac8268d1655f57bc302c123e991

commit eed3be47967f0ac8268d1655f57bc302c123e991
Author:     Mark Johnston <markj@FreeBSD.org>
AuthorDate: 2025-05-12 13:39:27 +0000
Commit:     Mark Johnston <markj@FreeBSD.org>
CommitDate: 2025-05-12 14:08:39 +0000

    linuxkpi: Fix up jiffies handling
    
    A few issues found by code inspection while hunting for bugzilla PR
    286512:
    - The "expires" field in struct delayed_work should be unsigned.
    - In linux_timer_jiffies_until(), clamp the return value to INT_MAX:
      this return value is used as a ticks count, not a jiffies count, so we
      should avoid returning too large a value, lest it get truncated.  It's
      unlikely we are dealing with values that large, but we should be
      careful anyway.
    - In linux_add_to_sleepqueue(), truncate the timeout to INT_MAX, as this
      value is passed to sleepq_set_timeout() as a ticks value.  Typically
      it's multiplied by ticks_sbt to get an sbintime, and we should make
      sure the multiplication doesn't overflow.  In drm-kmod, there is at
      least one call mod_delayed_work(... MAX_SCHEDULE_TIMEOUT).
    
    Fixes:          325aa4dbd10d ("linuxkpi: Introduce a properly typed jiffies")
    Reviewed by:    olce, bz, dumbbell, kib
    Tested by:      dumbbell, bz
    Differential Revision:  https://reviews.freebsd.org/D50192
---
 sys/compat/linuxkpi/common/include/linux/jiffies.h | 11 -----
 .../linuxkpi/common/include/linux/workqueue.h      |  2 +-
 sys/compat/linuxkpi/common/src/linux_compat.c      | 16 +++++++
 sys/compat/linuxkpi/common/src/linux_schedule.c    | 51 +++++++++++-----------
 sys/compat/linuxkpi/common/src/linux_work.c        |  7 +++
 5 files changed, 49 insertions(+), 38 deletions(-)

diff --git a/sys/compat/linuxkpi/common/include/linux/jiffies.h b/sys/compat/linuxkpi/common/include/linux/jiffies.h
index df6ca129b37c..c2409726e874 100644
--- a/sys/compat/linuxkpi/common/include/linux/jiffies.h
+++ b/sys/compat/linuxkpi/common/include/linux/jiffies.h
@@ -140,15 +140,4 @@ get_jiffies_64(void)
 	return ((uint64_t)jiffies);
 }
 
-static inline unsigned long
-linux_timer_jiffies_until(unsigned long expires)
-{
-	unsigned long delta = expires - jiffies;
-
-	/* guard against already expired values */
-	if ((long)delta < 1)
-		delta = 1;
-	return (delta);
-}
-
 #endif	/* _LINUXKPI_LINUX_JIFFIES_H_ */
diff --git a/sys/compat/linuxkpi/common/include/linux/workqueue.h b/sys/compat/linuxkpi/common/include/linux/workqueue.h
index 25ee861d3015..66d3981d4229 100644
--- a/sys/compat/linuxkpi/common/include/linux/workqueue.h
+++ b/sys/compat/linuxkpi/common/include/linux/workqueue.h
@@ -90,7 +90,7 @@ struct delayed_work {
 	struct {
 		struct callout callout;
 		struct mtx mtx;
-		long	expires;
+		unsigned long expires;
 	} timer;
 };
 
diff --git a/sys/compat/linuxkpi/common/src/linux_compat.c b/sys/compat/linuxkpi/common/src/linux_compat.c
index 019e08f59d44..ae15fc553ecd 100644
--- a/sys/compat/linuxkpi/common/src/linux_compat.c
+++ b/sys/compat/linuxkpi/common/src/linux_compat.c
@@ -2071,6 +2071,22 @@ linux_timer_callback_wrapper(void *context)
 	timer->function(timer->data);
 }
 
+static int
+linux_timer_jiffies_until(unsigned long expires)
+{
+	unsigned long delta = expires - jiffies;
+
+	/*
+	 * Guard against already expired values and make sure that the value can
+	 * be used as a tick count, rather than a jiffies count.
+	 */
+	if ((long)delta < 1)
+		delta = 1;
+	else if (delta > INT_MAX)
+		delta = INT_MAX;
+	return ((int)delta);
+}
+
 int
 mod_timer(struct timer_list *timer, unsigned long expires)
 {
diff --git a/sys/compat/linuxkpi/common/src/linux_schedule.c b/sys/compat/linuxkpi/common/src/linux_schedule.c
index 507d6fc417d0..3f3605096d62 100644
--- a/sys/compat/linuxkpi/common/src/linux_schedule.c
+++ b/sys/compat/linuxkpi/common/src/linux_schedule.c
@@ -38,29 +38,47 @@
 #include <linux/spinlock.h>
 #include <linux/wait.h>
 
+/*
+ * Convert a relative time in jiffies to a tick count, suitable for use with
+ * native FreeBSD interfaces (callouts, sleepqueues, etc.).
+ */
+static int
+linux_jiffies_timeout_to_ticks(long timeout)
+{
+	if (timeout < 1)
+		return (1);
+	else if (timeout == MAX_SCHEDULE_TIMEOUT)
+		return (0);
+	else if (timeout > INT_MAX)
+		return (INT_MAX);
+	else
+		return (timeout);
+}
+
 static int
 linux_add_to_sleepqueue(void *wchan, struct task_struct *task,
     const char *wmesg, long timeout, int state)
 {
-	int flags, ret;
+	int flags, ret, stimeout;
 
 	MPASS((state & ~(TASK_PARKED | TASK_NORMAL)) == 0);
 
 	flags = SLEEPQ_SLEEP | ((state & TASK_INTERRUPTIBLE) != 0 ?
 	    SLEEPQ_INTERRUPTIBLE : 0);
+	stimeout = linux_jiffies_timeout_to_ticks(timeout);
 
 	sleepq_add(wchan, NULL, wmesg, flags, 0);
-	if (timeout != 0)
-		sleepq_set_timeout(wchan, timeout);
+	if (stimeout != 0)
+		sleepq_set_timeout(wchan, stimeout);
 
 	DROP_GIANT();
 	if ((state & TASK_INTERRUPTIBLE) != 0) {
-		if (timeout == 0)
+		if (stimeout == 0)
 			ret = -sleepq_wait_sig(wchan, 0);
 		else
 			ret = -sleepq_timedwait_sig(wchan, 0);
 	} else {
-		if (timeout == 0) {
+		if (stimeout == 0) {
 			sleepq_wait(wchan, 0);
 			ret = 0;
 		} else
@@ -258,12 +276,6 @@ linux_wait_event_common(wait_queue_head_t *wqh, wait_queue_t *wq, long timeout,
 	if (lock != NULL)
 		spin_unlock_irq(lock);
 
-	/* range check timeout */
-	if (timeout < 1)
-		timeout = 1;
-	else if (timeout == MAX_SCHEDULE_TIMEOUT)
-		timeout = 0;
-
 	task = current;
 
 	sleepq_lock(task);
@@ -285,17 +297,10 @@ linux_schedule_timeout(long timeout)
 {
 	struct task_struct *task;
 	long remainder;
-	int ret;
-	int state;
+	int ret, state;
 
 	task = current;
 
-	/* range check timeout */
-	if (timeout < 1)
-		timeout = 1;
-	else if (timeout == MAX_SCHEDULE_TIMEOUT)
-		timeout = 0;
-
 	remainder = jiffies + timeout;
 
 	sleepq_lock(task);
@@ -309,7 +314,7 @@ linux_schedule_timeout(long timeout)
 	}
 	set_task_state(task, TASK_RUNNING);
 
-	if (timeout == 0)
+	if (timeout == MAX_SCHEDULE_TIMEOUT)
 		return (MAX_SCHEDULE_TIMEOUT);
 
 	/* range check return value */
@@ -350,12 +355,6 @@ linux_wait_on_bit_timeout(unsigned long *word, int bit, unsigned int state,
 	void *wchan;
 	int ret;
 
-	/* range check timeout */
-	if (timeout < 1)
-		timeout = 1;
-	else if (timeout == MAX_SCHEDULE_TIMEOUT)
-		timeout = 0;
-
 	task = current;
 	wchan = bit_to_wchan(word, bit);
 	for (;;) {
diff --git a/sys/compat/linuxkpi/common/src/linux_work.c b/sys/compat/linuxkpi/common/src/linux_work.c
index cf15d1a9c41b..b1975d16025e 100644
--- a/sys/compat/linuxkpi/common/src/linux_work.c
+++ b/sys/compat/linuxkpi/common/src/linux_work.c
@@ -226,6 +226,13 @@ linux_queue_delayed_work_on(int cpu, struct workqueue_struct *wq,
 	if (atomic_read(&wq->draining) != 0)
 		return (!work_pending(&dwork->work));
 
+	/*
+	 * Clamp the delay to a valid ticks value, some consumers pass
+	 * MAX_SCHEDULE_TIMEOUT.
+	 */
+	if (delay > INT_MAX)
+		delay = INT_MAX;
+
 	mtx_lock(&dwork->timer.mtx);
 	switch (linux_update_state(&dwork->work.state, states)) {
 	case WORK_ST_EXEC:



Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?202505121408.54CE8l8o024492>