[PATCH v2] erofs: Prevent entering an infinite loop when i is 0

[Sandeep Dhavale]

Hi Liujinbao,
On Thu, Aug 29, 2024 at 5:24 AM liujinbao1 <jinbaoliu365 at gmail.com> wrote:
>
> From: liujinbao1 <liujinbao1 at xiaomi.com>
>
> When i=0 and err is not equal to 0,
> the while(-1) loop will enter into an
> infinite loop. This patch avoids this issue
>
> Signed-off-by: liujinbao1 <liujinbao1 at xiaomi.com>
> ---
>  fs/erofs/decompressor.c | 10 +++++-----
>  1 file changed, 5 insertions(+), 5 deletions(-)
>
> diff --git a/fs/erofs/decompressor.c b/fs/erofs/decompressor.c
> index c2253b6a5416..672f097966fa 100644
> --- a/fs/erofs/decompressor.c
> +++ b/fs/erofs/decompressor.c
> @@ -534,18 +534,18 @@ int z_erofs_parse_cfgs(struct super_block *sb, struct erofs_super_block *dsb)
>
>  int __init z_erofs_init_decompressor(void)
>  {
> -       int i, err;
> +       int i, err = 0;
>
>         for (i = 0; i < Z_EROFS_COMPRESSION_MAX; ++i) {
>                 err = z_erofs_decomp[i] ? z_erofs_decomp[i]->init() : 0;
> -               if (err) {
> -                       while (--i)
> +               if (err && i) {
> +                       while (i--)
Actually there is a subtle bug in this fix. We will never enter the if
block here when i=0 and err is set which we were trying to fix.
This will cause z_erofs_decomp[0]->init() error to get masked and we
will continue the outer for loop (i.e. when i=0 and err is set).

Yes original code had the bug but probably simpler and readable fix could be

diff --git a/fs/erofs/decompressor.c b/fs/erofs/decompressor.c
index c2253b6a5416..abf2db2ba10c 100644
--- a/fs/erofs/decompressor.c
+++ b/fs/erofs/decompressor.c
@@ -539,7 +539,7 @@ int __init z_erofs_init_decompressor(void)
        for (i = 0; i < Z_EROFS_COMPRESSION_MAX; ++i) {
                err = z_erofs_decomp[i] ? z_erofs_decomp[i]->init() : 0;
                if (err) {
-                       while (--i)
+                       while (i-- > 0)
                                if (z_erofs_decomp[i])
                                        z_erofs_decomp[i]->exit();
                        return err;

Let me know if you want me to send a fix with your Reported-by or you
can send quick v3.

Thanks,
Sandeep.

>                                 if (z_erofs_decomp[i])
>                                         z_erofs_decomp[i]->exit();
> -                       return err;
> +                       break;
>                 }
>         }
> -       return 0;
> +       return err;
>  }
>
> (1) The use of "break" is to enable a jump out of the for loop,
> otherwise it may not be able to exit the loop.
> (2) --i should be changed to i-- because when i is equal to 1,
> the "while (--i)" statement would exit the loop prematurely.
>
> and sorry for the delay in the response.
>