Hi Heiko

On Tue, Aug 6, 2024 at 6:29 AM Heiko Schocher hs@denx.de wrote:

Hello Michael,

On 05.08.24 19:31, Michael Trimarchi wrote:

...

The inode should be freed after a reference is get to avoid memory leak

Signed-off-by: Michael Trimarchi michael@amarulasolutions.com

fs/ubifs/ubifs.c | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-)

As discussed, that patch looks valid to me!

It would be good to have some testers...

Looking through the code, I think there is one more place:

$ git diff diff --git a/fs/ubifs/super.c b/fs/ubifs/super.c index d8d78a2d3d..a5d141055c 100644 --- a/fs/ubifs/super.c +++ b/fs/ubifs/super.c @@ -2322,6 +2322,7 @@ static int ubifs_fill_super(struct super_block *sb, void *data, int silent) } #else sb->s_root = NULL;

•   ubifs_iput(inode);
  

  #endif

Right, I think that people are reporting now those problem can try to give a quick try on it

Michael

     mutex_unlock(&c->umount_mutex);

$

What do you mean?

bye, Heiko -- DENX Software Engineering GmbH, Managing Director: Erika Unter HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany Phone: +49-8142-66989-52 Fax: +49-8142-66989-80 Email: hs@denx.de

-- Michael Nazzareno Trimarchi Co-Founder & Chief Executive Officer M. +39 347 913 2170 michael@amarulasolutions.com __________________________________

Amarula Solutions BV Joop Geesinkweg 125, 1114 AB, Amsterdam, NL T. +31 (0)85 111 9172 info@amarulasolutions.com www.amarulasolutions.com

Hi Alexander

On Wed, Aug 7, 2024 at 4:55 PM Alexander Dahl ada@thorsis.com wrote:

Hello Michael,

Am Mon, Aug 05, 2024 at 07:31:40PM +0200 schrieb Michael Trimarchi:

...

The inode should be freed after a reference is get to avoid memory leak

Signed-off-by: Michael Trimarchi michael@amarulasolutions.com

Tested this on top of my recent ubifs series. No obvious problems found on U-Boot command line with various combinations of commands, no crashes, so …

Tested-by: Alexander Dahl ada@thorsis.com

I think that we can resend including last heiko remark and let heiko to pick it from his own tree

Heiko I will add CoDevelopment if it's ok for you

Michael

Greets Alex

...

---

fs/ubifs/ubifs.c | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-)

diff --git a/fs/ubifs/ubifs.c b/fs/ubifs/ubifs.c index 048730db7f..6acd6c3776 100644 --- a/fs/ubifs/ubifs.c +++ b/fs/ubifs/ubifs.c @@ -319,9 +319,7 @@ static int filldir(struct ubifs_info *c, const char *name, int namlen, } ctime_r((time_t *)&inode->i_mtime, filetime); printf("%9lld %24.24s ", inode->i_size, filetime); -#ifndef __UBOOT__ ubifs_iput(inode); -#endif

  printf("%s\n", name);

@@ -557,6 +555,7 @@ static unsigned long ubifs_findfile(struct super_block *sb, char *filename)

                  /* We have some sort of symlink recursion, bail out */
                  if (symlink_count++ > 8) {

•                         ubifs_iput(inode);
                          printf("Symlink recursion, aborting\n");
                          return 0;
                  }
  

@@ -568,6 +567,7 @@ static unsigned long ubifs_findfile(struct super_block *sb, char *filename) * the leading slash */ next = name = link_name + 1; root_inum = 1;

•                         ubifs_iput(inode);
                          continue;
                  }
                  /* Relative to cur dir */
  

@@ -575,6 +575,7 @@ static unsigned long ubifs_findfile(struct super_block *sb, char *filename) link_name, next == NULL ? "" : next); memcpy(symlinkpath, buf, sizeof(buf)); next = name = symlinkpath;

•                 ubifs_iput(inode);
                  continue;
          }
  
  

@@ -583,8 +584,10 @@ static unsigned long ubifs_findfile(struct super_block *sb, char *filename) */

          /* Found the node!  */

•         if (!next || *next == '\0')
  

•         if (!next || *next == '\0') {
  

•                 ubifs_iput(inode);
                  return inum;
  

•         }
  
          root_inum = inum;
          name = next;
  

-- 2.43.0