ext4: Prevent an infinite loop in the lazyinit thread.

Hello,

Under the following conditions, the lazyinit thread can reschedule itself indefinitely without doing anything, consuming a large amount of the system resources:

In the ext4_run_li_request function, a start_time timestamp is taken. Right before elr->lr_timeout is computed, in the same function, the system clock is updated in userspace, from the Unix Epoch to the current time. The elr->lr_timeout takes a large value. The elr->lr_next_sched is then set to a value far away in the future.

/*
* Away from jiffies because of a time jump when computing
* elr->lr_timeout.
*/
elr->lr_next_sched = jiffies + elr->lr_timeout;

Back, in the ext4_lazyinit_thread that called the ext4_run_li_request, the following condition can be false:

// elr->lr_next_sched > next_wakeup
if (time_before(elr->lr_next_sched, next_wakeup))

        next_wakeup = elr->lr_next_sched;

so that next_wakeup is not updated. Assuming that next_wakeup was not updated above and still has the MAX_JIFFY_OFFSET value, the following condition will be true:

// next_wakeup == MAX_JIFFY_OFFSET
if ((time_after_eq(cur, next_wakeup)) ||

    (MAX_JIFFY_OFFSET == next_wakeup)) {
	cond_resched();
	continue;
}

causing us to process the li_request_list again. If we now have jiffies < elr->lr_next_sched, as we have already elr->lr_next_sched > next_wakeup, we will just continue without updating next_wakeup,

// jiffies < elr->lr_next_sched && elr->lr_next_sched > next_wakeup

if (time_before(jiffies, elr->lr_next_sched)) {
	if (time_before(elr->lr_next_sched, next_wakeup))
		next_wakeup = elr->lr_next_sched;
	continue;
}

and again, we will call cond_resched because next_wakeup is not updated, and we now have an infinite loop.

This was put into evidence with the following values:

jiffies = 4294938821
elr->lr_next_sched = 1966790060
next_wakeup = 1073741822 (MAX_JIFFY_OFFSET)

on an armv7 (32 bits) system, without an RTC, while updating the system clock during the lazyinit thread is working.

Fix that by using ktime_get_ns insted of ktime_get_real_ns and by using a boolean instead of MAX_JIFFY_OFFSET to determine whether the next_wakeup value has been set.

Thanks,

Mathieu

Mathieu Othacehe (1):
ext4: Prevent an infinite loop in the lazyinit thread.

 fs/ext4/super.c | 20 +++++++++++++-------
 1 file changed, 13 insertions(+), 7 deletions(-)

2.46.0

Use ktime_get_ns instead of ktime_get_real_ns when computing the lr_timeout not to be affected by system time jumps.

Use a boolean instead of the MAX_JIFFY_OFFSET value to determine whether the next_wakeup value has been set. Comparing elr->lr_next_sched to MAX_JIFFY_OFFSET can cause the lazyinit thread to loop indefinitely.

---
 fs/ext4/super.c | 20 +++++++++++++-------
 1 file changed, 13 insertions(+), 7 deletions(-)

index 9fcf44064c6a6..b4839ccd83ad5 100644
@@ -3712,12 +3712,12 @@ static int ext4_run_li_request(struct ext4_li_request *elr)
 		ret = 1;
 
 	if (!ret) {
-		start_time = ktime_get_real_ns();
+		start_time = ktime_get_ns();
 		ret = ext4_init_inode_table(sb, group,
 					    elr->lr_timeout ? 0 : 1);
 		trace_ext4_lazy_itable_init(sb, group);
 		if (elr->lr_timeout == 0) {
-			elr->lr_timeout = nsecs_to_jiffies((ktime_get_real_ns() - start_time) *
+			elr->lr_timeout = nsecs_to_jiffies((ktime_get_ns() - start_time) *
 				EXT4_SB(elr->lr_super)->s_li_wait_mult);
 		}
 		elr->lr_next_sched = jiffies + elr->lr_timeout;

@@ -3777,8 +3777,9 @@ static int ext4_lazyinit_thread(void *arg)
 
 cont_thread:
 	while (true) {
-		next_wakeup = MAX_JIFFY_OFFSET;
+		bool next_wakeup_initialized = false;
 
+		next_wakeup = 0;
 		mutex_lock(&eli->li_list_mtx);
 		if (list_empty(&eli->li_request_list)) {
 			mutex_unlock(&eli->li_list_mtx);

@@ -3791,8 +3792,11 @@ static int ext4_lazyinit_thread(void *arg)
 					 lr_request);
 
 			if (time_before(jiffies, elr->lr_next_sched)) {
-				if (time_before(elr->lr_next_sched, next_wakeup))
+				if (!next_wakeup_initialized ||
+				    time_before(elr->lr_next_sched, next_wakeup)) {
 					next_wakeup = elr->lr_next_sched;
+					next_wakeup_initialized = true;
+				}
 				continue;
 			}
 			if (down_read_trylock(&elr->lr_super->s_umount)) {

@@ -3820,16 +3824,18 @@ static int ext4_lazyinit_thread(void *arg)
 				elr->lr_next_sched = jiffies +
 					get_random_u32_below(EXT4_DEF_LI_MAX_START_DELAY * HZ);
 			}
-			if (time_before(elr->lr_next_sched, next_wakeup))
+			if (!next_wakeup_initialized ||
+			    time_before(elr->lr_next_sched, next_wakeup)) {
 				next_wakeup = elr->lr_next_sched;
+				next_wakeup_initialized = true;
+			}
 		}
 		mutex_unlock(&eli->li_list_mtx);
 
 		try_to_freeze();
 
 		cur = jiffies;
-		if ((time_after_eq(cur, next_wakeup)) ||
-		    (MAX_JIFFY_OFFSET == next_wakeup)) {
+		if (!next_wakeup_initialized || time_after_eq(cur, next_wakeup)) {
 			cond_resched();
 			continue;
 		}

2.46.0

Nice catch! The patch looks good so feel free to add:

But the analysis you've done in patch 0/1 would ideally be in the changelog of this patch so that we can easily get back to it in the future in git logs. Maybe Ted can handle that on commit?

Honza

Jan Kara <[email protected]>
SUSE Labs, CR

Applied, thanks!

[1/1] ext4: Prevent an infinite loop in the lazyinit thread.

      commit: e06a8c24f6445c2f1b5255caa4f63b38e31c43fa

Best regards,

Theodore Ts'o <[email protected]>