Currently if we request a feature that is not set in the Kernel config we fail silently and return all the available features. However, the man page indicates we should return an EINVAL.
We need to fix this issue since we can end up with a Kernel warning should a program request the feature UFFD_FEATURE_WP_UNPOPULATED on a kernel with the config not set with this feature.
[ 200.812896] WARNING: CPU: 91 PID: 13634 at mm/memory.c:1660 zap_pte_range+0x43d/0x660 [ 200.820738] Modules linked in: [ 200.869387] CPU: 91 PID: 13634 Comm: userfaultfd Kdump: loaded Not tainted 6.9.0-rc5+ #8 [ 200.877477] Hardware name: Dell Inc. PowerEdge R6525/0N7YGH, BIOS 2.7.3 03/30/2022 [ 200.885052] RIP: 0010:zap_pte_range+0x43d/0x660
Fixes: e06f1e1dd499 ("userfaultfd: wp: enabled write protection in userfaultfd API") Signed-off-by: Audra Mitchell audra@redhat.com --- fs/userfaultfd.c | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-)
diff --git a/fs/userfaultfd.c b/fs/userfaultfd.c index eee7320ab0b0..17e409ceaa33 100644 --- a/fs/userfaultfd.c +++ b/fs/userfaultfd.c @@ -2057,7 +2057,7 @@ static int userfaultfd_api(struct userfaultfd_ctx *ctx, goto out; features = uffdio_api.features; ret = -EINVAL; - if (uffdio_api.api != UFFD_API || (features & ~UFFD_API_FEATURES)) + if (uffdio_api.api != UFFD_API) goto err_out; ret = -EPERM; if ((features & UFFD_FEATURE_EVENT_FORK) && !capable(CAP_SYS_PTRACE)) @@ -2081,6 +2081,11 @@ static int userfaultfd_api(struct userfaultfd_ctx *ctx, uffdio_api.features &= ~UFFD_FEATURE_WP_UNPOPULATED; uffdio_api.features &= ~UFFD_FEATURE_WP_ASYNC; #endif + + ret = -EINVAL; + if (features & ~uffdio_api.features) + goto err_out; + uffdio_api.ioctls = UFFD_API_IOCTLS; ret = -EFAULT; if (copy_to_user(buf, &uffdio_api, sizeof(uffdio_api)))
Now that we have updated userfaultfd_api to correctly return EIVAL when a feature is requested but not available, let's fix the uffd-stress test to only set the UFFD_FEATURE_WP_UNPOPULATED feature when the config is set. In addition, still run the test if the CONFIG_PTE_MARKER_UFFD_WP is not set, just dont use the corresponding UFFD_FEATURE_WP_UNPOPULATED feature.
Signed-off-by: Audra Mitchell audra@redhat.com --- tools/testing/selftests/mm/uffd-stress.c | 14 +++++++++----- 1 file changed, 9 insertions(+), 5 deletions(-)
diff --git a/tools/testing/selftests/mm/uffd-stress.c b/tools/testing/selftests/mm/uffd-stress.c index f78bab0f3d45..b9b6d858eab8 100644 --- a/tools/testing/selftests/mm/uffd-stress.c +++ b/tools/testing/selftests/mm/uffd-stress.c @@ -38,6 +38,8 @@
#ifdef __NR_userfaultfd
+uint64_t features; + #define BOUNCE_RANDOM (1<<0) #define BOUNCE_RACINGFAULTS (1<<1) #define BOUNCE_VERIFY (1<<2) @@ -247,10 +249,14 @@ static int userfaultfd_stress(void) unsigned long nr; struct uffd_args args[nr_cpus]; uint64_t mem_size = nr_pages * page_size; + int flags = 0;
memset(args, 0, sizeof(struct uffd_args) * nr_cpus);
- if (uffd_test_ctx_init(UFFD_FEATURE_WP_UNPOPULATED, NULL)) + if (features & UFFD_FEATURE_WP_UNPOPULATED && test_type == TEST_ANON) + flags = UFFD_FEATURE_WP_UNPOPULATED; + + if (uffd_test_ctx_init(flags, NULL)) err("context init failed");
if (posix_memalign(&area, page_size, page_size)) @@ -385,8 +391,6 @@ static void set_test_type(const char *type)
static void parse_test_type_arg(const char *raw_type) { - uint64_t features = UFFD_API_FEATURES; - set_test_type(raw_type);
if (!test_type) @@ -409,8 +413,8 @@ static void parse_test_type_arg(const char *raw_type) * feature. */
- if (userfaultfd_open(&features)) - err("Userfaultfd open failed"); + if (uffd_get_features(&features)) + err("failed to get available features");
test_uffdio_wp = test_uffdio_wp && (features & UFFD_FEATURE_PAGEFAULT_FLAG_WP);
On Fri, Jun 21, 2024 at 02:12:23PM -0400, Audra Mitchell wrote:
Now that we have updated userfaultfd_api to correctly return EIVAL when a feature is requested but not available, let's fix the uffd-stress test to only set the UFFD_FEATURE_WP_UNPOPULATED feature when the config is set. In addition, still run the test if the CONFIG_PTE_MARKER_UFFD_WP is not set, just dont use the corresponding UFFD_FEATURE_WP_UNPOPULATED feature.
Signed-off-by: Audra Mitchell audra@redhat.com
Acked-by: Peter Xu peterx@redhat.com
If CONFIG_PTE_MARKER_UFFD_WP is disabled, then testing with test_uffdio_up enables calling uffdio_regsiter with the flag UFFDIO_REGISTER_MODE_WP. The kernel ensures in vma_can_userfault() that if CONFIG_PTE_MARKER_UFFD_WP is disabled, only allow the VM_UFFD_WP on anonymous vmas.
Signed-off-by: Audra Mitchell audra@redhat.com --- tools/testing/selftests/mm/uffd-stress.c | 3 +++ 1 file changed, 3 insertions(+)
diff --git a/tools/testing/selftests/mm/uffd-stress.c b/tools/testing/selftests/mm/uffd-stress.c index b9b6d858eab8..2601c9dfadd6 100644 --- a/tools/testing/selftests/mm/uffd-stress.c +++ b/tools/testing/selftests/mm/uffd-stress.c @@ -419,6 +419,9 @@ static void parse_test_type_arg(const char *raw_type) test_uffdio_wp = test_uffdio_wp && (features & UFFD_FEATURE_PAGEFAULT_FLAG_WP);
+ if (test_type != TEST_ANON && !(features & UFFD_FEATURE_WP_UNPOPULATED)) + test_uffdio_wp = false; + close(uffd); uffd = -1; }
On Fri, Jun 21, 2024 at 02:12:24PM -0400, Audra Mitchell wrote:
If CONFIG_PTE_MARKER_UFFD_WP is disabled, then testing with test_uffdio_up
Here you're talking about pte markers, then..
enables calling uffdio_regsiter with the flag UFFDIO_REGISTER_MODE_WP. The kernel ensures in vma_can_userfault() that if CONFIG_PTE_MARKER_UFFD_WP is disabled, only allow the VM_UFFD_WP on anonymous vmas.
Signed-off-by: Audra Mitchell audra@redhat.com
tools/testing/selftests/mm/uffd-stress.c | 3 +++ 1 file changed, 3 insertions(+)
diff --git a/tools/testing/selftests/mm/uffd-stress.c b/tools/testing/selftests/mm/uffd-stress.c index b9b6d858eab8..2601c9dfadd6 100644 --- a/tools/testing/selftests/mm/uffd-stress.c +++ b/tools/testing/selftests/mm/uffd-stress.c @@ -419,6 +419,9 @@ static void parse_test_type_arg(const char *raw_type) test_uffdio_wp = test_uffdio_wp && (features & UFFD_FEATURE_PAGEFAULT_FLAG_WP);
- if (test_type != TEST_ANON && !(features & UFFD_FEATURE_WP_UNPOPULATED))
test_uffdio_wp = false;
... here you're checking against wp_unpopulated. I'm slightly confused.
Are you running this test over shmem/hugetlb when the WP feature isn't supported?
I'm wondering whether you're looking for UFFD_FEATURE_WP_HUGETLBFS_SHMEM instead.
Thanks,
- close(uffd); uffd = -1;
}
2.44.0
On Fri, Jun 21, 2024 at 05:27:43PM -0400, Peter Xu wrote:
On Fri, Jun 21, 2024 at 02:12:24PM -0400, Audra Mitchell wrote:
If CONFIG_PTE_MARKER_UFFD_WP is disabled, then testing with test_uffdio_up
Here you're talking about pte markers, then..
enables calling uffdio_regsiter with the flag UFFDIO_REGISTER_MODE_WP. The kernel ensures in vma_can_userfault() that if CONFIG_PTE_MARKER_UFFD_WP is disabled, only allow the VM_UFFD_WP on anonymous vmas.
Signed-off-by: Audra Mitchell audra@redhat.com
tools/testing/selftests/mm/uffd-stress.c | 3 +++ 1 file changed, 3 insertions(+)
diff --git a/tools/testing/selftests/mm/uffd-stress.c b/tools/testing/selftests/mm/uffd-stress.c index b9b6d858eab8..2601c9dfadd6 100644 --- a/tools/testing/selftests/mm/uffd-stress.c +++ b/tools/testing/selftests/mm/uffd-stress.c @@ -419,6 +419,9 @@ static void parse_test_type_arg(const char *raw_type) test_uffdio_wp = test_uffdio_wp && (features & UFFD_FEATURE_PAGEFAULT_FLAG_WP);
- if (test_type != TEST_ANON && !(features & UFFD_FEATURE_WP_UNPOPULATED))
test_uffdio_wp = false;... here you're checking against wp_unpopulated. I'm slightly confused.
Are you running this test over shmem/hugetlb when the WP feature isn't supported?
I'm wondering whether you're looking for UFFD_FEATURE_WP_HUGETLBFS_SHMEM instead.
I can confirm, its all really confusing... So in userfaultfd_api, we disable three features if CONFIG_PTE_MARKER_UFFD_WP is not enabled- including UFFD_FEATURE_WP_UNPOPULATED:
#ifndef CONFIG_PTE_MARKER_UFFD_WP uffdio_api.features &= ~UFFD_FEATURE_WP_HUGETLBFS_SHMEM; uffdio_api.features &= ~UFFD_FEATURE_WP_UNPOPULATED; uffdio_api.features &= ~UFFD_FEATURE_WP_ASYNC; #endif
If you run the userfaultfd selftests with the run_vmtests script we get several failures stemming from trying to call uffdio_regsiter with the flag UFFDIO_REGISTER_MODE_WP. However, the kernel ensures in vma_can_userfault() that if CONFIG_PTE_MARKER_UFFD_WP is disabled, only allow the VM_UFFD_WP - which is set when you pass the UFFDIO_REGISTER_MODE_WP flag - on anonymous vmas.
In parse_test_type_arg() I added the features check against UFFD_FEATURE_WP_UNPOPULATED as it seemed the most well know feature/flag. I'm more than happy to take any suggestions and adapt them if you have any!
Thanks in advance and happy Monday!
-- Audra
Thanks,
- close(uffd); uffd = -1;
}
2.44.0
-- Peter Xu
On Mon, Jun 24, 2024 at 09:53:57AM -0400, Audra Mitchell wrote:
On Fri, Jun 21, 2024 at 05:27:43PM -0400, Peter Xu wrote:
On Fri, Jun 21, 2024 at 02:12:24PM -0400, Audra Mitchell wrote:
If CONFIG_PTE_MARKER_UFFD_WP is disabled, then testing with test_uffdio_up
Here you're talking about pte markers, then..
enables calling uffdio_regsiter with the flag UFFDIO_REGISTER_MODE_WP. The kernel ensures in vma_can_userfault() that if CONFIG_PTE_MARKER_UFFD_WP is disabled, only allow the VM_UFFD_WP on anonymous vmas.
Signed-off-by: Audra Mitchell audra@redhat.com
tools/testing/selftests/mm/uffd-stress.c | 3 +++ 1 file changed, 3 insertions(+)
diff --git a/tools/testing/selftests/mm/uffd-stress.c b/tools/testing/selftests/mm/uffd-stress.c index b9b6d858eab8..2601c9dfadd6 100644 --- a/tools/testing/selftests/mm/uffd-stress.c +++ b/tools/testing/selftests/mm/uffd-stress.c @@ -419,6 +419,9 @@ static void parse_test_type_arg(const char *raw_type) test_uffdio_wp = test_uffdio_wp && (features & UFFD_FEATURE_PAGEFAULT_FLAG_WP);
- if (test_type != TEST_ANON && !(features & UFFD_FEATURE_WP_UNPOPULATED))
test_uffdio_wp = false;... here you're checking against wp_unpopulated. I'm slightly confused.
Are you running this test over shmem/hugetlb when the WP feature isn't supported?
I'm wondering whether you're looking for UFFD_FEATURE_WP_HUGETLBFS_SHMEM instead.
I can confirm, its all really confusing... So in userfaultfd_api, we disable three features if CONFIG_PTE_MARKER_UFFD_WP is not enabled- including UFFD_FEATURE_WP_UNPOPULATED:
#ifndef CONFIG_PTE_MARKER_UFFD_WP uffdio_api.features &= ~UFFD_FEATURE_WP_HUGETLBFS_SHMEM; uffdio_api.features &= ~UFFD_FEATURE_WP_UNPOPULATED; uffdio_api.features &= ~UFFD_FEATURE_WP_ASYNC; #endif
If you run the userfaultfd selftests with the run_vmtests script we get several failures stemming from trying to call uffdio_regsiter with the flag UFFDIO_REGISTER_MODE_WP. However, the kernel ensures in vma_can_userfault() that if CONFIG_PTE_MARKER_UFFD_WP is disabled, only allow the VM_UFFD_WP - which is set when you pass the UFFDIO_REGISTER_MODE_WP flag - on anonymous vmas.
In parse_test_type_arg() I added the features check against UFFD_FEATURE_WP_UNPOPULATED as it seemed the most well know feature/flag. I'm more than happy to take any suggestions and adapt them if you have any!
There're documents for these features in the headers:
* UFFD_FEATURE_WP_HUGETLBFS_SHMEM indicates that userfaultfd * write-protection mode is supported on both shmem and hugetlbfs. * * UFFD_FEATURE_WP_UNPOPULATED indicates that userfaultfd * write-protection mode will always apply to unpopulated pages * (i.e. empty ptes). This will be the default behavior for shmem * & hugetlbfs, so this flag only affects anonymous memory behavior * when userfault write-protection mode is registered.
While in this context ("test_type != TEST_ANON") IIUC the accurate feature to check is UFFD_FEATURE_WP_HUGETLBFS_SHMEM.
In most kernels they should behave the same indeed, but note that since UNPOPULATED was introduced later than shmem/hugetlb support, it means on some kernel the result of checking these two features will be different.
Thanks,
On Mon, 24 Jun 2024 10:42:00 -0400 Peter Xu peterx@redhat.com wrote:
uffdio_api.features &= ~UFFD_FEATURE_WP_HUGETLBFS_SHMEM; uffdio_api.features &= ~UFFD_FEATURE_WP_UNPOPULATED; uffdio_api.features &= ~UFFD_FEATURE_WP_ASYNC;#endif
If you run the userfaultfd selftests with the run_vmtests script we get several failures stemming from trying to call uffdio_regsiter with the flag UFFDIO_REGISTER_MODE_WP. However, the kernel ensures in vma_can_userfault() that if CONFIG_PTE_MARKER_UFFD_WP is disabled, only allow the VM_UFFD_WP - which is set when you pass the UFFDIO_REGISTER_MODE_WP flag - on anonymous vmas.
In parse_test_type_arg() I added the features check against UFFD_FEATURE_WP_UNPOPULATED as it seemed the most well know feature/flag. I'm more than happy to take any suggestions and adapt them if you have any!
There're documents for these features in the headers:
* UFFD_FEATURE_WP_HUGETLBFS_SHMEM indicates that userfaultfd * write-protection mode is supported on both shmem and hugetlbfs. * * UFFD_FEATURE_WP_UNPOPULATED indicates that userfaultfd * write-protection mode will always apply to unpopulated pages * (i.e. empty ptes). This will be the default behavior for shmem * & hugetlbfs, so this flag only affects anonymous memory behavior * when userfault write-protection mode is registered.While in this context ("test_type != TEST_ANON") IIUC the accurate feature to check is UFFD_FEATURE_WP_HUGETLBFS_SHMEM.
In most kernels they should behave the same indeed, but note that since UNPOPULATED was introduced later than shmem/hugetlb support, it means on some kernel the result of checking these two features will be different.
I'm unsure what to do with this series. Peter, your review comments are unclear - do you request updates?
Also, the patches weren't cc:linux-mm which limits the audience. I'll drop this version. Audra, please continue to move this forward.
On Tue, Jun 25, 2024 at 04:05:58PM -0700, Andrew Morton wrote:
On Mon, 24 Jun 2024 10:42:00 -0400 Peter Xu peterx@redhat.com wrote:
uffdio_api.features &= ~UFFD_FEATURE_WP_HUGETLBFS_SHMEM; uffdio_api.features &= ~UFFD_FEATURE_WP_UNPOPULATED; uffdio_api.features &= ~UFFD_FEATURE_WP_ASYNC;#endif
If you run the userfaultfd selftests with the run_vmtests script we get several failures stemming from trying to call uffdio_regsiter with the flag UFFDIO_REGISTER_MODE_WP. However, the kernel ensures in vma_can_userfault() that if CONFIG_PTE_MARKER_UFFD_WP is disabled, only allow the VM_UFFD_WP - which is set when you pass the UFFDIO_REGISTER_MODE_WP flag - on anonymous vmas.
In parse_test_type_arg() I added the features check against UFFD_FEATURE_WP_UNPOPULATED as it seemed the most well know feature/flag. I'm more than happy to take any suggestions and adapt them if you have any!
There're documents for these features in the headers:
* UFFD_FEATURE_WP_HUGETLBFS_SHMEM indicates that userfaultfd * write-protection mode is supported on both shmem and hugetlbfs. * * UFFD_FEATURE_WP_UNPOPULATED indicates that userfaultfd * write-protection mode will always apply to unpopulated pages * (i.e. empty ptes). This will be the default behavior for shmem * & hugetlbfs, so this flag only affects anonymous memory behavior * when userfault write-protection mode is registered.While in this context ("test_type != TEST_ANON") IIUC the accurate feature to check is UFFD_FEATURE_WP_HUGETLBFS_SHMEM.
In most kernels they should behave the same indeed, but note that since UNPOPULATED was introduced later than shmem/hugetlb support, it means on some kernel the result of checking these two features will be different.
I'm unsure what to do with this series. Peter, your review comments are unclear - do you request updates?
Yes, or some clarification from Audra would also work.
What I was trying to say is here I think the code should check against UFFD_FEATURE_WP_HUGETLBFS_SHMEM instead.
Thanks,
On Tue, Jun 25, 2024 at 07:55:14PM -0400, Peter Xu wrote:
On Tue, Jun 25, 2024 at 04:05:58PM -0700, Andrew Morton wrote:
On Mon, 24 Jun 2024 10:42:00 -0400 Peter Xu peterx@redhat.com wrote:
uffdio_api.features &= ~UFFD_FEATURE_WP_HUGETLBFS_SHMEM; uffdio_api.features &= ~UFFD_FEATURE_WP_UNPOPULATED; uffdio_api.features &= ~UFFD_FEATURE_WP_ASYNC;#endif
If you run the userfaultfd selftests with the run_vmtests script we get several failures stemming from trying to call uffdio_regsiter with the flag UFFDIO_REGISTER_MODE_WP. However, the kernel ensures in vma_can_userfault() that if CONFIG_PTE_MARKER_UFFD_WP is disabled, only allow the VM_UFFD_WP - which is set when you pass the UFFDIO_REGISTER_MODE_WP flag - on anonymous vmas.
In parse_test_type_arg() I added the features check against UFFD_FEATURE_WP_UNPOPULATED as it seemed the most well know feature/flag. I'm more than happy to take any suggestions and adapt them if you have any!
There're documents for these features in the headers:
* UFFD_FEATURE_WP_HUGETLBFS_SHMEM indicates that userfaultfd * write-protection mode is supported on both shmem and hugetlbfs. * * UFFD_FEATURE_WP_UNPOPULATED indicates that userfaultfd * write-protection mode will always apply to unpopulated pages * (i.e. empty ptes). This will be the default behavior for shmem * & hugetlbfs, so this flag only affects anonymous memory behavior * when userfault write-protection mode is registered.While in this context ("test_type != TEST_ANON") IIUC the accurate feature to check is UFFD_FEATURE_WP_HUGETLBFS_SHMEM.
In most kernels they should behave the same indeed, but note that since UNPOPULATED was introduced later than shmem/hugetlb support, it means on some kernel the result of checking these two features will be different.
I'm unsure what to do with this series. Peter, your review comments are unclear - do you request updates?
Yes, or some clarification from Audra would also work.
What I was trying to say is here I think the code should check against UFFD_FEATURE_WP_HUGETLBFS_SHMEM instead.
I was meaning to reply back and ask if Andrew wanted me to push a v3 and change the check from UFFD_FEATURE_WP_UNPOPULATED to UFFD_FEATURE_WP_HUGETLBFS_SHMEM or if he just wanted to do it, but I'll go ahead and submit v3 with the change shortly.
Also as an aside I ran scripts/get_maintainer.pl to get the email list. I probably should have thought a little bit about why the linux-mm list was missing....
Sorry about the delay and confusion!
Thanks,
-- Peter Xu
On Fri, Jun 21, 2024 at 02:12:22PM -0400, Audra Mitchell wrote:
Currently if we request a feature that is not set in the Kernel config we fail silently and return all the available features. However, the man page indicates we should return an EINVAL.
We need to fix this issue since we can end up with a Kernel warning should a program request the feature UFFD_FEATURE_WP_UNPOPULATED on a kernel with the config not set with this feature.
[ 200.812896] WARNING: CPU: 91 PID: 13634 at mm/memory.c:1660 zap_pte_range+0x43d/0x660 [ 200.820738] Modules linked in: [ 200.869387] CPU: 91 PID: 13634 Comm: userfaultfd Kdump: loaded Not tainted 6.9.0-rc5+ #8 [ 200.877477] Hardware name: Dell Inc. PowerEdge R6525/0N7YGH, BIOS 2.7.3 03/30/2022 [ 200.885052] RIP: 0010:zap_pte_range+0x43d/0x660
Fixes: e06f1e1dd499 ("userfaultfd: wp: enabled write protection in userfaultfd API") Signed-off-by: Audra Mitchell audra@redhat.com
Please prefix the subject with "mm/uffd:" or "mm/userfault:" if there's a repost.
Acked-by: Peter Xu peterx@redhat.com
Thanks,
On Fri, 21 Jun 2024 14:12:22 -0400 Audra Mitchell audra@redhat.com wrote:
Currently if we request a feature that is not set in the Kernel config we fail silently and return all the available features. However, the man page indicates we should return an EINVAL.
We need to fix this issue since we can end up with a Kernel warning should a program request the feature UFFD_FEATURE_WP_UNPOPULATED on a kernel with the config not set with this feature.
[ 200.812896] WARNING: CPU: 91 PID: 13634 at mm/memory.c:1660 zap_pte_range+0x43d/0x660 [ 200.820738] Modules linked in: [ 200.869387] CPU: 91 PID: 13634 Comm: userfaultfd Kdump: loaded Not tainted 6.9.0-rc5+ #8 [ 200.877477] Hardware name: Dell Inc. PowerEdge R6525/0N7YGH, BIOS 2.7.3 03/30/2022 [ 200.885052] RIP: 0010:zap_pte_range+0x43d/0x660
Fixes: e06f1e1dd499 ("userfaultfd: wp: enabled write protection in userfaultfd API")
A userspace-triggerable WARN is bad. I added cc:stable to this.
On Fri, Jun 21, 2024 at 06:03:30PM -0700, Andrew Morton wrote:
On Fri, 21 Jun 2024 14:12:22 -0400 Audra Mitchell audra@redhat.com wrote:
Currently if we request a feature that is not set in the Kernel config we fail silently and return all the available features. However, the man page indicates we should return an EINVAL.
We need to fix this issue since we can end up with a Kernel warning should a program request the feature UFFD_FEATURE_WP_UNPOPULATED on a kernel with the config not set with this feature.
[ 200.812896] WARNING: CPU: 91 PID: 13634 at mm/memory.c:1660 zap_pte_range+0x43d/0x660 [ 200.820738] Modules linked in: [ 200.869387] CPU: 91 PID: 13634 Comm: userfaultfd Kdump: loaded Not tainted 6.9.0-rc5+ #8 [ 200.877477] Hardware name: Dell Inc. PowerEdge R6525/0N7YGH, BIOS 2.7.3 03/30/2022 [ 200.885052] RIP: 0010:zap_pte_range+0x43d/0x660
Fixes: e06f1e1dd499 ("userfaultfd: wp: enabled write protection in userfaultfd API")
A userspace-triggerable WARN is bad. I added cc:stable to this.
Andrew,
Note that this change may fix a WARN, but it may also start to break userspace which might be worse if it happens, IMHO. I forgot to mention that here, but only mentioned that in v1, and from that POV not copying stable seems the right thing.
https://lore.kernel.org/all/ZjuIEH8TW2tWcqXQ@x1n/
In summary: I think we can stick with Fixes on e06f1e1dd499, but we don't copy stable. The major reason we don't copy stable here is not only about complexity of such backport, but also that there can be apps trying to pass in unsupported bits (even if the kernel didn't support it) but keep using MISSING mode only, then we shouldn't fail them easily after a stable upgrade. Just smells dangerous to backport.
I'm not sure what's the best solution for this. A sweet spot might be that we start to fix it in new kernels only but not stable ones.
Thanks,
On Sun, 23 Jun 2024 11:26:37 -0400 Peter Xu peterx@redhat.com wrote:
Fixes: e06f1e1dd499 ("userfaultfd: wp: enabled write protection in userfaultfd API")
A userspace-triggerable WARN is bad. I added cc:stable to this.
Andrew,
Note that this change may fix a WARN, but it may also start to break userspace which might be worse if it happens, IMHO. I forgot to mention that here, but only mentioned that in v1, and from that POV not copying stable seems the right thing.
https://lore.kernel.org/all/ZjuIEH8TW2tWcqXQ@x1n/
In summary: I think we can stick with Fixes on e06f1e1dd499, but we don't copy stable. The major reason we don't copy stable here is not only about complexity of such backport, but also that there can be apps trying to pass in unsupported bits (even if the kernel didn't support it) but keep using MISSING mode only, then we shouldn't fail them easily after a stable upgrade. Just smells dangerous to backport.
OK. And I'll move it into the 6.11-rc1 queue, for the next merge window.
linux-kselftest-mirror@lists.linaro.org
-
Andrew Morton -
Audra Mitchell -
Peter Xu