* [PATCH] io_uring: Fix use of uninitialized ret in io_eventfd_register()
@ 2022-02-07 16:24 Nathan Chancellor
2022-02-07 19:32 ` Nick Desaulniers
` (2 more replies)
0 siblings, 3 replies; 6+ messages in thread
From: Nathan Chancellor @ 2022-02-07 16:24 UTC (permalink / raw)
To: Jens Axboe, Pavel Begunkov
Cc: Nick Desaulniers, Usama Arif, io-uring, linux-kernel, llvm,
Nathan Chancellor
Clang warns:
fs/io_uring.c:9396:9: warning: variable 'ret' is uninitialized when used here [-Wuninitialized]
return ret;
^~~
fs/io_uring.c:9373:13: note: initialize the variable 'ret' to silence this warning
int fd, ret;
^
= 0
1 warning generated.
Just return 0 directly and reduce the scope of ret to the if statement,
as that is the only place that it is used, which is how the function was
before the fixes commit.
Fixes: 1a75fac9a0f9 ("io_uring: avoid ring quiesce while registering/unregistering eventfd")
Link: https://github.com/ClangBuiltLinux/linux/issues/1579
Signed-off-by: Nathan Chancellor <[email protected]>
---
fs/io_uring.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/fs/io_uring.c b/fs/io_uring.c
index 5479f0607430..7ef04bb66da1 100644
--- a/fs/io_uring.c
+++ b/fs/io_uring.c
@@ -9370,7 +9370,7 @@ static int io_eventfd_register(struct io_ring_ctx *ctx, void __user *arg,
{
struct io_ev_fd *ev_fd;
__s32 __user *fds = arg;
- int fd, ret;
+ int fd;
ev_fd = rcu_dereference_protected(ctx->io_ev_fd,
lockdep_is_held(&ctx->uring_lock));
@@ -9386,14 +9386,14 @@ static int io_eventfd_register(struct io_ring_ctx *ctx, void __user *arg,
ev_fd->cq_ev_fd = eventfd_ctx_fdget(fd);
if (IS_ERR(ev_fd->cq_ev_fd)) {
- ret = PTR_ERR(ev_fd->cq_ev_fd);
+ int ret = PTR_ERR(ev_fd->cq_ev_fd);
kfree(ev_fd);
return ret;
}
ev_fd->eventfd_async = eventfd_async;
rcu_assign_pointer(ctx->io_ev_fd, ev_fd);
- return ret;
+ return 0;
}
static void io_eventfd_put(struct rcu_head *rcu)
base-commit: 88a0394bc27de2dd8a8715970f289c5627052532
--
2.35.1
^ permalink raw reply related [flat|nested] 6+ messages in thread* Re: [PATCH] io_uring: Fix use of uninitialized ret in io_eventfd_register()
2022-02-07 16:24 [PATCH] io_uring: Fix use of uninitialized ret in io_eventfd_register() Nathan Chancellor
@ 2022-02-07 19:32 ` Nick Desaulniers
2022-02-07 19:36 ` Nathan Chancellor
2022-02-08 13:37 ` [External] " Usama Arif
2022-02-08 13:58 ` Jens Axboe
2 siblings, 1 reply; 6+ messages in thread
From: Nick Desaulniers @ 2022-02-07 19:32 UTC (permalink / raw)
To: Nathan Chancellor
Cc: Jens Axboe, Pavel Begunkov, Usama Arif, io-uring, linux-kernel,
llvm
On Mon, Feb 7, 2022 at 8:24 AM Nathan Chancellor <[email protected]> wrote:
>
> Clang warns:
>
> fs/io_uring.c:9396:9: warning: variable 'ret' is uninitialized when used here [-Wuninitialized]
> return ret;
> ^~~
> fs/io_uring.c:9373:13: note: initialize the variable 'ret' to silence this warning
> int fd, ret;
> ^
> = 0
> 1 warning generated.
>
> Just return 0 directly and reduce the scope of ret to the if statement,
> as that is the only place that it is used, which is how the function was
> before the fixes commit.
>
> Fixes: 1a75fac9a0f9 ("io_uring: avoid ring quiesce while registering/unregistering eventfd")
Did SHA's change? In linux-next, I see:
commit b77e315a9644 ("io_uring: avoid ring quiesce while
registering/unregistering eventfd")
otherwise LGTM
> Link: https://github.com/ClangBuiltLinux/linux/issues/1579
> Signed-off-by: Nathan Chancellor <[email protected]>
> ---
> fs/io_uring.c | 6 +++---
> 1 file changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/fs/io_uring.c b/fs/io_uring.c
> index 5479f0607430..7ef04bb66da1 100644
> --- a/fs/io_uring.c
> +++ b/fs/io_uring.c
> @@ -9370,7 +9370,7 @@ static int io_eventfd_register(struct io_ring_ctx *ctx, void __user *arg,
> {
> struct io_ev_fd *ev_fd;
> __s32 __user *fds = arg;
> - int fd, ret;
> + int fd;
>
> ev_fd = rcu_dereference_protected(ctx->io_ev_fd,
> lockdep_is_held(&ctx->uring_lock));
> @@ -9386,14 +9386,14 @@ static int io_eventfd_register(struct io_ring_ctx *ctx, void __user *arg,
>
> ev_fd->cq_ev_fd = eventfd_ctx_fdget(fd);
> if (IS_ERR(ev_fd->cq_ev_fd)) {
> - ret = PTR_ERR(ev_fd->cq_ev_fd);
> + int ret = PTR_ERR(ev_fd->cq_ev_fd);
> kfree(ev_fd);
> return ret;
> }
> ev_fd->eventfd_async = eventfd_async;
>
> rcu_assign_pointer(ctx->io_ev_fd, ev_fd);
> - return ret;
> + return 0;
> }
>
> static void io_eventfd_put(struct rcu_head *rcu)
>
> base-commit: 88a0394bc27de2dd8a8715970f289c5627052532
> --
> 2.35.1
>
>
--
Thanks,
~Nick Desaulniers
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH] io_uring: Fix use of uninitialized ret in io_eventfd_register()
2022-02-07 19:32 ` Nick Desaulniers
@ 2022-02-07 19:36 ` Nathan Chancellor
2022-02-07 19:38 ` Nick Desaulniers
0 siblings, 1 reply; 6+ messages in thread
From: Nathan Chancellor @ 2022-02-07 19:36 UTC (permalink / raw)
To: Nick Desaulniers
Cc: Jens Axboe, Pavel Begunkov, Usama Arif, io-uring, linux-kernel,
llvm
On Mon, Feb 07, 2022 at 11:32:03AM -0800, Nick Desaulniers wrote:
> On Mon, Feb 7, 2022 at 8:24 AM Nathan Chancellor <[email protected]> wrote:
> >
> > Clang warns:
> >
> > fs/io_uring.c:9396:9: warning: variable 'ret' is uninitialized when used here [-Wuninitialized]
> > return ret;
> > ^~~
> > fs/io_uring.c:9373:13: note: initialize the variable 'ret' to silence this warning
> > int fd, ret;
> > ^
> > = 0
> > 1 warning generated.
> >
> > Just return 0 directly and reduce the scope of ret to the if statement,
> > as that is the only place that it is used, which is how the function was
> > before the fixes commit.
> >
> > Fixes: 1a75fac9a0f9 ("io_uring: avoid ring quiesce while registering/unregistering eventfd")
>
> Did SHA's change? In linux-next, I see:
> commit b77e315a9644 ("io_uring: avoid ring quiesce while
> registering/unregistering eventfd")
> otherwise LGTM
Yes, this is against Jens' latest for-5.18/io_uring branch, which was
rebased after next-20220207 was released.
https://git.kernel.dk/cgit/linux-block/log/?h=for-5.18/io_uring
Cheers,
Nathan
> > Link: https://github.com/ClangBuiltLinux/linux/issues/1579
> > Signed-off-by: Nathan Chancellor <[email protected]>
> > ---
> > fs/io_uring.c | 6 +++---
> > 1 file changed, 3 insertions(+), 3 deletions(-)
> >
> > diff --git a/fs/io_uring.c b/fs/io_uring.c
> > index 5479f0607430..7ef04bb66da1 100644
> > --- a/fs/io_uring.c
> > +++ b/fs/io_uring.c
> > @@ -9370,7 +9370,7 @@ static int io_eventfd_register(struct io_ring_ctx *ctx, void __user *arg,
> > {
> > struct io_ev_fd *ev_fd;
> > __s32 __user *fds = arg;
> > - int fd, ret;
> > + int fd;
> >
> > ev_fd = rcu_dereference_protected(ctx->io_ev_fd,
> > lockdep_is_held(&ctx->uring_lock));
> > @@ -9386,14 +9386,14 @@ static int io_eventfd_register(struct io_ring_ctx *ctx, void __user *arg,
> >
> > ev_fd->cq_ev_fd = eventfd_ctx_fdget(fd);
> > if (IS_ERR(ev_fd->cq_ev_fd)) {
> > - ret = PTR_ERR(ev_fd->cq_ev_fd);
> > + int ret = PTR_ERR(ev_fd->cq_ev_fd);
> > kfree(ev_fd);
> > return ret;
> > }
> > ev_fd->eventfd_async = eventfd_async;
> >
> > rcu_assign_pointer(ctx->io_ev_fd, ev_fd);
> > - return ret;
> > + return 0;
> > }
> >
> > static void io_eventfd_put(struct rcu_head *rcu)
> >
> > base-commit: 88a0394bc27de2dd8a8715970f289c5627052532
> > --
> > 2.35.1
> >
> >
>
>
> --
> Thanks,
> ~Nick Desaulniers
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH] io_uring: Fix use of uninitialized ret in io_eventfd_register()
2022-02-07 19:36 ` Nathan Chancellor
@ 2022-02-07 19:38 ` Nick Desaulniers
0 siblings, 0 replies; 6+ messages in thread
From: Nick Desaulniers @ 2022-02-07 19:38 UTC (permalink / raw)
To: Nathan Chancellor
Cc: Jens Axboe, Pavel Begunkov, Usama Arif, io-uring, linux-kernel,
llvm
On Mon, Feb 7, 2022 at 11:36 AM Nathan Chancellor <[email protected]> wrote:
>
> On Mon, Feb 07, 2022 at 11:32:03AM -0800, Nick Desaulniers wrote:
> > On Mon, Feb 7, 2022 at 8:24 AM Nathan Chancellor <[email protected]> wrote:
> > >
> > > Clang warns:
> > >
> > > fs/io_uring.c:9396:9: warning: variable 'ret' is uninitialized when used here [-Wuninitialized]
> > > return ret;
> > > ^~~
> > > fs/io_uring.c:9373:13: note: initialize the variable 'ret' to silence this warning
> > > int fd, ret;
> > > ^
> > > = 0
> > > 1 warning generated.
> > >
> > > Just return 0 directly and reduce the scope of ret to the if statement,
> > > as that is the only place that it is used, which is how the function was
> > > before the fixes commit.
> > >
> > > Fixes: 1a75fac9a0f9 ("io_uring: avoid ring quiesce while registering/unregistering eventfd")
> >
> > Did SHA's change? In linux-next, I see:
> > commit b77e315a9644 ("io_uring: avoid ring quiesce while
> > registering/unregistering eventfd")
> > otherwise LGTM
>
> Yes, this is against Jens' latest for-5.18/io_uring branch, which was
> rebased after next-20220207 was released.
>
> https://git.kernel.dk/cgit/linux-block/log/?h=for-5.18/io_uring
Thanks for the explanation.
Reviewed-by: Nick Desaulniers <[email protected]>
>
> Cheers,
> Nathan
>
> > > Link: https://github.com/ClangBuiltLinux/linux/issues/1579
> > > Signed-off-by: Nathan Chancellor <[email protected]>
> > > ---
> > > fs/io_uring.c | 6 +++---
> > > 1 file changed, 3 insertions(+), 3 deletions(-)
> > >
> > > diff --git a/fs/io_uring.c b/fs/io_uring.c
> > > index 5479f0607430..7ef04bb66da1 100644
> > > --- a/fs/io_uring.c
> > > +++ b/fs/io_uring.c
> > > @@ -9370,7 +9370,7 @@ static int io_eventfd_register(struct io_ring_ctx *ctx, void __user *arg,
> > > {
> > > struct io_ev_fd *ev_fd;
> > > __s32 __user *fds = arg;
> > > - int fd, ret;
> > > + int fd;
> > >
> > > ev_fd = rcu_dereference_protected(ctx->io_ev_fd,
> > > lockdep_is_held(&ctx->uring_lock));
> > > @@ -9386,14 +9386,14 @@ static int io_eventfd_register(struct io_ring_ctx *ctx, void __user *arg,
> > >
> > > ev_fd->cq_ev_fd = eventfd_ctx_fdget(fd);
> > > if (IS_ERR(ev_fd->cq_ev_fd)) {
> > > - ret = PTR_ERR(ev_fd->cq_ev_fd);
> > > + int ret = PTR_ERR(ev_fd->cq_ev_fd);
> > > kfree(ev_fd);
> > > return ret;
> > > }
> > > ev_fd->eventfd_async = eventfd_async;
> > >
> > > rcu_assign_pointer(ctx->io_ev_fd, ev_fd);
> > > - return ret;
> > > + return 0;
> > > }
> > >
> > > static void io_eventfd_put(struct rcu_head *rcu)
> > >
> > > base-commit: 88a0394bc27de2dd8a8715970f289c5627052532
> > > --
> > > 2.35.1
> > >
> > >
> >
> >
> > --
> > Thanks,
> > ~Nick Desaulniers
>
--
Thanks,
~Nick Desaulniers
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [External] [PATCH] io_uring: Fix use of uninitialized ret in io_eventfd_register()
2022-02-07 16:24 [PATCH] io_uring: Fix use of uninitialized ret in io_eventfd_register() Nathan Chancellor
2022-02-07 19:32 ` Nick Desaulniers
@ 2022-02-08 13:37 ` Usama Arif
2022-02-08 13:58 ` Jens Axboe
2 siblings, 0 replies; 6+ messages in thread
From: Usama Arif @ 2022-02-08 13:37 UTC (permalink / raw)
To: Nathan Chancellor, Jens Axboe, Pavel Begunkov, trix,
Nick Desaulniers
Cc: io-uring, linux-kernel, llvm
On 07/02/2022 16:24, Nathan Chancellor wrote:
> Clang warns:
>
> fs/io_uring.c:9396:9: warning: variable 'ret' is uninitialized when used here [-Wuninitialized]
> return ret;
> ^~~
> fs/io_uring.c:9373:13: note: initialize the variable 'ret' to silence this warning
> int fd, ret;
> ^
> = 0
> 1 warning generated.
>
> Just return 0 directly and reduce the scope of ret to the if statement,
> as that is the only place that it is used, which is how the function was
> before the fixes commit.
>
> Fixes: 1a75fac9a0f9 ("io_uring: avoid ring quiesce while registering/unregistering eventfd")
> Link: https://github.com/ClangBuiltLinux/linux/issues/1579
> Signed-off-by: Nathan Chancellor <[email protected]>
> ---
> fs/io_uring.c | 6 +++---
> 1 file changed, 3 insertions(+), 3 deletions(-)
>
Thanks for pointing this out. Just for some background in earlier
revisions of the patch for "io_uring: avoid ring quiesce while
registering/unregistering eventfd" the error return part was being
handled differently, but ended up looking similar to before in the final
revision that got merged, but i forgot to change this part back. Would
it be possible to fold the below diff into the patch as well Jens?
Thanks and sorry for missing this!
Regards,
Usama
> diff --git a/fs/io_uring.c b/fs/io_uring.c
> index 5479f0607430..7ef04bb66da1 100644
> --- a/fs/io_uring.c
> +++ b/fs/io_uring.c
> @@ -9370,7 +9370,7 @@ static int io_eventfd_register(struct io_ring_ctx *ctx, void __user *arg,
> {
> struct io_ev_fd *ev_fd;
> __s32 __user *fds = arg;
> - int fd, ret;
> + int fd;
>
> ev_fd = rcu_dereference_protected(ctx->io_ev_fd,
> lockdep_is_held(&ctx->uring_lock));
> @@ -9386,14 +9386,14 @@ static int io_eventfd_register(struct io_ring_ctx *ctx, void __user *arg,
>
> ev_fd->cq_ev_fd = eventfd_ctx_fdget(fd);
> if (IS_ERR(ev_fd->cq_ev_fd)) {
> - ret = PTR_ERR(ev_fd->cq_ev_fd);
> + int ret = PTR_ERR(ev_fd->cq_ev_fd);
> kfree(ev_fd);
> return ret;
> }
> ev_fd->eventfd_async = eventfd_async;
>
> rcu_assign_pointer(ctx->io_ev_fd, ev_fd);
> - return ret;
> + return 0;
> }
>
> static void io_eventfd_put(struct rcu_head *rcu)
>
> base-commit: 88a0394bc27de2dd8a8715970f289c5627052532
>
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH] io_uring: Fix use of uninitialized ret in io_eventfd_register()
2022-02-07 16:24 [PATCH] io_uring: Fix use of uninitialized ret in io_eventfd_register() Nathan Chancellor
2022-02-07 19:32 ` Nick Desaulniers
2022-02-08 13:37 ` [External] " Usama Arif
@ 2022-02-08 13:58 ` Jens Axboe
2 siblings, 0 replies; 6+ messages in thread
From: Jens Axboe @ 2022-02-08 13:58 UTC (permalink / raw)
To: Pavel Begunkov, Nathan Chancellor
Cc: io-uring, llvm, Usama Arif, linux-kernel, Nick Desaulniers
On Mon, 7 Feb 2022 09:24:11 -0700, Nathan Chancellor wrote:
> Clang warns:
>
> fs/io_uring.c:9396:9: warning: variable 'ret' is uninitialized when used here [-Wuninitialized]
> return ret;
> ^~~
> fs/io_uring.c:9373:13: note: initialize the variable 'ret' to silence this warning
> int fd, ret;
> ^
> = 0
> 1 warning generated.
>
> [...]
Applied, thanks!
[1/1] io_uring: Fix use of uninitialized ret in io_eventfd_register()
commit: 4c65723081332607ca331072b0f8a90189e2e447
Best regards,
--
Jens Axboe
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2022-02-08 13:58 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2022-02-07 16:24 [PATCH] io_uring: Fix use of uninitialized ret in io_eventfd_register() Nathan Chancellor
2022-02-07 19:32 ` Nick Desaulniers
2022-02-07 19:36 ` Nathan Chancellor
2022-02-07 19:38 ` Nick Desaulniers
2022-02-08 13:37 ` [External] " Usama Arif
2022-02-08 13:58 ` Jens Axboe
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox