* [RFC PATCH] ubifs: Call ubifs_iput when ubifs_iget is used @ 2024-08-05 17:31 Michael Trimarchi 2024-08-06 4:28 ` Heiko Schocher 2024-08-07 14:55 ` Alexander Dahl 0 siblings, 2 replies; 6+ messages in thread From: Michael Trimarchi @ 2024-08-05 17:31 UTC (permalink / raw) To: Heiko Schocher Cc: u-boot, Tom Rini, Piotr Wojtaszczyk, Marek Vasut, Michael Trimarchi 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(-) 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 ^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [RFC PATCH] ubifs: Call ubifs_iput when ubifs_iget is used 2024-08-05 17:31 [RFC PATCH] ubifs: Call ubifs_iput when ubifs_iget is used Michael Trimarchi @ 2024-08-06 4:28 ` Heiko Schocher 2024-08-06 7:13 ` Michael Nazzareno Trimarchi 2024-08-07 14:55 ` Alexander Dahl 1 sibling, 1 reply; 6+ messages in thread From: Heiko Schocher @ 2024-08-06 4:28 UTC (permalink / raw) To: Michael Trimarchi; +Cc: u-boot, Tom Rini, Piotr Wojtaszczyk, Marek Vasut 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 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 ^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [RFC PATCH] ubifs: Call ubifs_iput when ubifs_iget is used 2024-08-06 4:28 ` Heiko Schocher @ 2024-08-06 7:13 ` Michael Nazzareno Trimarchi 0 siblings, 0 replies; 6+ messages in thread From: Michael Nazzareno Trimarchi @ 2024-08-06 7:13 UTC (permalink / raw) To: hs; +Cc: u-boot, Tom Rini, Piotr Wojtaszczyk, Marek Vasut 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 ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [RFC PATCH] ubifs: Call ubifs_iput when ubifs_iget is used 2024-08-05 17:31 [RFC PATCH] ubifs: Call ubifs_iput when ubifs_iget is used Michael Trimarchi 2024-08-06 4:28 ` Heiko Schocher @ 2024-08-07 14:55 ` Alexander Dahl 2024-08-07 14:57 ` Michael Nazzareno Trimarchi 1 sibling, 1 reply; 6+ messages in thread From: Alexander Dahl @ 2024-08-07 14:55 UTC (permalink / raw) To: Michael Trimarchi Cc: Heiko Schocher, u-boot, Tom Rini, Piotr Wojtaszczyk, Marek Vasut 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> 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 > ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [RFC PATCH] ubifs: Call ubifs_iput when ubifs_iget is used 2024-08-07 14:55 ` Alexander Dahl @ 2024-08-07 14:57 ` Michael Nazzareno Trimarchi 2024-08-07 15:14 ` Heiko Schocher 0 siblings, 1 reply; 6+ messages in thread From: Michael Nazzareno Trimarchi @ 2024-08-07 14:57 UTC (permalink / raw) To: Michael Trimarchi, Heiko Schocher, u-boot, Tom Rini, Piotr Wojtaszczyk, Marek Vasut 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 > > -- 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 ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [RFC PATCH] ubifs: Call ubifs_iput when ubifs_iget is used 2024-08-07 14:57 ` Michael Nazzareno Trimarchi @ 2024-08-07 15:14 ` Heiko Schocher 0 siblings, 0 replies; 6+ messages in thread From: Heiko Schocher @ 2024-08-07 15:14 UTC (permalink / raw) To: Michael Nazzareno Trimarchi, u-boot, Tom Rini, Piotr Wojtaszczyk, Marek Vasut On 07.08.24 16:57, Michael Nazzareno Trimarchi wrote: > 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 Fine for me! Thanks! bye, Heiko > > 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 >>> > > > -- 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 ^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2024-08-07 15:14 UTC | newest] Thread overview: 6+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2024-08-05 17:31 [RFC PATCH] ubifs: Call ubifs_iput when ubifs_iget is used Michael Trimarchi 2024-08-06 4:28 ` Heiko Schocher 2024-08-06 7:13 ` Michael Nazzareno Trimarchi 2024-08-07 14:55 ` Alexander Dahl 2024-08-07 14:57 ` Michael Nazzareno Trimarchi 2024-08-07 15:14 ` Heiko Schocher
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox