* [RFC PATCH 0/1] IDR fix for potential id mismatch
@ 2025-11-27 9:27 Jan Sokolowski
2025-11-27 9:27 ` [RFC PATCH 1/1] idr: do not create idr if new id would be outside given range Jan Sokolowski
0 siblings, 1 reply; 12+ messages in thread
From: Jan Sokolowski @ 2025-11-27 9:27 UTC (permalink / raw)
To: linux-kernel
Cc: Jan Sokolowski, Christian König, Matthew Wilcox,
Andrew Morton, linux-fsdevel, linux-mm
When debugging an issue found in drm subsystem (link to the
discussion in Link tag), a bug was found in idr library
where requesting id in range would return id outside
requested range. Didn't see in documentation that this is how
idr should behave.
This is an RFC as this library is deprecated but still in use by other
subsystems. Is this fix proper?
Link: https://lists.freedesktop.org/archives/dri-devel/2025-November/538294.html
Cc: "Christian König" <christian.koenig@amd.com>
Cc: Matthew Wilcox <willy@infradead.org>
Cc: Andrew Morton <akpm@linux-foundation.org>
Cc: linux-fsdevel@vger.kernel.org
Cc: linux-mm@kvack.org
Cc: linux-kernel@vger.kernel.org
---
Jan Sokolowski (1):
idr: do not create idr if new id would be outside given range
lib/idr.c | 15 +++++++++++++++
1 file changed, 15 insertions(+)
--
2.43.0
^ permalink raw reply [flat|nested] 12+ messages in thread
* [RFC PATCH 1/1] idr: do not create idr if new id would be outside given range
2025-11-27 9:27 [RFC PATCH 0/1] IDR fix for potential id mismatch Jan Sokolowski
@ 2025-11-27 9:27 ` Jan Sokolowski
2025-11-27 13:38 ` Matthew Wilcox
2025-11-27 13:54 ` Matthew Wilcox
0 siblings, 2 replies; 12+ messages in thread
From: Jan Sokolowski @ 2025-11-27 9:27 UTC (permalink / raw)
To: linux-kernel
Cc: Jan Sokolowski, Christian König, Matthew Wilcox,
Andrew Morton, linux-fsdevel, linux-mm
A scenario was found where trying to add id in range 0,1
would return an id of 2, which is outside the range and thus
now what the user would expect.
Return -EINVAL if new id would fall outside the range.
Signed-off-by: Jan Sokolowski <jan.sokolowski@intel.com>
Cc: "Christian König" <christian.koenig@amd.com>
Cc: Matthew Wilcox <willy@infradead.org>
Cc: Andrew Morton <akpm@linux-foundation.org>
Cc: linux-fsdevel@vger.kernel.org
Cc: linux-mm@kvack.org
Cc: linux-kernel@vger.kernel.org
---
lib/idr.c | 12 ++++++++++++
1 file changed, 12 insertions(+)
diff --git a/lib/idr.c b/lib/idr.c
index e2adc457abb4..8c786e50f2da 100644
--- a/lib/idr.c
+++ b/lib/idr.c
@@ -74,6 +74,7 @@ EXPORT_SYMBOL_GPL(idr_alloc_u32);
* exclude simultaneous writers.
*
* Return: The newly allocated ID, -ENOMEM if memory allocation failed,
+ * -EINVAL is start value is less than 0 or if new id would be in wrong range,
* or -ENOSPC if no free IDs could be found.
*/
int idr_alloc(struct idr *idr, void *ptr, int start, int end, gfp_t gfp)
@@ -88,6 +89,11 @@ int idr_alloc(struct idr *idr, void *ptr, int start, int end, gfp_t gfp)
if (ret)
return ret;
+ if (WARN_ON_ONCE(id < start || (id >= end && end != 0))) {
+ idr_remove(idr, id);
+ return -EINVAL;
+ }
+
return id;
}
EXPORT_SYMBOL_GPL(idr_alloc);
@@ -112,6 +118,7 @@ EXPORT_SYMBOL_GPL(idr_alloc);
* exclude simultaneous writers.
*
* Return: The newly allocated ID, -ENOMEM if memory allocation failed,
+ * -EINVAL if new id would be in wrong range,
* or -ENOSPC if no free IDs could be found.
*/
int idr_alloc_cyclic(struct idr *idr, void *ptr, int start, int end, gfp_t gfp)
@@ -130,6 +137,11 @@ int idr_alloc_cyclic(struct idr *idr, void *ptr, int start, int end, gfp_t gfp)
if (err)
return err;
+ if (WARN_ON_ONCE(id < start || (id >= end && end != 0))) {
+ idr_remove(idr, id);
+ return -EINVAL;
+ }
+
idr->idr_next = id + 1;
return id;
}
--
2.43.0
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [RFC PATCH 1/1] idr: do not create idr if new id would be outside given range
2025-11-27 9:27 ` [RFC PATCH 1/1] idr: do not create idr if new id would be outside given range Jan Sokolowski
@ 2025-11-27 13:38 ` Matthew Wilcox
2025-11-27 13:54 ` Matthew Wilcox
1 sibling, 0 replies; 12+ messages in thread
From: Matthew Wilcox @ 2025-11-27 13:38 UTC (permalink / raw)
To: Jan Sokolowski
Cc: linux-kernel, Christian König, Andrew Morton, linux-fsdevel,
linux-mm
On Thu, Nov 27, 2025 at 10:27:32AM +0100, Jan Sokolowski wrote:
> @@ -88,6 +89,11 @@ int idr_alloc(struct idr *idr, void *ptr, int start, int end, gfp_t gfp)
> if (ret)
> return ret;
>
> + if (WARN_ON_ONCE(id < start || (id >= end && end != 0))) {
> + idr_remove(idr, id);
> + return -EINVAL;
> + }
This is certainly the wrong way to fix any problem that does exist.
And it should return -ENOSPC, not -EINVAL. -EINVAL is "the arguments
are wrong", not "the data structure is full".
I'll go read the thread now.
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [RFC PATCH 1/1] idr: do not create idr if new id would be outside given range
2025-11-27 9:27 ` [RFC PATCH 1/1] idr: do not create idr if new id would be outside given range Jan Sokolowski
2025-11-27 13:38 ` Matthew Wilcox
@ 2025-11-27 13:54 ` Matthew Wilcox
2025-11-27 14:03 ` Christian König
1 sibling, 1 reply; 12+ messages in thread
From: Matthew Wilcox @ 2025-11-27 13:54 UTC (permalink / raw)
To: Jan Sokolowski
Cc: linux-kernel, Christian König, Andrew Morton, linux-fsdevel,
linux-mm
On Thu, Nov 27, 2025 at 10:27:32AM +0100, Jan Sokolowski wrote:
> A scenario was found where trying to add id in range 0,1
> would return an id of 2, which is outside the range and thus
> now what the user would expect.
Can you do a bit better with this bug report? Under what circumstances
does this happen? Preferably answer in the form of a test case for the
IDR test suite. Here's my attempt to recreate your situation based on
what I read in that thread. It doesn't show a problem, so clearly I got
something wrong.
To run the test suite, apply this patch, then
$ make -C tools/testing/radix-tree
$ ./tools/testing/radix-tree/idr-test
diff --git a/tools/testing/radix-tree/idr-test.c b/tools/testing/radix-tree/idr-test.c
index 2f830ff8396c..774c0c9c141f 100644
--- a/tools/testing/radix-tree/idr-test.c
+++ b/tools/testing/radix-tree/idr-test.c
@@ -57,6 +57,21 @@ void idr_alloc_test(void)
idr_destroy(&idr);
}
+void idr_alloc2_test(void)
+{
+ int id;
+ DEFINE_IDR(idr);
+
+ id = idr_alloc(&idr, idr_alloc2_test, 0, 1, GFP_KERNEL);
+ printf("id = %d\n", id);
+ assert(id == 0);
+ id = idr_alloc(&idr, idr_alloc2_test, 0, 1, GFP_KERNEL);
+ printf("id = %d\n", id);
+ assert(id == -ENOSPC);
+
+ idr_destroy(&idr);
+}
+
void idr_replace_test(void)
{
DEFINE_IDR(idr);
@@ -409,6 +424,7 @@ void idr_checks(void)
idr_replace_test();
idr_alloc_test();
+ idr_alloc2_test();
idr_null_test();
idr_nowait_test();
idr_get_next_test(0);
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [RFC PATCH 1/1] idr: do not create idr if new id would be outside given range
2025-11-27 13:54 ` Matthew Wilcox
@ 2025-11-27 14:03 ` Christian König
2025-11-27 14:11 ` Matthew Wilcox
0 siblings, 1 reply; 12+ messages in thread
From: Christian König @ 2025-11-27 14:03 UTC (permalink / raw)
To: Matthew Wilcox, Jan Sokolowski
Cc: linux-kernel, Andrew Morton, linux-fsdevel, linux-mm
On 11/27/25 14:54, Matthew Wilcox wrote:
> On Thu, Nov 27, 2025 at 10:27:32AM +0100, Jan Sokolowski wrote:
>> A scenario was found where trying to add id in range 0,1
>> would return an id of 2, which is outside the range and thus
>> now what the user would expect.
>
> Can you do a bit better with this bug report? Under what circumstances
> does this happen? Preferably answer in the form of a test case for the
> IDR test suite. Here's my attempt to recreate your situation based on
> what I read in that thread. It doesn't show a problem, so clearly I got
> something wrong.
According to Jan the observation he has is that this code:
idr_init_base(&idr, 1);
id = idr_alloc(&idr, dummy_ptr, 0, 1, GFP_KERNEL);
Gives him id=2 in return.
That clearly seems to be problematic considering that start=0 and end=1 should either give you id=0 or an error because idr->idr_base should be initialized to 1.
But I'm still not sure if Jan's observation is actually correct, cause I also don't see how that possible happen.
Regards,
Christian.
> To run the test suite, apply this patch, then
>
> $ make -C tools/testing/radix-tree
> $ ./tools/testing/radix-tree/idr-test
>
> diff --git a/tools/testing/radix-tree/idr-test.c b/tools/testing/radix-tree/idr-test.c
> index 2f830ff8396c..774c0c9c141f 100644
> --- a/tools/testing/radix-tree/idr-test.c
> +++ b/tools/testing/radix-tree/idr-test.c
> @@ -57,6 +57,21 @@ void idr_alloc_test(void)
> idr_destroy(&idr);
> }
>
> +void idr_alloc2_test(void)
> +{
> + int id;
> + DEFINE_IDR(idr);
> +
> + id = idr_alloc(&idr, idr_alloc2_test, 0, 1, GFP_KERNEL);
> + printf("id = %d\n", id);
> + assert(id == 0);
> + id = idr_alloc(&idr, idr_alloc2_test, 0, 1, GFP_KERNEL);
> + printf("id = %d\n", id);
> + assert(id == -ENOSPC);
> +
> + idr_destroy(&idr);
> +}
> +
> void idr_replace_test(void)
> {
> DEFINE_IDR(idr);
> @@ -409,6 +424,7 @@ void idr_checks(void)
>
> idr_replace_test();
> idr_alloc_test();
> + idr_alloc2_test();
> idr_null_test();
> idr_nowait_test();
> idr_get_next_test(0);
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [RFC PATCH 1/1] idr: do not create idr if new id would be outside given range
2025-11-27 14:03 ` Christian König
@ 2025-11-27 14:11 ` Matthew Wilcox
2025-11-27 14:55 ` Matthew Wilcox
0 siblings, 1 reply; 12+ messages in thread
From: Matthew Wilcox @ 2025-11-27 14:11 UTC (permalink / raw)
To: Christian König
Cc: Jan Sokolowski, linux-kernel, Andrew Morton, linux-fsdevel, linux-mm
On Thu, Nov 27, 2025 at 03:03:20PM +0100, Christian König wrote:
> On 11/27/25 14:54, Matthew Wilcox wrote:
> > On Thu, Nov 27, 2025 at 10:27:32AM +0100, Jan Sokolowski wrote:
> >> A scenario was found where trying to add id in range 0,1
> >> would return an id of 2, which is outside the range and thus
> >> now what the user would expect.
> >
> > Can you do a bit better with this bug report? Under what circumstances
> > does this happen? Preferably answer in the form of a test case for the
> > IDR test suite. Here's my attempt to recreate your situation based on
> > what I read in that thread. It doesn't show a problem, so clearly I got
> > something wrong.
>
> According to Jan the observation he has is that this code:
>
> idr_init_base(&idr, 1);
> id = idr_alloc(&idr, dummy_ptr, 0, 1, GFP_KERNEL);
>
> Gives him id=2 in return.
Hm. That's not what it does for me. It gives me id == 1, which isn't
correct! I'll look into that, but it'd be helpful to know what
combination of inputs gives us 2.
To be completely clear, here's what I'm looking at:
+void idr_alloc2_test(void)
+{
+ int id;
+ struct idr idr = IDR_INIT_BASE(idr, 1);
+
+ id = idr_alloc(&idr, idr_alloc2_test, 0, 1, GFP_KERNEL);
+ printf("id = %d\n", id);
and I think that should return -ENOSPC instead of 1, since we told it to
allocate exclusive of 1.
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [RFC PATCH 1/1] idr: do not create idr if new id would be outside given range
2025-11-27 14:11 ` Matthew Wilcox
@ 2025-11-27 14:55 ` Matthew Wilcox
2025-11-27 15:02 ` Christian König
2025-11-28 9:03 ` Sokolowski, Jan
0 siblings, 2 replies; 12+ messages in thread
From: Matthew Wilcox @ 2025-11-27 14:55 UTC (permalink / raw)
To: Christian König
Cc: Jan Sokolowski, linux-kernel, Andrew Morton, linux-fsdevel, linux-mm
On Thu, Nov 27, 2025 at 02:11:02PM +0000, Matthew Wilcox wrote:
> Hm. That's not what it does for me. It gives me id == 1, which isn't
> correct! I'll look into that, but it'd be helpful to know what
> combination of inputs gives us 2.
Oh, never mind, I see what's happening.
int idr_alloc(struct idr *idr, void *ptr, int start, int end, gfp_t gfp)
ret = idr_alloc_u32(idr, ptr, &id, end > 0 ? end - 1 : INT_MAX, gfp);
so it's passing 0 as 'max' to idr_alloc_u32() which does:
slot = idr_get_free(&idr->idr_rt, &iter, gfp, max - base);
and max - base becomes -1 or rather ULONG_MAX, and so we'll literally
allocate any number. If the first slot is full, we'll get back 1
and then add 'base' to it, giving 2.
Here's the new test-case:
+void idr_alloc2_test(void)
+{
+ int id;
+ struct idr idr = IDR_INIT_BASE(idr, 1);
+
+ id = idr_alloc(&idr, idr_alloc2_test, 0, 1, GFP_KERNEL);
+ assert(id == -ENOSPC);
+
+ id = idr_alloc(&idr, idr_alloc2_test, 1, 2, GFP_KERNEL);
+ assert(id == 1);
+
+ id = idr_alloc(&idr, idr_alloc2_test, 0, 1, GFP_KERNEL);
+ assert(id == -ENOSPC);
+
+ id = idr_alloc(&idr, idr_alloc2_test, 0, 2, GFP_KERNEL);
+ assert(id == -ENOSPC);
+
+ idr_destroy(&idr);
+}
and with this patch, it passes:
+++ b/lib/idr.c
@@ -40,6 +40,8 @@ int idr_alloc_u32(struct idr *idr, void *ptr, u32 *nextid,
if (WARN_ON_ONCE(!(idr->idr_rt.xa_flags & ROOT_IS_IDR)))
idr->idr_rt.xa_flags |= IDR_RT_MARKER;
+ if (max < base)
+ return -ENOSPC;
id = (id < base) ? 0 : id - base;
radix_tree_iter_init(&iter, id);
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [RFC PATCH 1/1] idr: do not create idr if new id would be outside given range
2025-11-27 14:55 ` Matthew Wilcox
@ 2025-11-27 15:02 ` Christian König
2025-11-28 9:03 ` Sokolowski, Jan
1 sibling, 0 replies; 12+ messages in thread
From: Christian König @ 2025-11-27 15:02 UTC (permalink / raw)
To: Matthew Wilcox
Cc: Jan Sokolowski, linux-kernel, Andrew Morton, linux-fsdevel, linux-mm
On 11/27/25 15:55, Matthew Wilcox wrote:
> On Thu, Nov 27, 2025 at 02:11:02PM +0000, Matthew Wilcox wrote:
>> Hm. That's not what it does for me. It gives me id == 1, which isn't
>> correct! I'll look into that, but it'd be helpful to know what
>> combination of inputs gives us 2.
>
> Oh, never mind, I see what's happening.
>
> int idr_alloc(struct idr *idr, void *ptr, int start, int end, gfp_t gfp)
>
> ret = idr_alloc_u32(idr, ptr, &id, end > 0 ? end - 1 : INT_MAX, gfp);
> so it's passing 0 as 'max' to idr_alloc_u32() which does:
>
> slot = idr_get_free(&idr->idr_rt, &iter, gfp, max - base);
>
> and max - base becomes -1 or rather ULONG_MAX, and so we'll literally
> allocate any number. If the first slot is full, we'll get back 1
> and then add 'base' to it, giving 2.
Oh wow, not every day that we stumble over a bug in such a core kernel functionality.
> Here's the new test-case:
>
> +void idr_alloc2_test(void)
> +{
> + int id;
> + struct idr idr = IDR_INIT_BASE(idr, 1);
> +
> + id = idr_alloc(&idr, idr_alloc2_test, 0, 1, GFP_KERNEL);
> + assert(id == -ENOSPC);
> +
> + id = idr_alloc(&idr, idr_alloc2_test, 1, 2, GFP_KERNEL);
> + assert(id == 1);
> +
> + id = idr_alloc(&idr, idr_alloc2_test, 0, 1, GFP_KERNEL);
> + assert(id == -ENOSPC);
> +
> + id = idr_alloc(&idr, idr_alloc2_test, 0, 2, GFP_KERNEL);
> + assert(id == -ENOSPC);
> +
> + idr_destroy(&idr);
> +}
>
> and with this patch, it passes:
>
> +++ b/lib/idr.c
> @@ -40,6 +40,8 @@ int idr_alloc_u32(struct idr *idr, void *ptr, u32 *nextid,
>
> if (WARN_ON_ONCE(!(idr->idr_rt.xa_flags & ROOT_IS_IDR)))
> idr->idr_rt.xa_flags |= IDR_RT_MARKER;
> + if (max < base)
> + return -ENOSPC;
>
> id = (id < base) ? 0 : id - base;
> radix_tree_iter_init(&iter, id);
>
Not sure what it's worth but feel free to add Reviewed-by: Christian König <christian.koenig@amd.com> to both the test case and the solution.
Thanks for the quick help,
Christian.
^ permalink raw reply [flat|nested] 12+ messages in thread
* RE: [RFC PATCH 1/1] idr: do not create idr if new id would be outside given range
2025-11-27 14:55 ` Matthew Wilcox
2025-11-27 15:02 ` Christian König
@ 2025-11-28 9:03 ` Sokolowski, Jan
2025-11-28 15:52 ` Matthew Wilcox
1 sibling, 1 reply; 12+ messages in thread
From: Sokolowski, Jan @ 2025-11-28 9:03 UTC (permalink / raw)
To: Matthew Wilcox, Christian König
Cc: linux-kernel, Andrew Morton, linux-fsdevel, linux-mm
So, shall I send a V2 of that patch and add you as co-developer there?
Regards
Jan
> -----Original Message-----
> From: Matthew Wilcox <willy@infradead.org>
> Sent: Thursday, November 27, 2025 3:55 PM
> To: Christian König <christian.koenig@amd.com>
> Cc: Sokolowski, Jan <jan.sokolowski@intel.com>; linux-
> kernel@vger.kernel.org; Andrew Morton <akpm@linux-foundation.org>;
> linux-fsdevel@vger.kernel.org; linux-mm@kvack.org
> Subject: Re: [RFC PATCH 1/1] idr: do not create idr if new id would be outside
> given range
>
> On Thu, Nov 27, 2025 at 02:11:02PM +0000, Matthew Wilcox wrote:
> > Hm. That's not what it does for me. It gives me id == 1, which isn't
> > correct! I'll look into that, but it'd be helpful to know what
> > combination of inputs gives us 2.
>
> Oh, never mind, I see what's happening.
>
> int idr_alloc(struct idr *idr, void *ptr, int start, int end, gfp_t gfp)
>
> ret = idr_alloc_u32(idr, ptr, &id, end > 0 ? end - 1 : INT_MAX, gfp);
> so it's passing 0 as 'max' to idr_alloc_u32() which does:
>
> slot = idr_get_free(&idr->idr_rt, &iter, gfp, max - base);
>
> and max - base becomes -1 or rather ULONG_MAX, and so we'll literally
> allocate any number. If the first slot is full, we'll get back 1
> and then add 'base' to it, giving 2.
>
> Here's the new test-case:
>
> +void idr_alloc2_test(void)
> +{
> + int id;
> + struct idr idr = IDR_INIT_BASE(idr, 1);
> +
> + id = idr_alloc(&idr, idr_alloc2_test, 0, 1, GFP_KERNEL);
> + assert(id == -ENOSPC);
> +
> + id = idr_alloc(&idr, idr_alloc2_test, 1, 2, GFP_KERNEL);
> + assert(id == 1);
> +
> + id = idr_alloc(&idr, idr_alloc2_test, 0, 1, GFP_KERNEL);
> + assert(id == -ENOSPC);
> +
> + id = idr_alloc(&idr, idr_alloc2_test, 0, 2, GFP_KERNEL);
> + assert(id == -ENOSPC);
> +
> + idr_destroy(&idr);
> +}
>
> and with this patch, it passes:
>
> +++ b/lib/idr.c
> @@ -40,6 +40,8 @@ int idr_alloc_u32(struct idr *idr, void *ptr, u32 *nextid,
>
> if (WARN_ON_ONCE(!(idr->idr_rt.xa_flags & ROOT_IS_IDR)))
> idr->idr_rt.xa_flags |= IDR_RT_MARKER;
> + if (max < base)
> + return -ENOSPC;
>
> id = (id < base) ? 0 : id - base;
> radix_tree_iter_init(&iter, id);
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [RFC PATCH 1/1] idr: do not create idr if new id would be outside given range
2025-11-28 9:03 ` Sokolowski, Jan
@ 2025-11-28 15:52 ` Matthew Wilcox
2025-11-28 16:47 ` Sokolowski, Jan
0 siblings, 1 reply; 12+ messages in thread
From: Matthew Wilcox @ 2025-11-28 15:52 UTC (permalink / raw)
To: Sokolowski, Jan
Cc: Christian König, linux-kernel, Andrew Morton, linux-fsdevel,
linux-mm
On Fri, Nov 28, 2025 at 09:03:08AM +0000, Sokolowski, Jan wrote:
> So, shall I send a V2 of that patch and add you as co-developer there?
No. You didn't co-develop anything. You reported the bug, badly.
What I'm trying to do right now is figure out what the syzbot report
actually was. In all the DRM specialness, you've lost the original
information, so I can't add the original syzbot links. All I can find
is https://gitlab.freedesktop.org/drm/xe/kernel/-/issues/6449
which doesn't link to a syzbot report, so that's a dead end.
> Regards
> Jan
>
> > -----Original Message-----
> > From: Matthew Wilcox <willy@infradead.org>
> > Sent: Thursday, November 27, 2025 3:55 PM
> > To: Christian König <christian.koenig@amd.com>
> > Cc: Sokolowski, Jan <jan.sokolowski@intel.com>; linux-
> > kernel@vger.kernel.org; Andrew Morton <akpm@linux-foundation.org>;
> > linux-fsdevel@vger.kernel.org; linux-mm@kvack.org
> > Subject: Re: [RFC PATCH 1/1] idr: do not create idr if new id would be outside
> > given range
> >
> > On Thu, Nov 27, 2025 at 02:11:02PM +0000, Matthew Wilcox wrote:
> > > Hm. That's not what it does for me. It gives me id == 1, which isn't
> > > correct! I'll look into that, but it'd be helpful to know what
> > > combination of inputs gives us 2.
> >
> > Oh, never mind, I see what's happening.
> >
> > int idr_alloc(struct idr *idr, void *ptr, int start, int end, gfp_t gfp)
> >
> > ret = idr_alloc_u32(idr, ptr, &id, end > 0 ? end - 1 : INT_MAX, gfp);
> > so it's passing 0 as 'max' to idr_alloc_u32() which does:
> >
> > slot = idr_get_free(&idr->idr_rt, &iter, gfp, max - base);
> >
> > and max - base becomes -1 or rather ULONG_MAX, and so we'll literally
> > allocate any number. If the first slot is full, we'll get back 1
> > and then add 'base' to it, giving 2.
> >
> > Here's the new test-case:
> >
> > +void idr_alloc2_test(void)
> > +{
> > + int id;
> > + struct idr idr = IDR_INIT_BASE(idr, 1);
> > +
> > + id = idr_alloc(&idr, idr_alloc2_test, 0, 1, GFP_KERNEL);
> > + assert(id == -ENOSPC);
> > +
> > + id = idr_alloc(&idr, idr_alloc2_test, 1, 2, GFP_KERNEL);
> > + assert(id == 1);
> > +
> > + id = idr_alloc(&idr, idr_alloc2_test, 0, 1, GFP_KERNEL);
> > + assert(id == -ENOSPC);
> > +
> > + id = idr_alloc(&idr, idr_alloc2_test, 0, 2, GFP_KERNEL);
> > + assert(id == -ENOSPC);
> > +
> > + idr_destroy(&idr);
> > +}
> >
> > and with this patch, it passes:
> >
> > +++ b/lib/idr.c
> > @@ -40,6 +40,8 @@ int idr_alloc_u32(struct idr *idr, void *ptr, u32 *nextid,
> >
> > if (WARN_ON_ONCE(!(idr->idr_rt.xa_flags & ROOT_IS_IDR)))
> > idr->idr_rt.xa_flags |= IDR_RT_MARKER;
> > + if (max < base)
> > + return -ENOSPC;
> >
> > id = (id < base) ? 0 : id - base;
> > radix_tree_iter_init(&iter, id);
>
^ permalink raw reply [flat|nested] 12+ messages in thread
* RE: [RFC PATCH 1/1] idr: do not create idr if new id would be outside given range
2025-11-28 15:52 ` Matthew Wilcox
@ 2025-11-28 16:47 ` Sokolowski, Jan
2025-11-28 17:50 ` Matthew Wilcox
0 siblings, 1 reply; 12+ messages in thread
From: Sokolowski, Jan @ 2025-11-28 16:47 UTC (permalink / raw)
To: Matthew Wilcox
Cc: Christian König, linux-kernel, Andrew Morton, linux-fsdevel,
linux-mm
> -----Original Message-----
> From: Matthew Wilcox <willy@infradead.org>
> Sent: Friday, November 28, 2025 4:52 PM
> To: Sokolowski, Jan <jan.sokolowski@intel.com>
> Cc: Christian König <christian.koenig@amd.com>; linux-
> kernel@vger.kernel.org; Andrew Morton <akpm@linux-foundation.org>;
> linux-fsdevel@vger.kernel.org; linux-mm@kvack.org
> Subject: Re: [RFC PATCH 1/1] idr: do not create idr if new id would be outside
> given range
>
> On Fri, Nov 28, 2025 at 09:03:08AM +0000, Sokolowski, Jan wrote:
> > So, shall I send a V2 of that patch and add you as co-developer there?
>
> No. You didn't co-develop anything. You reported the bug, badly.
>
And I've sent a potential patch on how it should've been fixed. That should count for something, right?
> What I'm trying to do right now is figure out what the syzbot report
> actually was. In all the DRM specialness, you've lost the original
> information, so I can't add the original syzbot links. All I can find
> is https://gitlab.freedesktop.org/drm/xe/kernel/-/issues/6449
> which doesn't link to a syzbot report, so that's a dead end.
>
> > Regards
> > Jan
> >
> > > -----Original Message-----
> > > From: Matthew Wilcox <willy@infradead.org>
> > > Sent: Thursday, November 27, 2025 3:55 PM
> > > To: Christian König <christian.koenig@amd.com>
> > > Cc: Sokolowski, Jan <jan.sokolowski@intel.com>; linux-
> > > kernel@vger.kernel.org; Andrew Morton <akpm@linux-foundation.org>;
> > > linux-fsdevel@vger.kernel.org; linux-mm@kvack.org
> > > Subject: Re: [RFC PATCH 1/1] idr: do not create idr if new id would be
> outside
> > > given range
> > >
> > > On Thu, Nov 27, 2025 at 02:11:02PM +0000, Matthew Wilcox wrote:
> > > > Hm. That's not what it does for me. It gives me id == 1, which isn't
> > > > correct! I'll look into that, but it'd be helpful to know what
> > > > combination of inputs gives us 2.
> > >
> > > Oh, never mind, I see what's happening.
> > >
> > > int idr_alloc(struct idr *idr, void *ptr, int start, int end, gfp_t gfp)
> > >
> > > ret = idr_alloc_u32(idr, ptr, &id, end > 0 ? end - 1 : INT_MAX, gfp);
> > > so it's passing 0 as 'max' to idr_alloc_u32() which does:
> > >
> > > slot = idr_get_free(&idr->idr_rt, &iter, gfp, max - base);
> > >
> > > and max - base becomes -1 or rather ULONG_MAX, and so we'll literally
> > > allocate any number. If the first slot is full, we'll get back 1
> > > and then add 'base' to it, giving 2.
> > >
> > > Here's the new test-case:
> > >
> > > +void idr_alloc2_test(void)
> > > +{
> > > + int id;
> > > + struct idr idr = IDR_INIT_BASE(idr, 1);
> > > +
> > > + id = idr_alloc(&idr, idr_alloc2_test, 0, 1, GFP_KERNEL);
> > > + assert(id == -ENOSPC);
> > > +
> > > + id = idr_alloc(&idr, idr_alloc2_test, 1, 2, GFP_KERNEL);
> > > + assert(id == 1);
> > > +
> > > + id = idr_alloc(&idr, idr_alloc2_test, 0, 1, GFP_KERNEL);
> > > + assert(id == -ENOSPC);
> > > +
> > > + id = idr_alloc(&idr, idr_alloc2_test, 0, 2, GFP_KERNEL);
> > > + assert(id == -ENOSPC);
> > > +
> > > + idr_destroy(&idr);
> > > +}
> > >
> > > and with this patch, it passes:
> > >
> > > +++ b/lib/idr.c
> > > @@ -40,6 +40,8 @@ int idr_alloc_u32(struct idr *idr, void *ptr, u32
> *nextid,
> > >
> > > if (WARN_ON_ONCE(!(idr->idr_rt.xa_flags & ROOT_IS_IDR)))
> > > idr->idr_rt.xa_flags |= IDR_RT_MARKER;
> > > + if (max < base)
> > > + return -ENOSPC;
> > >
> > > id = (id < base) ? 0 : id - base;
> > > radix_tree_iter_init(&iter, id);
> >
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [RFC PATCH 1/1] idr: do not create idr if new id would be outside given range
2025-11-28 16:47 ` Sokolowski, Jan
@ 2025-11-28 17:50 ` Matthew Wilcox
0 siblings, 0 replies; 12+ messages in thread
From: Matthew Wilcox @ 2025-11-28 17:50 UTC (permalink / raw)
To: Sokolowski, Jan
Cc: Christian König, linux-kernel, Andrew Morton, linux-fsdevel,
linux-mm
On Fri, Nov 28, 2025 at 04:47:17PM +0000, Sokolowski, Jan wrote:
> > No. You didn't co-develop anything. You reported the bug, badly.
> >
> And I've sent a potential patch on how it should've been fixed. That should count for something, right?
Literally everything about that patch was wrong. If I'd used any of it,
you'd have a point, but the entire approach was wrong.
You _can't_allow the allocation to succeed and then undo it. There might
be an RCU-protected reader which would see the intermediate state.
And that's an inconsistency we guarantee can't happen; an RCU reader
can see the state before the lock, the state after the lock. It must
not see a state that never happened.
If you'd written a test case, I'd happily add a co-developed-by tag.
But you didn't do that either.
^ permalink raw reply [flat|nested] 12+ messages in thread
end of thread, other threads:[~2025-11-28 17:50 UTC | newest]
Thread overview: 12+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2025-11-27 9:27 [RFC PATCH 0/1] IDR fix for potential id mismatch Jan Sokolowski
2025-11-27 9:27 ` [RFC PATCH 1/1] idr: do not create idr if new id would be outside given range Jan Sokolowski
2025-11-27 13:38 ` Matthew Wilcox
2025-11-27 13:54 ` Matthew Wilcox
2025-11-27 14:03 ` Christian König
2025-11-27 14:11 ` Matthew Wilcox
2025-11-27 14:55 ` Matthew Wilcox
2025-11-27 15:02 ` Christian König
2025-11-28 9:03 ` Sokolowski, Jan
2025-11-28 15:52 ` Matthew Wilcox
2025-11-28 16:47 ` Sokolowski, Jan
2025-11-28 17:50 ` Matthew Wilcox
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox