xorl %eax, %eax

Linux kernel EXT4 ext4_remove_li_request() Race Condition

leave a comment »

This was reported by Lukas Czerner of Red Hat and it was fixed in 2.6.39.1 release of the Linux kernel. Now if we have a look in fs/ext4/super.c file we will see the following routine.

static void ext4_unregister_li_request(struct super_block *sb)
{
        struct ext4_li_request *elr = EXT4_SB(sb)->s_li_request;

        if (!ext4_li_info)
                return;

        mutex_lock(&ext4_li_info->li_list_mtx);
        ext4_remove_li_request(elr);
        mutex_unlock(&ext4_li_info->li_list_mtx);
}

Here you can see that it retrieves a lazy initialization request (ext4_li_request) based on the provided super block parameter. Then it locks it using its MUTEX lock and removes it by calling ext4_remove_li_request() routine.

Although this appears to be fine, there is a race window between the retrieval of the ‘elr’ and the removal since the locking takes place only during the removal. During this small race window, there is a possibility of invoking ext4_lazyinit_thread() located in the same source code file which could free the same request as shown below.

/*
 * This is the function where ext4lazyinit thread lives. It walks
 * through the request list searching for next scheduled filesystem.
 * When such a fs is found, run the lazy initialization request
 * (ext4_rn_li_request) and keep track of the time spend in this
 * function. Based on that time we compute next schedule time of
 * the request. When walking through the list is complete, compute
 * next waking time and put itself into sleep.
 */
static int ext4_lazyinit_thread(void *arg)
{
        struct ext4_lazy_init *eli = (struct ext4_lazy_init *)arg;
        struct list_head *pos, *n;
        struct ext4_li_request *elr;
  ...
cont_thread:
        while (true) {
  ...
                list_for_each_safe(pos, n, &eli->li_request_list) {
                        elr = list_entry(pos, struct ext4_li_request,
                                         lr_request);

                        if (time_after_eq(jiffies, elr->lr_next_sched)) {
                                if (ext4_run_li_request(elr) != 0) {
                                        /* error, remove the lazy_init job */
                                        ext4_remove_li_request(elr);
                                        continue;
                                }
                        }

                        if (time_before(elr->lr_next_sched, next_wakeup))
                                next_wakeup = elr->lr_next_sched;
                }
  ...
        return 0;
}

This means that the second invocation will result in a double free. So, the fix was to obviously rearrange the ext4_unregister_li_request()’s code to add the missing locks..

 static void ext4_unregister_li_request(struct super_block *sb)
 {
-	struct ext4_li_request *elr = EXT4_SB(sb)->s_li_request;
-
-	if (!ext4_li_info)
+	mutex_lock(&ext4_li_mtx);
+	if (!ext4_li_info) {
+		mutex_unlock(&ext4_li_mtx);
 		return;
+	}
 
 	mutex_lock(&ext4_li_info->li_list_mtx);
-	ext4_remove_li_request(elr);
+	ext4_remove_li_request(EXT4_SB(sb)->s_li_request);
 	mutex_unlock(&ext4_li_info->li_list_mtx);
+	mutex_unlock(&ext4_li_mtx);
 }

Using this patch, the whole retrieval and removal are performed inside the MUTEX locks to avoid such race conditions.

Written by xorl

June 9, 2011 at 15:46

Posted in bugs, linux

Leave a Reply

Fill in your details below or click an icon to log in:

WordPress.com Logo

You are commenting using your WordPress.com account. Log Out / Change )

Twitter picture

You are commenting using your Twitter account. Log Out / Change )

Facebook photo

You are commenting using your Facebook account. Log Out / Change )

Google+ photo

You are commenting using your Google+ account. Log Out / Change )

Connecting to %s