* [bug report] io_uring/zcrx: add support for multiple ifqs
@ 2025-04-23 7:15 Dan Carpenter
2025-04-23 10:04 ` Pavel Begunkov
0 siblings, 1 reply; 5+ messages in thread
From: Dan Carpenter @ 2025-04-23 7:15 UTC (permalink / raw)
To: Pavel Begunkov; +Cc: io-uring
Hello Pavel Begunkov,
Commit 9c2a1c508442 ("io_uring/zcrx: add support for multiple ifqs")
from Apr 20, 2025 (linux-next), leads to the following Smatch static
checker warning:
io_uring/zcrx.c:457 io_register_zcrx_ifq()
error: uninitialized symbol 'id'.
io_uring/zcrx.c
355 int io_register_zcrx_ifq(struct io_ring_ctx *ctx,
356 struct io_uring_zcrx_ifq_reg __user *arg)
357 {
358 struct pp_memory_provider_params mp_param = {};
359 struct io_uring_zcrx_area_reg area;
360 struct io_uring_zcrx_ifq_reg reg;
361 struct io_uring_region_desc rd;
362 struct io_zcrx_ifq *ifq;
363 int ret;
364 u32 id;
365
366 /*
367 * 1. Interface queue allocation.
368 * 2. It can observe data destined for sockets of other tasks.
369 */
370 if (!capable(CAP_NET_ADMIN))
371 return -EPERM;
372
373 /* mandatory io_uring features for zc rx */
374 if (!(ctx->flags & IORING_SETUP_DEFER_TASKRUN &&
375 ctx->flags & IORING_SETUP_CQE32))
376 return -EINVAL;
377 if (copy_from_user(®, arg, sizeof(reg)))
378 return -EFAULT;
379 if (copy_from_user(&rd, u64_to_user_ptr(reg.region_ptr), sizeof(rd)))
380 return -EFAULT;
381 if (memchr_inv(®.__resv, 0, sizeof(reg.__resv)) ||
382 reg.__resv2 || reg.zcrx_id)
383 return -EINVAL;
384 if (reg.if_rxq == -1 || !reg.rq_entries || reg.flags)
385 return -EINVAL;
386 if (reg.rq_entries > IO_RQ_MAX_ENTRIES) {
387 if (!(ctx->flags & IORING_SETUP_CLAMP))
388 return -EINVAL;
389 reg.rq_entries = IO_RQ_MAX_ENTRIES;
390 }
391 reg.rq_entries = roundup_pow_of_two(reg.rq_entries);
392
393 if (copy_from_user(&area, u64_to_user_ptr(reg.area_ptr), sizeof(area)))
394 return -EFAULT;
395
396 ifq = io_zcrx_ifq_alloc(ctx);
397 if (!ifq)
398 return -ENOMEM;
399
400 scoped_guard(mutex, &ctx->mmap_lock) {
401 /* preallocate id */
402 ret = xa_alloc(&ctx->zcrx_ctxs, &id, NULL, xa_limit_31b, GFP_KERNEL);
403 if (ret)
404 goto err;
Potentially uninitialized on this path. Presumably we don't need to
erase id if alloc fails.
405 }
406
407 ret = io_allocate_rbuf_ring(ifq, ®, &rd, id);
408 if (ret)
409 goto err;
410
411 ret = io_zcrx_create_area(ifq, &ifq->area, &area);
412 if (ret)
413 goto err;
414
415 ifq->rq_entries = reg.rq_entries;
416
417 ret = -ENODEV;
418 ifq->netdev = netdev_get_by_index(current->nsproxy->net_ns, reg.if_idx,
419 &ifq->netdev_tracker, GFP_KERNEL);
420 if (!ifq->netdev)
421 goto err;
422
423 ifq->dev = ifq->netdev->dev.parent;
424 ret = -EOPNOTSUPP;
425 if (!ifq->dev)
426 goto err;
427 get_device(ifq->dev);
428
429 mp_param.mp_ops = &io_uring_pp_zc_ops;
430 mp_param.mp_priv = ifq;
431 ret = net_mp_open_rxq(ifq->netdev, reg.if_rxq, &mp_param);
432 if (ret)
433 goto err;
434 ifq->if_rxq = reg.if_rxq;
435
436 reg.offsets.rqes = sizeof(struct io_uring);
437 reg.offsets.head = offsetof(struct io_uring, head);
438 reg.offsets.tail = offsetof(struct io_uring, tail);
439 reg.zcrx_id = id;
440
441 scoped_guard(mutex, &ctx->mmap_lock) {
442 /* publish ifq */
443 ret = -ENOMEM;
444 if (xa_store(&ctx->zcrx_ctxs, id, ifq, GFP_KERNEL))
445 goto err;
446 }
447
448 if (copy_to_user(arg, ®, sizeof(reg)) ||
449 copy_to_user(u64_to_user_ptr(reg.region_ptr), &rd, sizeof(rd)) ||
450 copy_to_user(u64_to_user_ptr(reg.area_ptr), &area, sizeof(area))) {
451 ret = -EFAULT;
452 goto err;
453 }
454 return 0;
455 err:
456 scoped_guard(mutex, &ctx->mmap_lock)
--> 457 xa_erase(&ctx->zcrx_ctxs, id);
^^
458 io_zcrx_ifq_free(ifq);
459 return ret;
460 }
regards,
dan carpenter
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [bug report] io_uring/zcrx: add support for multiple ifqs
2025-04-23 7:15 [bug report] io_uring/zcrx: add support for multiple ifqs Dan Carpenter
@ 2025-04-23 10:04 ` Pavel Begunkov
2025-04-23 10:30 ` Pavel Begunkov
0 siblings, 1 reply; 5+ messages in thread
From: Pavel Begunkov @ 2025-04-23 10:04 UTC (permalink / raw)
To: Dan Carpenter; +Cc: io-uring
On 4/23/25 08:15, Dan Carpenter wrote:
> Hello Pavel Begunkov,
>
> Commit 9c2a1c508442 ("io_uring/zcrx: add support for multiple ifqs")
> from Apr 20, 2025 (linux-next), leads to the following Smatch static
> checker warning:
>
> io_uring/zcrx.c:457 io_register_zcrx_ifq()
> error: uninitialized symbol 'id'.
>
> io_uring/zcrx.c
...
> 396 ifq = io_zcrx_ifq_alloc(ctx);
> 397 if (!ifq)
> 398 return -ENOMEM;
> 399
> 400 scoped_guard(mutex, &ctx->mmap_lock) {
> 401 /* preallocate id */
> 402 ret = xa_alloc(&ctx->zcrx_ctxs, &id, NULL, xa_limit_31b, GFP_KERNEL);
> 403 if (ret)
> 404 goto err;
>
> Potentially uninitialized on this path. Presumably we don't need to
> erase id if alloc fails.
Thanks for letting know
--
Pavel Begunkov
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [bug report] io_uring/zcrx: add support for multiple ifqs
2025-04-23 10:04 ` Pavel Begunkov
@ 2025-04-23 10:30 ` Pavel Begunkov
2025-04-23 13:14 ` Jens Axboe
0 siblings, 1 reply; 5+ messages in thread
From: Pavel Begunkov @ 2025-04-23 10:30 UTC (permalink / raw)
To: Dan Carpenter; +Cc: io-uring
On 4/23/25 11:04, Pavel Begunkov wrote:
> On 4/23/25 08:15, Dan Carpenter wrote:
>> Hello Pavel Begunkov,
>>
>> Commit 9c2a1c508442 ("io_uring/zcrx: add support for multiple ifqs")
>> from Apr 20, 2025 (linux-next), leads to the following Smatch static
>> checker warning:
>>
>> io_uring/zcrx.c:457 io_register_zcrx_ifq()
>> error: uninitialized symbol 'id'.
>>
>> io_uring/zcrx.c
> ...
>> 396 ifq = io_zcrx_ifq_alloc(ctx);
>> 397 if (!ifq)
>> 398 return -ENOMEM;
>> 399
>> 400 scoped_guard(mutex, &ctx->mmap_lock) {
>> 401 /* preallocate id */
>> 402 ret = xa_alloc(&ctx->zcrx_ctxs, &id, NULL, xa_limit_31b, GFP_KERNEL);
>> 403 if (ret)
>> 404 goto err;
>>
>> Potentially uninitialized on this path. Presumably we don't need to
>> erase id if alloc fails.
>
> Thanks for letting know
Jens, do you want a separate patch or to fix it up as it's the top
patch? This should do
diff --git a/io_uring/zcrx.c b/io_uring/zcrx.c
index 39744302fad1..ac2a05364b24 100644
--- a/io_uring/zcrx.c
+++ b/io_uring/zcrx.c
@@ -450,7 +450,7 @@ int io_register_zcrx_ifq(struct io_ring_ctx *ctx,
/* preallocate id */
ret = xa_alloc(&ctx->zcrx_ctxs, &id, NULL, xa_limit_31b, GFP_KERNEL);
if (ret)
- goto err;
+ goto ifq_free;
}
ret = io_allocate_rbuf_ring(ifq, ®, &rd, id);
@@ -506,6 +506,7 @@ int io_register_zcrx_ifq(struct io_ring_ctx *ctx,
err:
scoped_guard(mutex, &ctx->mmap_lock)
xa_erase(&ctx->zcrx_ctxs, id);
+ifq_free:
io_zcrx_ifq_free(ifq);
return ret;
}
--
Pavel Begunkov
^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [bug report] io_uring/zcrx: add support for multiple ifqs
2025-04-23 10:30 ` Pavel Begunkov
@ 2025-04-23 13:14 ` Jens Axboe
2025-04-23 15:59 ` Pavel Begunkov
0 siblings, 1 reply; 5+ messages in thread
From: Jens Axboe @ 2025-04-23 13:14 UTC (permalink / raw)
To: Pavel Begunkov, Dan Carpenter; +Cc: io-uring
On 4/23/25 4:30 AM, Pavel Begunkov wrote:
> On 4/23/25 11:04, Pavel Begunkov wrote:
>> On 4/23/25 08:15, Dan Carpenter wrote:
>>> Hello Pavel Begunkov,
>>>
>>> Commit 9c2a1c508442 ("io_uring/zcrx: add support for multiple ifqs")
>>> from Apr 20, 2025 (linux-next), leads to the following Smatch static
>>> checker warning:
>>>
>>> io_uring/zcrx.c:457 io_register_zcrx_ifq()
>>> error: uninitialized symbol 'id'.
>>>
>>> io_uring/zcrx.c
>> ...
>>> 396 ifq = io_zcrx_ifq_alloc(ctx);
>>> 397 if (!ifq)
>>> 398 return -ENOMEM;
>>> 399
>>> 400 scoped_guard(mutex, &ctx->mmap_lock) {
>>> 401 /* preallocate id */
>>> 402 ret = xa_alloc(&ctx->zcrx_ctxs, &id, NULL, xa_limit_31b, GFP_KERNEL);
>>> 403 if (ret)
>>> 404 goto err;
>>>
>>> Potentially uninitialized on this path. Presumably we don't need to
>>> erase id if alloc fails.
>>
>> Thanks for letting know
>
> Jens, do you want a separate patch or to fix it up as it's the top
> patch? This should do
I can just fold it in - done!
--
Jens Axboe
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [bug report] io_uring/zcrx: add support for multiple ifqs
2025-04-23 13:14 ` Jens Axboe
@ 2025-04-23 15:59 ` Pavel Begunkov
0 siblings, 0 replies; 5+ messages in thread
From: Pavel Begunkov @ 2025-04-23 15:59 UTC (permalink / raw)
To: Jens Axboe, Dan Carpenter; +Cc: io-uring
On 4/23/25 14:14, Jens Axboe wrote:
> On 4/23/25 4:30 AM, Pavel Begunkov wrote:
>> On 4/23/25 11:04, Pavel Begunkov wrote:
>>> On 4/23/25 08:15, Dan Carpenter wrote:
>>>> Hello Pavel Begunkov,
>>>>
>>>> Commit 9c2a1c508442 ("io_uring/zcrx: add support for multiple ifqs")
>>>> from Apr 20, 2025 (linux-next), leads to the following Smatch static
>>>> checker warning:
>>>>
>>>> io_uring/zcrx.c:457 io_register_zcrx_ifq()
>>>> error: uninitialized symbol 'id'.
>>>>
>>>> io_uring/zcrx.c
>>> ...
>>>> 396 ifq = io_zcrx_ifq_alloc(ctx);
>>>> 397 if (!ifq)
>>>> 398 return -ENOMEM;
>>>> 399
>>>> 400 scoped_guard(mutex, &ctx->mmap_lock) {
>>>> 401 /* preallocate id */
>>>> 402 ret = xa_alloc(&ctx->zcrx_ctxs, &id, NULL, xa_limit_31b, GFP_KERNEL);
>>>> 403 if (ret)
>>>> 404 goto err;
>>>>
>>>> Potentially uninitialized on this path. Presumably we don't need to
>>>> erase id if alloc fails.
>>>
>>> Thanks for letting know
>>
>> Jens, do you want a separate patch or to fix it up as it's the top
>> patch? This should do
>
> I can just fold it in - done!
Great, thanks!
--
Pavel Begunkov
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2025-04-23 15:58 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-04-23 7:15 [bug report] io_uring/zcrx: add support for multiple ifqs Dan Carpenter
2025-04-23 10:04 ` Pavel Begunkov
2025-04-23 10:30 ` Pavel Begunkov
2025-04-23 13:14 ` Jens Axboe
2025-04-23 15:59 ` Pavel Begunkov
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox