[PATCH] mm/gup_test: fix race with PIN_LONGTERM_TEST ioctls

[PATCH] mm/gup_test: fix race with PIN_LONGTERM_TEST ioctls

[Yunhui Cui]

The PIN_LONGTERM_TEST helpers keep their state in global variables that
are protected by pin_longterm_test_mutex when accessed from ioctl().
However, gup_test_release() calls pin_longterm_test_stop() without
holding that mutex.

This can race with PIN_LONGTERM_TEST_STOP and let two callers operate on
the same pages array concurrently, corrupting the test state and possibly
freeing it twice:

 CPU 0                              CPU 1
 -----                              -----
 ioctl(PIN_LONGTERM_TEST_STOP)
   mutex_lock(&pin_longterm_test_mutex)
   pin_longterm_test_stop()
     if (pin_longterm_test_pages)
       kvfree(pin_longterm_test_pages)

                                    close()
                                      gup_test_release()
                                        pin_longterm_test_stop()
                                          if (pin_longterm_test_pages)
                                            kvfree(pin_longterm_test_pages)

     pin_longterm_test_pages = NULL
   mutex_unlock(&pin_longterm_test_mutex)

Protect the release path with the same mutex so that stop and release
cannot run pin_longterm_test_stop() concurrently.

Fixes: c77369b437f9 ("mm/gup_test: start/stop/read functionality for PIN LONGTERM test")
Cc: stable@vger.kernel.org
Signed-off-by: Yunhui Cui <cuiyunhui@bytedance.com>
---
 mm/gup_test.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/mm/gup_test.c b/mm/gup_test.c
index 9dd48db897b95..d1c2b1014f0ef 100644
--- a/mm/gup_test.c
+++ b/mm/gup_test.c
@@ -373,7 +373,9 @@ static long gup_test_ioctl(struct file *filep, unsigned int cmd,
 
 static int gup_test_release(struct inode *inode, struct file *file)
 {
+	mutex_lock(&pin_longterm_test_mutex);
 	pin_longterm_test_stop();
+	mutex_unlock(&pin_longterm_test_mutex);
 
 	return 0;
 }
-- 
2.39.5

Re: [PATCH] mm/gup_test: fix race with PIN_LONGTERM_TEST ioctls

[John Hubbard]

On 6/7/26 7:50 PM, Yunhui Cui wrote:
> The PIN_LONGTERM_TEST helpers keep their state in global variables that
> are protected by pin_longterm_test_mutex when accessed from ioctl().
> However, gup_test_release() calls pin_longterm_test_stop() without
> holding that mutex.
> 
> This can race with PIN_LONGTERM_TEST_STOP and let two callers operate on
> the same pages array concurrently, corrupting the test state and possibly
> freeing it twice:

Let's add here that there are *no* such callers in the kernel, today.

> 
>  CPU 0                              CPU 1
>  -----                              -----
>  ioctl(PIN_LONGTERM_TEST_STOP)
>    mutex_lock(&pin_longterm_test_mutex)
>    pin_longterm_test_stop()
>      if (pin_longterm_test_pages)
>        kvfree(pin_longterm_test_pages)
> 
>                                     close()
>                                       gup_test_release()
>                                         pin_longterm_test_stop()
>                                           if (pin_longterm_test_pages)
>                                             kvfree(pin_longterm_test_pages)
> 
>      pin_longterm_test_pages = NULL
>    mutex_unlock(&pin_longterm_test_mutex)
> 
> Protect the release path with the same mutex so that stop and release
> cannot run pin_longterm_test_stop() concurrently.
> 
> Fixes: c77369b437f9 ("mm/gup_test: start/stop/read functionality for PIN LONGTERM test")
> Cc: stable@vger.kernel.org

umm, no, to "Cc: stable". This is the sort of thing that gives AI
a bad name. Specifically:

* Nothing in tree can possibly hit this race condition.

* This fix is purely static code analysis hygiene: correcting
  a theoretical problem that does not actually provide any
  sort of vulnerability fix in the kernel.

So claiming that the fix must go to stable is AI just making
overly grandiose claims, which I'm getting used to seeing lately,
but it still irritates.

> Signed-off-by: Yunhui Cui <cuiyunhui@bytedance.com>
> ---
>  mm/gup_test.c | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/mm/gup_test.c b/mm/gup_test.c
> index 9dd48db897b95..d1c2b1014f0ef 100644
> --- a/mm/gup_test.c
> +++ b/mm/gup_test.c
> @@ -373,7 +373,9 @@ static long gup_test_ioctl(struct file *filep, unsigned int cmd,
>  
>  static int gup_test_release(struct inode *inode, struct file *file)
>  {
> +	mutex_lock(&pin_longterm_test_mutex);
>  	pin_longterm_test_stop();
> +	mutex_unlock(&pin_longterm_test_mutex);
>  
>  	return 0;
>  }

With "Cc: stable", removed, please feel free to add:

Reviewed-by: John Hubbard <jhubbard@nvidia.com>

thanks,
-- 
John Hubbard

Re: [PATCH] mm/gup_test: fix race with PIN_LONGTERM_TEST ioctls

[David Hildenbrand (Arm)]

On 6/12/26 06:20, John Hubbard wrote:
> On 6/7/26 7:50 PM, Yunhui Cui wrote:
>> The PIN_LONGTERM_TEST helpers keep their state in global variables that
>> are protected by pin_longterm_test_mutex when accessed from ioctl().
>> However, gup_test_release() calls pin_longterm_test_stop() without
>> holding that mutex.
>>
>> This can race with PIN_LONGTERM_TEST_STOP and let two callers operate on
>> the same pages array concurrently, corrupting the test state and possibly
>> freeing it twice:
> 
> Let's add here that there are *no* such callers in the kernel, today.
> 
>>
>>  CPU 0                              CPU 1
>>  -----                              -----
>>  ioctl(PIN_LONGTERM_TEST_STOP)
>>    mutex_lock(&pin_longterm_test_mutex)
>>    pin_longterm_test_stop()
>>      if (pin_longterm_test_pages)
>>        kvfree(pin_longterm_test_pages)
>>
>>                                     close()
>>                                       gup_test_release()
>>                                         pin_longterm_test_stop()
>>                                           if (pin_longterm_test_pages)
>>                                             kvfree(pin_longterm_test_pages)
>>
>>      pin_longterm_test_pages = NULL
>>    mutex_unlock(&pin_longterm_test_mutex)
>>
>> Protect the release path with the same mutex so that stop and release
>> cannot run pin_longterm_test_stop() concurrently.
>>
>> Fixes: c77369b437f9 ("mm/gup_test: start/stop/read functionality for PIN LONGTERM test")
>> Cc: stable@vger.kernel.org
> 
> umm, no, to "Cc: stable". This is the sort of thing that gives AI
> a bad name. Specifically:
> 
> * Nothing in tree can possibly hit this race condition.
> 
> * This fix is purely static code analysis hygiene: correcting
>   a theoretical problem that does not actually provide any
>   sort of vulnerability fix in the kernel.
> 

Yes, all rather useless churn that consumes our capacity.

-- 
Cheers,

David

Re: [PATCH] mm/gup_test: fix race with PIN_LONGTERM_TEST ioctls

[David Hildenbrand (Arm)]

On 6/8/26 04:50, Yunhui Cui wrote:
> The PIN_LONGTERM_TEST helpers keep their state in global variables that
> are protected by pin_longterm_test_mutex when accessed from ioctl().
> However, gup_test_release() calls pin_longterm_test_stop() without
> holding that mutex.
> 
> This can race with PIN_LONGTERM_TEST_STOP and let two callers operate on
> the same pages array concurrently, corrupting the test state and possibly
> freeing it twice:
> 
>  CPU 0                              CPU 1
>  -----                              -----
>  ioctl(PIN_LONGTERM_TEST_STOP)
>    mutex_lock(&pin_longterm_test_mutex)
>    pin_longterm_test_stop()
>      if (pin_longterm_test_pages)
>        kvfree(pin_longterm_test_pages)
> 
>                                     close()
>                                       gup_test_release()
>                                         pin_longterm_test_stop()
>                                           if (pin_longterm_test_pages)
>                                             kvfree(pin_longterm_test_pages)
> 
>      pin_longterm_test_pages = NULL
>    mutex_unlock(&pin_longterm_test_mutex)

Okay, thinking about this some more ...

I think what's really required here is that we have two separate "struct file",
because otherwise release() cannot race with unlocked_ioctl().

Which is something we didn't expect when we added this functionality.

I think the proper way to handle this is by moving the state to the
"struct file", to actually cleanly allow concurrent usage.

So instead, I think we should do the following (untested):

From 29e3d6fe00c4bd843d11bb548efa89bca478436c Mon Sep 17 00:00:00 2001
From: "David Hildenbrand (Arm)" <david@kernel.org>
Date: Fri, 12 Jun 2026 09:22:25 +0200
Subject: [PATCH] mm/gup_test: keep longterm pin state per file

The pin longterm test currently stores its data globally, shared among
multiple concurrent users of the interface (multiple open file
descriptors -> multiple "struct file"'s). That makes
the gup_test interface problematic to use concurrently: two users, such
as concurrent selftest runs, can interfere with the same longterm
pin state.

While this has not been observed as a problem so far in practice, let's
just handle it cleanly. There could be a way to trigger selftest
failures by e.g., running the cow.c and gup_longerm.c selftests
concurrently, but we usually run them sequentially. Let's add a "Fixes"
tag to be safe.

Fixes: c77369b437f9 ("mm/gup_test: start/stop/read functionality for PIN LONGTERM test")
Signed-off-by: David Hildenbrand (Arm) <david@kernel.org>
---
 mm/gup_test.c | 93 +++++++++++++++++++++++++++++++++------------------
 1 file changed, 61 insertions(+), 32 deletions(-)

diff --git a/mm/gup_test.c b/mm/gup_test.c
index 9dd48db897b9..16916056677e 100644
--- a/mm/gup_test.c
+++ b/mm/gup_test.c
@@ -8,6 +8,12 @@
 #include <linux/highmem.h>
 #include "gup_test.h"
 
+struct gup_test_data {
+	struct mutex longterm_mutex;
+	struct page **longterm_pages;
+	unsigned long longterm_nr_pages;
+};
+
 static void put_back_pages(unsigned int cmd, struct page **pages,
 			   unsigned long nr_pages, unsigned int gup_test_flags)
 {
@@ -204,23 +210,20 @@ static int __gup_test_ioctl(unsigned int cmd,
 	return ret;
 }
 
-static DEFINE_MUTEX(pin_longterm_test_mutex);
-static struct page **pin_longterm_test_pages;
-static unsigned long pin_longterm_test_nr_pages;
-
-static inline void pin_longterm_test_stop(void)
+static inline void pin_longterm_test_stop(struct gup_test_data *data)
 {
-	if (pin_longterm_test_pages) {
-		if (pin_longterm_test_nr_pages)
-			unpin_user_pages(pin_longterm_test_pages,
-					 pin_longterm_test_nr_pages);
-		kvfree(pin_longterm_test_pages);
-		pin_longterm_test_pages = NULL;
-		pin_longterm_test_nr_pages = 0;
+	if (data->longterm_pages) {
+		if (data->longterm_nr_pages)
+			unpin_user_pages(data->longterm_pages,
+					 data->longterm_nr_pages);
+		kvfree(data->longterm_pages);
+		data->longterm_pages = NULL;
+		data->longterm_nr_pages = 0;
 	}
 }
 
-static inline int pin_longterm_test_start(unsigned long arg)
+static inline int pin_longterm_test_start(struct gup_test_data *data,
+		unsigned long arg)
 {
 	long nr_pages, cur_pages, addr, remaining_pages;
 	int gup_flags = FOLL_LONGTERM;
@@ -229,7 +232,7 @@ static inline int pin_longterm_test_start(unsigned long arg)
 	int ret = 0;
 	bool fast;
 
-	if (pin_longterm_test_pages)
+	if (data->longterm_pages)
 		return -EINVAL;
 
 	if (copy_from_user(&args, (void __user *)arg, sizeof(args)))
@@ -259,12 +262,12 @@ static inline int pin_longterm_test_start(unsigned long arg)
 		return -EINTR;
 	}
 
-	pin_longterm_test_pages = pages;
-	pin_longterm_test_nr_pages = 0;
+	data->longterm_pages = pages;
+	data->longterm_nr_pages = 0;
 
-	while (nr_pages - pin_longterm_test_nr_pages) {
-		remaining_pages = nr_pages - pin_longterm_test_nr_pages;
-		addr = args.addr + pin_longterm_test_nr_pages * PAGE_SIZE;
+	while (nr_pages - data->longterm_nr_pages) {
+		remaining_pages = nr_pages - data->longterm_nr_pages;
+		addr = args.addr + data->longterm_nr_pages * PAGE_SIZE;
 
 		if (fast)
 			cur_pages = pin_user_pages_fast(addr, remaining_pages,
@@ -273,11 +276,11 @@ static inline int pin_longterm_test_start(unsigned long arg)
 			cur_pages = pin_user_pages(addr, remaining_pages,
 						   gup_flags, pages);
 		if (cur_pages < 0) {
-			pin_longterm_test_stop();
+			pin_longterm_test_stop(data);
 			ret = cur_pages;
 			break;
 		}
-		pin_longterm_test_nr_pages += cur_pages;
+		data->longterm_nr_pages += cur_pages;
 		pages += cur_pages;
 	}
 
@@ -286,19 +289,20 @@ static inline int pin_longterm_test_start(unsigned long arg)
 	return ret;
 }
 
-static inline int pin_longterm_test_read(unsigned long arg)
+static inline int pin_longterm_test_read(struct gup_test_data *data,
+		unsigned long arg)
 {
 	__u64 user_addr;
 	unsigned long i;
 
-	if (!pin_longterm_test_pages)
+	if (!data->longterm_pages)
 		return -EINVAL;
 
 	if (copy_from_user(&user_addr, (void __user *)arg, sizeof(user_addr)))
 		return -EFAULT;
 
-	for (i = 0; i < pin_longterm_test_nr_pages; i++) {
-		void *addr = kmap_local_page(pin_longterm_test_pages[i]);
+	for (i = 0; i < data->longterm_nr_pages; i++) {
+		void *addr = kmap_local_page(data->longterm_pages[i]);
 		unsigned long ret;
 
 		ret = copy_to_user((void __user *)(unsigned long)user_addr, addr,
@@ -314,25 +318,26 @@ static inline int pin_longterm_test_read(unsigned long arg)
 static long pin_longterm_test_ioctl(struct file *filep, unsigned int cmd,
 				    unsigned long arg)
 {
+	struct gup_test_data *data = filep->private_data;
 	int ret = -EINVAL;
 
-	if (mutex_lock_killable(&pin_longterm_test_mutex))
+	if (mutex_lock_killable(&data->longterm_mutex))
 		return -EINTR;
 
 	switch (cmd) {
 	case PIN_LONGTERM_TEST_START:
-		ret = pin_longterm_test_start(arg);
+		ret = pin_longterm_test_start(data, arg);
 		break;
 	case PIN_LONGTERM_TEST_STOP:
-		pin_longterm_test_stop();
+		pin_longterm_test_stop(data);
 		ret = 0;
 		break;
 	case PIN_LONGTERM_TEST_READ:
-		ret = pin_longterm_test_read(arg);
+		ret = pin_longterm_test_read(data, arg);
 		break;
 	}
 
-	mutex_unlock(&pin_longterm_test_mutex);
+	mutex_unlock(&data->longterm_mutex);
 	return ret;
 }
 
@@ -371,15 +376,39 @@ static long gup_test_ioctl(struct file *filep, unsigned int cmd,
 	return 0;
 }
 
+static int gup_test_open(struct inode *inode, struct file *file)
+{
+	struct gup_test_data *data;
+	int ret;
+
+	data = kzalloc(sizeof(*data), GFP_KERNEL);
+	if (!data)
+		return -ENOMEM;
+
+	ret = nonseekable_open(inode, file);
+	if (ret) {
+		kfree(data);
+		return ret;
+	}
+
+	mutex_init(&data->longterm_mutex);
+	file->private_data = data;
+	return 0;
+}
+
 static int gup_test_release(struct inode *inode, struct file *file)
 {
-	pin_longterm_test_stop();
+	struct gup_test_data *data = file->private_data;
+
+	pin_longterm_test_stop(data);
+	kfree(data);
+	file->private_data = NULL;
 
 	return 0;
 }
 
 static const struct file_operations gup_test_fops = {
-	.open = nonseekable_open,
+	.open = gup_test_open,
 	.unlocked_ioctl = gup_test_ioctl,
 	.compat_ioctl = compat_ptr_ioctl,
 	.release = gup_test_release,
-- 
2.43.0



Can you review+test that change? Thanks!

-- 
Cheers,

David

Re: [External] Re: [PATCH] mm/gup_test: fix race with PIN_LONGTERM_TEST ioctls

[yunhui cui]

Hi David,

On Fri, Jun 12, 2026 at 3:36 PM David Hildenbrand (Arm)
<david@kernel.org> wrote:
>
> On 6/8/26 04:50, Yunhui Cui wrote:
> > The PIN_LONGTERM_TEST helpers keep their state in global variables that
> > are protected by pin_longterm_test_mutex when accessed from ioctl().
> > However, gup_test_release() calls pin_longterm_test_stop() without
> > holding that mutex.
> >
> > This can race with PIN_LONGTERM_TEST_STOP and let two callers operate on
> > the same pages array concurrently, corrupting the test state and possibly
> > freeing it twice:
> >
> >  CPU 0                              CPU 1
> >  -----                              -----
> >  ioctl(PIN_LONGTERM_TEST_STOP)
> >    mutex_lock(&pin_longterm_test_mutex)
> >    pin_longterm_test_stop()
> >      if (pin_longterm_test_pages)
> >        kvfree(pin_longterm_test_pages)
> >
> >                                     close()
> >                                       gup_test_release()
> >                                         pin_longterm_test_stop()
> >                                           if (pin_longterm_test_pages)
> >                                             kvfree(pin_longterm_test_pages)
> >
> >      pin_longterm_test_pages = NULL
> >    mutex_unlock(&pin_longterm_test_mutex)
>
> Okay, thinking about this some more ...
>
> I think what's really required here is that we have two separate "struct file",
> because otherwise release() cannot race with unlocked_ioctl().
>
> Which is something we didn't expect when we added this functionality.
>
> I think the proper way to handle this is by moving the state to the
> "struct file", to actually cleanly allow concurrent usage.
>
> So instead, I think we should do the following (untested):
>
> From 29e3d6fe00c4bd843d11bb548efa89bca478436c Mon Sep 17 00:00:00 2001
> From: "David Hildenbrand (Arm)" <david@kernel.org>
> Date: Fri, 12 Jun 2026 09:22:25 +0200
> Subject: [PATCH] mm/gup_test: keep longterm pin state per file
>
> The pin longterm test currently stores its data globally, shared among
> multiple concurrent users of the interface (multiple open file
> descriptors -> multiple "struct file"'s). That makes
> the gup_test interface problematic to use concurrently: two users, such
> as concurrent selftest runs, can interfere with the same longterm
> pin state.
>
> While this has not been observed as a problem so far in practice, let's
> just handle it cleanly. There could be a way to trigger selftest
> failures by e.g., running the cow.c and gup_longerm.c selftests
> concurrently, but we usually run them sequentially. Let's add a "Fixes"
> tag to be safe.
>
> Fixes: c77369b437f9 ("mm/gup_test: start/stop/read functionality for PIN LONGTERM test")
> Signed-off-by: David Hildenbrand (Arm) <david@kernel.org>
> ---
>  mm/gup_test.c | 93 +++++++++++++++++++++++++++++++++------------------
>  1 file changed, 61 insertions(+), 32 deletions(-)
>
> diff --git a/mm/gup_test.c b/mm/gup_test.c
> index 9dd48db897b9..16916056677e 100644
> --- a/mm/gup_test.c
> +++ b/mm/gup_test.c
> @@ -8,6 +8,12 @@
>  #include <linux/highmem.h>
>  #include "gup_test.h"
>
> +struct gup_test_data {
> +       struct mutex longterm_mutex;
> +       struct page **longterm_pages;
> +       unsigned long longterm_nr_pages;
> +};
> +
>  static void put_back_pages(unsigned int cmd, struct page **pages,
>                            unsigned long nr_pages, unsigned int gup_test_flags)
>  {
> @@ -204,23 +210,20 @@ static int __gup_test_ioctl(unsigned int cmd,
>         return ret;
>  }
>
> -static DEFINE_MUTEX(pin_longterm_test_mutex);
> -static struct page **pin_longterm_test_pages;
> -static unsigned long pin_longterm_test_nr_pages;
> -
> -static inline void pin_longterm_test_stop(void)
> +static inline void pin_longterm_test_stop(struct gup_test_data *data)
>  {
> -       if (pin_longterm_test_pages) {
> -               if (pin_longterm_test_nr_pages)
> -                       unpin_user_pages(pin_longterm_test_pages,
> -                                        pin_longterm_test_nr_pages);
> -               kvfree(pin_longterm_test_pages);
> -               pin_longterm_test_pages = NULL;
> -               pin_longterm_test_nr_pages = 0;
> +       if (data->longterm_pages) {
> +               if (data->longterm_nr_pages)
> +                       unpin_user_pages(data->longterm_pages,
> +                                        data->longterm_nr_pages);
> +               kvfree(data->longterm_pages);
> +               data->longterm_pages = NULL;
> +               data->longterm_nr_pages = 0;
>         }
>  }
>
> -static inline int pin_longterm_test_start(unsigned long arg)
> +static inline int pin_longterm_test_start(struct gup_test_data *data,
> +               unsigned long arg)
>  {
>         long nr_pages, cur_pages, addr, remaining_pages;
>         int gup_flags = FOLL_LONGTERM;
> @@ -229,7 +232,7 @@ static inline int pin_longterm_test_start(unsigned long arg)
>         int ret = 0;
>         bool fast;
>
> -       if (pin_longterm_test_pages)
> +       if (data->longterm_pages)
>                 return -EINVAL;
>
>         if (copy_from_user(&args, (void __user *)arg, sizeof(args)))
> @@ -259,12 +262,12 @@ static inline int pin_longterm_test_start(unsigned long arg)
>                 return -EINTR;
>         }
>
> -       pin_longterm_test_pages = pages;
> -       pin_longterm_test_nr_pages = 0;
> +       data->longterm_pages = pages;
> +       data->longterm_nr_pages = 0;
>
> -       while (nr_pages - pin_longterm_test_nr_pages) {
> -               remaining_pages = nr_pages - pin_longterm_test_nr_pages;
> -               addr = args.addr + pin_longterm_test_nr_pages * PAGE_SIZE;
> +       while (nr_pages - data->longterm_nr_pages) {
> +               remaining_pages = nr_pages - data->longterm_nr_pages;
> +               addr = args.addr + data->longterm_nr_pages * PAGE_SIZE;
>
>                 if (fast)
>                         cur_pages = pin_user_pages_fast(addr, remaining_pages,
> @@ -273,11 +276,11 @@ static inline int pin_longterm_test_start(unsigned long arg)
>                         cur_pages = pin_user_pages(addr, remaining_pages,
>                                                    gup_flags, pages);
>                 if (cur_pages < 0) {
> -                       pin_longterm_test_stop();
> +                       pin_longterm_test_stop(data);
>                         ret = cur_pages;
>                         break;
>                 }
> -               pin_longterm_test_nr_pages += cur_pages;
> +               data->longterm_nr_pages += cur_pages;
>                 pages += cur_pages;
>         }
>
> @@ -286,19 +289,20 @@ static inline int pin_longterm_test_start(unsigned long arg)
>         return ret;
>  }
>
> -static inline int pin_longterm_test_read(unsigned long arg)
> +static inline int pin_longterm_test_read(struct gup_test_data *data,
> +               unsigned long arg)
>  {
>         __u64 user_addr;
>         unsigned long i;
>
> -       if (!pin_longterm_test_pages)
> +       if (!data->longterm_pages)
>                 return -EINVAL;
>
>         if (copy_from_user(&user_addr, (void __user *)arg, sizeof(user_addr)))
>                 return -EFAULT;
>
> -       for (i = 0; i < pin_longterm_test_nr_pages; i++) {
> -               void *addr = kmap_local_page(pin_longterm_test_pages[i]);
> +       for (i = 0; i < data->longterm_nr_pages; i++) {
> +               void *addr = kmap_local_page(data->longterm_pages[i]);
>                 unsigned long ret;
>
>                 ret = copy_to_user((void __user *)(unsigned long)user_addr, addr,
> @@ -314,25 +318,26 @@ static inline int pin_longterm_test_read(unsigned long arg)
>  static long pin_longterm_test_ioctl(struct file *filep, unsigned int cmd,
>                                     unsigned long arg)
>  {
> +       struct gup_test_data *data = filep->private_data;
>         int ret = -EINVAL;
>
> -       if (mutex_lock_killable(&pin_longterm_test_mutex))
> +       if (mutex_lock_killable(&data->longterm_mutex))
>                 return -EINTR;
>
>         switch (cmd) {
>         case PIN_LONGTERM_TEST_START:
> -               ret = pin_longterm_test_start(arg);
> +               ret = pin_longterm_test_start(data, arg);
>                 break;
>         case PIN_LONGTERM_TEST_STOP:
> -               pin_longterm_test_stop();
> +               pin_longterm_test_stop(data);
>                 ret = 0;
>                 break;
>         case PIN_LONGTERM_TEST_READ:
> -               ret = pin_longterm_test_read(arg);
> +               ret = pin_longterm_test_read(data, arg);
>                 break;
>         }
>
> -       mutex_unlock(&pin_longterm_test_mutex);
> +       mutex_unlock(&data->longterm_mutex);
>         return ret;
>  }
>
> @@ -371,15 +376,39 @@ static long gup_test_ioctl(struct file *filep, unsigned int cmd,
>         return 0;
>  }
>
> +static int gup_test_open(struct inode *inode, struct file *file)
> +{
> +       struct gup_test_data *data;
> +       int ret;
> +
> +       data = kzalloc(sizeof(*data), GFP_KERNEL);
> +       if (!data)
> +               return -ENOMEM;
> +
> +       ret = nonseekable_open(inode, file);
> +       if (ret) {
> +               kfree(data);
> +               return ret;
> +       }
> +
> +       mutex_init(&data->longterm_mutex);
> +       file->private_data = data;
> +       return 0;
> +}
> +
>  static int gup_test_release(struct inode *inode, struct file *file)
>  {
> -       pin_longterm_test_stop();
> +       struct gup_test_data *data = file->private_data;
> +
> +       pin_longterm_test_stop(data);
> +       kfree(data);
> +       file->private_data = NULL;
>
>         return 0;
>  }
>
>  static const struct file_operations gup_test_fops = {
> -       .open = nonseekable_open,
> +       .open = gup_test_open,
>         .unlocked_ioctl = gup_test_ioctl,
>         .compat_ioctl = compat_ptr_ioctl,
>         .release = gup_test_release,
> --
> 2.43.0
>
>
>
> Can you review+test that change? Thanks!

Sorry for the late reply.
I tested your patch and it works fine for me.
Tested-by: Yunhui Cui <cuiyunhui@bytedance.com>

>
> --
> Cheers,
>
> David

 Thanks,
 Yunhui

Re: [External] Re: [PATCH] mm/gup_test: fix race with PIN_LONGTERM_TEST ioctls

[David Hildenbrand (Arm)]

>>
>> Can you review+test that change? Thanks!
> 
> Sorry for the late reply.
> I tested your patch and it works fine for me.
> Tested-by: Yunhui Cui <cuiyunhui@bytedance.com>

Thanks, let me send it out as a proper patch.

-- 
Cheers,

David