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

From: Gao Xiang
Date: Thu Aug 29 2024 - 07:16:10 EST


Hi liujinbao1,

Since it's a trivial bug which can be simply fixed, I just
would like to confirm if you still have time and interest to
send a workable patch (after a week).

If you don't have time to follow up, I will ask Hongzhen to
fix this with your "Reported-by:" tomorrow.

Thanks,
Gao Xiang

On 2024/8/23 20:03, Gao Xiang wrote:


On 2024/8/23 11:05, liujinbao1 wrote:
From: liujinbao1 <liujinbao1@xxxxxxxxxx>

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@xxxxxxxxxx>
---
  fs/erofs/decompressor.c | 8 +++-----
  1 file changed, 3 insertions(+), 5 deletions(-)

Hi,

The patch is corrupted and the patch subject
line is also broken.

I think it should be:
[PATCH v2] erofs: prevent entering an infinite loop when i is 0


On 2024/8/22 14:27, liujinbao1 wrote:
From: liujinbao1 <liujinbao1@xxxxxxxxxx>

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.

Missing your Signed-off-by here.

---
  fs/erofs/decompressor.c | 2 ++
  1 file changed, 2 insertions(+)

diff --git a/fs/erofs/decompressor.c b/fs/erofs/decompressor.c index
c2253b6a5416..1b2b8cc7911c 100644
--- a/fs/erofs/decompressor.c
+++ b/fs/erofs/decompressor.c
@@ -539,6 +539,8 @@ 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) {
+                    if (!i)
+                            return err;
                      while (--i)
                              if (z_erofs_decomp[i])
                                      z_erofs_decomp[i]->exit();


Thanks for catching this, how about the following diff (space-demaged).

If it looks good to you, could you please send another version?

diff --git a/fs/erofs/decompressor.c b/fs/erofs/decompressor.c index c2253b6a5416..c9b2bc1309d2 100644
--- a/fs/erofs/decompressor.c
+++ b/fs/erofs/decompressor.c
@@ -534,18 +534,16 @@ 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) {
+              if (err && i)
                        while (--i)
                                if (z_erofs_decomp[i])
                                        z_erofs_decomp[i]->exit();
-                      return err;
+                        break;
-              }
        }
-      return 0;
+      return err;
}

missing break?

Why needing a break?

Thanks,
Gao Xiang