* [PATCH v2] JFS: fix memleak in jfs_mount
@ 2021-09-04 2:37 Dongliang Mu
2021-09-23 0:53 ` Dongliang Mu
0 siblings, 1 reply; 5+ messages in thread
From: Dongliang Mu @ 2021-09-04 2:37 UTC (permalink / raw)
To: Dave Kleikamp; +Cc: Dongliang Mu, jfs-discussion, linux-kernel
In jfs_mount, when diMount(ipaimap2) fails, it goes to errout35. However,
the following code does not free ipaimap2 allocated by diReadSpecial.
Fix this by refactoring the error handling code of jfs_mount. To be
specific, modify the lable name and free ipaimap2 when the above error
ocurrs.
Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2")
Signed-off-by: Dongliang Mu <mudongliangabcd@gmail.com>
---
v1->v2: change "return rc" to "goto out"; fix one coding style
fs/jfs/jfs_mount.c | 51 ++++++++++++++++++++--------------------------
1 file changed, 22 insertions(+), 29 deletions(-)
diff --git a/fs/jfs/jfs_mount.c b/fs/jfs/jfs_mount.c
index 5d7d7170c03c..aa4ff7bcaff2 100644
--- a/fs/jfs/jfs_mount.c
+++ b/fs/jfs/jfs_mount.c
@@ -81,14 +81,14 @@ int jfs_mount(struct super_block *sb)
* (initialize mount inode from the superblock)
*/
if ((rc = chkSuper(sb))) {
- goto errout20;
+ goto out;
}
ipaimap = diReadSpecial(sb, AGGREGATE_I, 0);
if (ipaimap == NULL) {
jfs_err("jfs_mount: Failed to read AGGREGATE_I");
rc = -EIO;
- goto errout20;
+ goto out;
}
sbi->ipaimap = ipaimap;
@@ -99,7 +99,7 @@ int jfs_mount(struct super_block *sb)
*/
if ((rc = diMount(ipaimap))) {
jfs_err("jfs_mount: diMount(ipaimap) failed w/rc = %d", rc);
- goto errout21;
+ goto err_ipaimap;
}
/*
@@ -108,7 +108,7 @@ int jfs_mount(struct super_block *sb)
ipbmap = diReadSpecial(sb, BMAP_I, 0);
if (ipbmap == NULL) {
rc = -EIO;
- goto errout22;
+ goto err_umount_ipaimap;
}
jfs_info("jfs_mount: ipbmap:0x%p", ipbmap);
@@ -120,7 +120,7 @@ int jfs_mount(struct super_block *sb)
*/
if ((rc = dbMount(ipbmap))) {
jfs_err("jfs_mount: dbMount failed w/rc = %d", rc);
- goto errout22;
+ goto err_ipbmap;
}
/*
@@ -139,7 +139,7 @@ int jfs_mount(struct super_block *sb)
if (!ipaimap2) {
jfs_err("jfs_mount: Failed to read AGGREGATE_I");
rc = -EIO;
- goto errout35;
+ goto err_umount_ipbmap;
}
sbi->ipaimap2 = ipaimap2;
@@ -151,7 +151,7 @@ int jfs_mount(struct super_block *sb)
if ((rc = diMount(ipaimap2))) {
jfs_err("jfs_mount: diMount(ipaimap2) failed, rc = %d",
rc);
- goto errout35;
+ goto err_ipaimap2;
}
} else
/* Secondary aggregate inode table is not valid */
@@ -168,7 +168,7 @@ int jfs_mount(struct super_block *sb)
jfs_err("jfs_mount: Failed to read FILESYSTEM_I");
/* open fileset secondary inode allocation map */
rc = -EIO;
- goto errout40;
+ goto err_umount_ipaimap2;
}
jfs_info("jfs_mount: ipimap:0x%p", ipimap);
@@ -178,41 +178,34 @@ int jfs_mount(struct super_block *sb)
/* initialize fileset inode allocation map */
if ((rc = diMount(ipimap))) {
jfs_err("jfs_mount: diMount failed w/rc = %d", rc);
- goto errout41;
+ goto err_ipimap;
}
- goto out;
+ return rc;
/*
* unwind on error
*/
- errout41: /* close fileset inode allocation map inode */
+err_ipimap:
+ /* close fileset inode allocation map inode */
diFreeSpecial(ipimap);
-
- errout40: /* fileset closed */
-
+err_umount_ipaimap2:
/* close secondary aggregate inode allocation map */
- if (ipaimap2) {
+ if (ipaimap2)
diUnmount(ipaimap2, 1);
+err_ipaimap2:
+ /* close aggregate inodes */
+ if (ipaimap2)
diFreeSpecial(ipaimap2);
- }
-
- errout35:
-
- /* close aggregate block allocation map */
+err_umount_ipbmap: /* close aggregate block allocation map */
dbUnmount(ipbmap, 1);
+err_ipbmap: /* close aggregate inodes */
diFreeSpecial(ipbmap);
-
- errout22: /* close aggregate inode allocation map */
-
+err_umount_ipaimap: /* close aggregate inode allocation map */
diUnmount(ipaimap, 1);
-
- errout21: /* close aggregate inodes */
+err_ipaimap: /* close aggregate inodes */
diFreeSpecial(ipaimap);
- errout20: /* aggregate closed */
-
- out:
-
+out:
if (rc)
jfs_err("Mount JFS Failure: %d", rc);
--
2.25.1
^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH v2] JFS: fix memleak in jfs_mount
2021-09-04 2:37 [PATCH v2] JFS: fix memleak in jfs_mount Dongliang Mu
@ 2021-09-23 0:53 ` Dongliang Mu
2021-09-23 14:54 ` Dave Kleikamp
0 siblings, 1 reply; 5+ messages in thread
From: Dongliang Mu @ 2021-09-23 0:53 UTC (permalink / raw)
To: Dave Kleikamp; +Cc: jfs-discussion, linux-kernel
Any update on this thread?
On Sat, Sep 4, 2021 at 10:38 AM Dongliang Mu <mudongliangabcd@gmail.com> wrote:
>
> In jfs_mount, when diMount(ipaimap2) fails, it goes to errout35. However,
> the following code does not free ipaimap2 allocated by diReadSpecial.
>
> Fix this by refactoring the error handling code of jfs_mount. To be
> specific, modify the lable name and free ipaimap2 when the above error
> ocurrs.
>
> Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2")
> Signed-off-by: Dongliang Mu <mudongliangabcd@gmail.com>
> ---
> v1->v2: change "return rc" to "goto out"; fix one coding style
> fs/jfs/jfs_mount.c | 51 ++++++++++++++++++++--------------------------
> 1 file changed, 22 insertions(+), 29 deletions(-)
>
> diff --git a/fs/jfs/jfs_mount.c b/fs/jfs/jfs_mount.c
> index 5d7d7170c03c..aa4ff7bcaff2 100644
> --- a/fs/jfs/jfs_mount.c
> +++ b/fs/jfs/jfs_mount.c
> @@ -81,14 +81,14 @@ int jfs_mount(struct super_block *sb)
> * (initialize mount inode from the superblock)
> */
> if ((rc = chkSuper(sb))) {
> - goto errout20;
> + goto out;
> }
>
> ipaimap = diReadSpecial(sb, AGGREGATE_I, 0);
> if (ipaimap == NULL) {
> jfs_err("jfs_mount: Failed to read AGGREGATE_I");
> rc = -EIO;
> - goto errout20;
> + goto out;
> }
> sbi->ipaimap = ipaimap;
>
> @@ -99,7 +99,7 @@ int jfs_mount(struct super_block *sb)
> */
> if ((rc = diMount(ipaimap))) {
> jfs_err("jfs_mount: diMount(ipaimap) failed w/rc = %d", rc);
> - goto errout21;
> + goto err_ipaimap;
> }
>
> /*
> @@ -108,7 +108,7 @@ int jfs_mount(struct super_block *sb)
> ipbmap = diReadSpecial(sb, BMAP_I, 0);
> if (ipbmap == NULL) {
> rc = -EIO;
> - goto errout22;
> + goto err_umount_ipaimap;
> }
>
> jfs_info("jfs_mount: ipbmap:0x%p", ipbmap);
> @@ -120,7 +120,7 @@ int jfs_mount(struct super_block *sb)
> */
> if ((rc = dbMount(ipbmap))) {
> jfs_err("jfs_mount: dbMount failed w/rc = %d", rc);
> - goto errout22;
> + goto err_ipbmap;
> }
>
> /*
> @@ -139,7 +139,7 @@ int jfs_mount(struct super_block *sb)
> if (!ipaimap2) {
> jfs_err("jfs_mount: Failed to read AGGREGATE_I");
> rc = -EIO;
> - goto errout35;
> + goto err_umount_ipbmap;
> }
> sbi->ipaimap2 = ipaimap2;
>
> @@ -151,7 +151,7 @@ int jfs_mount(struct super_block *sb)
> if ((rc = diMount(ipaimap2))) {
> jfs_err("jfs_mount: diMount(ipaimap2) failed, rc = %d",
> rc);
> - goto errout35;
> + goto err_ipaimap2;
> }
> } else
> /* Secondary aggregate inode table is not valid */
> @@ -168,7 +168,7 @@ int jfs_mount(struct super_block *sb)
> jfs_err("jfs_mount: Failed to read FILESYSTEM_I");
> /* open fileset secondary inode allocation map */
> rc = -EIO;
> - goto errout40;
> + goto err_umount_ipaimap2;
> }
> jfs_info("jfs_mount: ipimap:0x%p", ipimap);
>
> @@ -178,41 +178,34 @@ int jfs_mount(struct super_block *sb)
> /* initialize fileset inode allocation map */
> if ((rc = diMount(ipimap))) {
> jfs_err("jfs_mount: diMount failed w/rc = %d", rc);
> - goto errout41;
> + goto err_ipimap;
> }
>
> - goto out;
> + return rc;
>
> /*
> * unwind on error
> */
> - errout41: /* close fileset inode allocation map inode */
> +err_ipimap:
> + /* close fileset inode allocation map inode */
> diFreeSpecial(ipimap);
> -
> - errout40: /* fileset closed */
> -
> +err_umount_ipaimap2:
> /* close secondary aggregate inode allocation map */
> - if (ipaimap2) {
> + if (ipaimap2)
> diUnmount(ipaimap2, 1);
> +err_ipaimap2:
> + /* close aggregate inodes */
> + if (ipaimap2)
> diFreeSpecial(ipaimap2);
> - }
> -
> - errout35:
> -
> - /* close aggregate block allocation map */
> +err_umount_ipbmap: /* close aggregate block allocation map */
> dbUnmount(ipbmap, 1);
> +err_ipbmap: /* close aggregate inodes */
> diFreeSpecial(ipbmap);
> -
> - errout22: /* close aggregate inode allocation map */
> -
> +err_umount_ipaimap: /* close aggregate inode allocation map */
> diUnmount(ipaimap, 1);
> -
> - errout21: /* close aggregate inodes */
> +err_ipaimap: /* close aggregate inodes */
> diFreeSpecial(ipaimap);
> - errout20: /* aggregate closed */
> -
> - out:
> -
> +out:
> if (rc)
> jfs_err("Mount JFS Failure: %d", rc);
>
> --
> 2.25.1
>
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH v2] JFS: fix memleak in jfs_mount
2021-09-23 0:53 ` Dongliang Mu
@ 2021-09-23 14:54 ` Dave Kleikamp
2021-11-08 1:10 ` Dongliang Mu
0 siblings, 1 reply; 5+ messages in thread
From: Dave Kleikamp @ 2021-09-23 14:54 UTC (permalink / raw)
To: Dongliang Mu; +Cc: jfs-discussion, linux-kernel
On 9/22/21 19:53, Dongliang Mu wrote:
> Any update on this thread?
Sorry for taking this long to get back to it. The patch looks good. It
should show up in linux-next shortly.
Shaggy
>
> On Sat, Sep 4, 2021 at 10:38 AM Dongliang Mu <mudongliangabcd@gmail.com> wrote:
>>
>> In jfs_mount, when diMount(ipaimap2) fails, it goes to errout35. However,
>> the following code does not free ipaimap2 allocated by diReadSpecial.
>>
>> Fix this by refactoring the error handling code of jfs_mount. To be
>> specific, modify the lable name and free ipaimap2 when the above error
>> ocurrs.
>>
>> Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2")
>> Signed-off-by: Dongliang Mu <mudongliangabcd@gmail.com>
>> ---
>> v1->v2: change "return rc" to "goto out"; fix one coding style
>> fs/jfs/jfs_mount.c | 51 ++++++++++++++++++++--------------------------
>> 1 file changed, 22 insertions(+), 29 deletions(-)
>>
>> diff --git a/fs/jfs/jfs_mount.c b/fs/jfs/jfs_mount.c
>> index 5d7d7170c03c..aa4ff7bcaff2 100644
>> --- a/fs/jfs/jfs_mount.c
>> +++ b/fs/jfs/jfs_mount.c
>> @@ -81,14 +81,14 @@ int jfs_mount(struct super_block *sb)
>> * (initialize mount inode from the superblock)
>> */
>> if ((rc = chkSuper(sb))) {
>> - goto errout20;
>> + goto out;
>> }
>>
>> ipaimap = diReadSpecial(sb, AGGREGATE_I, 0);
>> if (ipaimap == NULL) {
>> jfs_err("jfs_mount: Failed to read AGGREGATE_I");
>> rc = -EIO;
>> - goto errout20;
>> + goto out;
>> }
>> sbi->ipaimap = ipaimap;
>>
>> @@ -99,7 +99,7 @@ int jfs_mount(struct super_block *sb)
>> */
>> if ((rc = diMount(ipaimap))) {
>> jfs_err("jfs_mount: diMount(ipaimap) failed w/rc = %d", rc);
>> - goto errout21;
>> + goto err_ipaimap;
>> }
>>
>> /*
>> @@ -108,7 +108,7 @@ int jfs_mount(struct super_block *sb)
>> ipbmap = diReadSpecial(sb, BMAP_I, 0);
>> if (ipbmap == NULL) {
>> rc = -EIO;
>> - goto errout22;
>> + goto err_umount_ipaimap;
>> }
>>
>> jfs_info("jfs_mount: ipbmap:0x%p", ipbmap);
>> @@ -120,7 +120,7 @@ int jfs_mount(struct super_block *sb)
>> */
>> if ((rc = dbMount(ipbmap))) {
>> jfs_err("jfs_mount: dbMount failed w/rc = %d", rc);
>> - goto errout22;
>> + goto err_ipbmap;
>> }
>>
>> /*
>> @@ -139,7 +139,7 @@ int jfs_mount(struct super_block *sb)
>> if (!ipaimap2) {
>> jfs_err("jfs_mount: Failed to read AGGREGATE_I");
>> rc = -EIO;
>> - goto errout35;
>> + goto err_umount_ipbmap;
>> }
>> sbi->ipaimap2 = ipaimap2;
>>
>> @@ -151,7 +151,7 @@ int jfs_mount(struct super_block *sb)
>> if ((rc = diMount(ipaimap2))) {
>> jfs_err("jfs_mount: diMount(ipaimap2) failed, rc = %d",
>> rc);
>> - goto errout35;
>> + goto err_ipaimap2;
>> }
>> } else
>> /* Secondary aggregate inode table is not valid */
>> @@ -168,7 +168,7 @@ int jfs_mount(struct super_block *sb)
>> jfs_err("jfs_mount: Failed to read FILESYSTEM_I");
>> /* open fileset secondary inode allocation map */
>> rc = -EIO;
>> - goto errout40;
>> + goto err_umount_ipaimap2;
>> }
>> jfs_info("jfs_mount: ipimap:0x%p", ipimap);
>>
>> @@ -178,41 +178,34 @@ int jfs_mount(struct super_block *sb)
>> /* initialize fileset inode allocation map */
>> if ((rc = diMount(ipimap))) {
>> jfs_err("jfs_mount: diMount failed w/rc = %d", rc);
>> - goto errout41;
>> + goto err_ipimap;
>> }
>>
>> - goto out;
>> + return rc;
>>
>> /*
>> * unwind on error
>> */
>> - errout41: /* close fileset inode allocation map inode */
>> +err_ipimap:
>> + /* close fileset inode allocation map inode */
>> diFreeSpecial(ipimap);
>> -
>> - errout40: /* fileset closed */
>> -
>> +err_umount_ipaimap2:
>> /* close secondary aggregate inode allocation map */
>> - if (ipaimap2) {
>> + if (ipaimap2)
>> diUnmount(ipaimap2, 1);
>> +err_ipaimap2:
>> + /* close aggregate inodes */
>> + if (ipaimap2)
>> diFreeSpecial(ipaimap2);
>> - }
>> -
>> - errout35:
>> -
>> - /* close aggregate block allocation map */
>> +err_umount_ipbmap: /* close aggregate block allocation map */
>> dbUnmount(ipbmap, 1);
>> +err_ipbmap: /* close aggregate inodes */
>> diFreeSpecial(ipbmap);
>> -
>> - errout22: /* close aggregate inode allocation map */
>> -
>> +err_umount_ipaimap: /* close aggregate inode allocation map */
>> diUnmount(ipaimap, 1);
>> -
>> - errout21: /* close aggregate inodes */
>> +err_ipaimap: /* close aggregate inodes */
>> diFreeSpecial(ipaimap);
>> - errout20: /* aggregate closed */
>> -
>> - out:
>> -
>> +out:
>> if (rc)
>> jfs_err("Mount JFS Failure: %d", rc);
>>
>> --
>> 2.25.1
>>
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH v2] JFS: fix memleak in jfs_mount
2021-09-23 14:54 ` Dave Kleikamp
@ 2021-11-08 1:10 ` Dongliang Mu
2021-11-08 1:15 ` Dongliang Mu
0 siblings, 1 reply; 5+ messages in thread
From: Dongliang Mu @ 2021-11-08 1:10 UTC (permalink / raw)
To: Dave Kleikamp; +Cc: jfs-discussion, linux-kernel
On Thu, Sep 23, 2021 at 10:54 PM Dave Kleikamp <dave.kleikamp@oracle.com> wrote:
>
> On 9/22/21 19:53, Dongliang Mu wrote:
> > Any update on this thread?
>
> Sorry for taking this long to get back to it. The patch looks good. It
> should show up in linux-next shortly.
>
Hi Dave,
In my local syzkaller testing, the upstream is still vulnerable to
this patch. How does the patch applied in linux-next propagate to
upstream?
> Shaggy
>
> >
> > On Sat, Sep 4, 2021 at 10:38 AM Dongliang Mu <mudongliangabcd@gmail.com> wrote:
> >>
> >> In jfs_mount, when diMount(ipaimap2) fails, it goes to errout35. However,
> >> the following code does not free ipaimap2 allocated by diReadSpecial.
> >>
> >> Fix this by refactoring the error handling code of jfs_mount. To be
> >> specific, modify the lable name and free ipaimap2 when the above error
> >> ocurrs.
> >>
> >> Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2")
> >> Signed-off-by: Dongliang Mu <mudongliangabcd@gmail.com>
> >> ---
> >> v1->v2: change "return rc" to "goto out"; fix one coding style
> >> fs/jfs/jfs_mount.c | 51 ++++++++++++++++++++--------------------------
> >> 1 file changed, 22 insertions(+), 29 deletions(-)
> >>
> >> diff --git a/fs/jfs/jfs_mount.c b/fs/jfs/jfs_mount.c
> >> index 5d7d7170c03c..aa4ff7bcaff2 100644
> >> --- a/fs/jfs/jfs_mount.c
> >> +++ b/fs/jfs/jfs_mount.c
> >> @@ -81,14 +81,14 @@ int jfs_mount(struct super_block *sb)
> >> * (initialize mount inode from the superblock)
> >> */
> >> if ((rc = chkSuper(sb))) {
> >> - goto errout20;
> >> + goto out;
> >> }
> >>
> >> ipaimap = diReadSpecial(sb, AGGREGATE_I, 0);
> >> if (ipaimap == NULL) {
> >> jfs_err("jfs_mount: Failed to read AGGREGATE_I");
> >> rc = -EIO;
> >> - goto errout20;
> >> + goto out;
> >> }
> >> sbi->ipaimap = ipaimap;
> >>
> >> @@ -99,7 +99,7 @@ int jfs_mount(struct super_block *sb)
> >> */
> >> if ((rc = diMount(ipaimap))) {
> >> jfs_err("jfs_mount: diMount(ipaimap) failed w/rc = %d", rc);
> >> - goto errout21;
> >> + goto err_ipaimap;
> >> }
> >>
> >> /*
> >> @@ -108,7 +108,7 @@ int jfs_mount(struct super_block *sb)
> >> ipbmap = diReadSpecial(sb, BMAP_I, 0);
> >> if (ipbmap == NULL) {
> >> rc = -EIO;
> >> - goto errout22;
> >> + goto err_umount_ipaimap;
> >> }
> >>
> >> jfs_info("jfs_mount: ipbmap:0x%p", ipbmap);
> >> @@ -120,7 +120,7 @@ int jfs_mount(struct super_block *sb)
> >> */
> >> if ((rc = dbMount(ipbmap))) {
> >> jfs_err("jfs_mount: dbMount failed w/rc = %d", rc);
> >> - goto errout22;
> >> + goto err_ipbmap;
> >> }
> >>
> >> /*
> >> @@ -139,7 +139,7 @@ int jfs_mount(struct super_block *sb)
> >> if (!ipaimap2) {
> >> jfs_err("jfs_mount: Failed to read AGGREGATE_I");
> >> rc = -EIO;
> >> - goto errout35;
> >> + goto err_umount_ipbmap;
> >> }
> >> sbi->ipaimap2 = ipaimap2;
> >>
> >> @@ -151,7 +151,7 @@ int jfs_mount(struct super_block *sb)
> >> if ((rc = diMount(ipaimap2))) {
> >> jfs_err("jfs_mount: diMount(ipaimap2) failed, rc = %d",
> >> rc);
> >> - goto errout35;
> >> + goto err_ipaimap2;
> >> }
> >> } else
> >> /* Secondary aggregate inode table is not valid */
> >> @@ -168,7 +168,7 @@ int jfs_mount(struct super_block *sb)
> >> jfs_err("jfs_mount: Failed to read FILESYSTEM_I");
> >> /* open fileset secondary inode allocation map */
> >> rc = -EIO;
> >> - goto errout40;
> >> + goto err_umount_ipaimap2;
> >> }
> >> jfs_info("jfs_mount: ipimap:0x%p", ipimap);
> >>
> >> @@ -178,41 +178,34 @@ int jfs_mount(struct super_block *sb)
> >> /* initialize fileset inode allocation map */
> >> if ((rc = diMount(ipimap))) {
> >> jfs_err("jfs_mount: diMount failed w/rc = %d", rc);
> >> - goto errout41;
> >> + goto err_ipimap;
> >> }
> >>
> >> - goto out;
> >> + return rc;
> >>
> >> /*
> >> * unwind on error
> >> */
> >> - errout41: /* close fileset inode allocation map inode */
> >> +err_ipimap:
> >> + /* close fileset inode allocation map inode */
> >> diFreeSpecial(ipimap);
> >> -
> >> - errout40: /* fileset closed */
> >> -
> >> +err_umount_ipaimap2:
> >> /* close secondary aggregate inode allocation map */
> >> - if (ipaimap2) {
> >> + if (ipaimap2)
> >> diUnmount(ipaimap2, 1);
> >> +err_ipaimap2:
> >> + /* close aggregate inodes */
> >> + if (ipaimap2)
> >> diFreeSpecial(ipaimap2);
> >> - }
> >> -
> >> - errout35:
> >> -
> >> - /* close aggregate block allocation map */
> >> +err_umount_ipbmap: /* close aggregate block allocation map */
> >> dbUnmount(ipbmap, 1);
> >> +err_ipbmap: /* close aggregate inodes */
> >> diFreeSpecial(ipbmap);
> >> -
> >> - errout22: /* close aggregate inode allocation map */
> >> -
> >> +err_umount_ipaimap: /* close aggregate inode allocation map */
> >> diUnmount(ipaimap, 1);
> >> -
> >> - errout21: /* close aggregate inodes */
> >> +err_ipaimap: /* close aggregate inodes */
> >> diFreeSpecial(ipaimap);
> >> - errout20: /* aggregate closed */
> >> -
> >> - out:
> >> -
> >> +out:
> >> if (rc)
> >> jfs_err("Mount JFS Failure: %d", rc);
> >>
> >> --
> >> 2.25.1
> >>
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH v2] JFS: fix memleak in jfs_mount
2021-11-08 1:10 ` Dongliang Mu
@ 2021-11-08 1:15 ` Dongliang Mu
0 siblings, 0 replies; 5+ messages in thread
From: Dongliang Mu @ 2021-11-08 1:15 UTC (permalink / raw)
To: Dave Kleikamp; +Cc: jfs-discussion, linux-kernel
On Mon, Nov 8, 2021 at 9:10 AM Dongliang Mu <mudongliangabcd@gmail.com> wrote:
>
> On Thu, Sep 23, 2021 at 10:54 PM Dave Kleikamp <dave.kleikamp@oracle.com> wrote:
> >
> > On 9/22/21 19:53, Dongliang Mu wrote:
> > > Any update on this thread?
> >
> > Sorry for taking this long to get back to it. The patch looks good. It
> > should show up in linux-next shortly.
> >
>
> Hi Dave,
>
> In my local syzkaller testing, the upstream is still vulnerable to
> this patch. How does the patch applied in linux-next propagate to
> upstream?
Please ignore it. I have found the commit in the upstream.
>
> > Shaggy
> >
> > >
> > > On Sat, Sep 4, 2021 at 10:38 AM Dongliang Mu <mudongliangabcd@gmail.com> wrote:
> > >>
> > >> In jfs_mount, when diMount(ipaimap2) fails, it goes to errout35. However,
> > >> the following code does not free ipaimap2 allocated by diReadSpecial.
> > >>
> > >> Fix this by refactoring the error handling code of jfs_mount. To be
> > >> specific, modify the lable name and free ipaimap2 when the above error
> > >> ocurrs.
> > >>
> > >> Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2")
> > >> Signed-off-by: Dongliang Mu <mudongliangabcd@gmail.com>
> > >> ---
> > >> v1->v2: change "return rc" to "goto out"; fix one coding style
> > >> fs/jfs/jfs_mount.c | 51 ++++++++++++++++++++--------------------------
> > >> 1 file changed, 22 insertions(+), 29 deletions(-)
> > >>
> > >> diff --git a/fs/jfs/jfs_mount.c b/fs/jfs/jfs_mount.c
> > >> index 5d7d7170c03c..aa4ff7bcaff2 100644
> > >> --- a/fs/jfs/jfs_mount.c
> > >> +++ b/fs/jfs/jfs_mount.c
> > >> @@ -81,14 +81,14 @@ int jfs_mount(struct super_block *sb)
> > >> * (initialize mount inode from the superblock)
> > >> */
> > >> if ((rc = chkSuper(sb))) {
> > >> - goto errout20;
> > >> + goto out;
> > >> }
> > >>
> > >> ipaimap = diReadSpecial(sb, AGGREGATE_I, 0);
> > >> if (ipaimap == NULL) {
> > >> jfs_err("jfs_mount: Failed to read AGGREGATE_I");
> > >> rc = -EIO;
> > >> - goto errout20;
> > >> + goto out;
> > >> }
> > >> sbi->ipaimap = ipaimap;
> > >>
> > >> @@ -99,7 +99,7 @@ int jfs_mount(struct super_block *sb)
> > >> */
> > >> if ((rc = diMount(ipaimap))) {
> > >> jfs_err("jfs_mount: diMount(ipaimap) failed w/rc = %d", rc);
> > >> - goto errout21;
> > >> + goto err_ipaimap;
> > >> }
> > >>
> > >> /*
> > >> @@ -108,7 +108,7 @@ int jfs_mount(struct super_block *sb)
> > >> ipbmap = diReadSpecial(sb, BMAP_I, 0);
> > >> if (ipbmap == NULL) {
> > >> rc = -EIO;
> > >> - goto errout22;
> > >> + goto err_umount_ipaimap;
> > >> }
> > >>
> > >> jfs_info("jfs_mount: ipbmap:0x%p", ipbmap);
> > >> @@ -120,7 +120,7 @@ int jfs_mount(struct super_block *sb)
> > >> */
> > >> if ((rc = dbMount(ipbmap))) {
> > >> jfs_err("jfs_mount: dbMount failed w/rc = %d", rc);
> > >> - goto errout22;
> > >> + goto err_ipbmap;
> > >> }
> > >>
> > >> /*
> > >> @@ -139,7 +139,7 @@ int jfs_mount(struct super_block *sb)
> > >> if (!ipaimap2) {
> > >> jfs_err("jfs_mount: Failed to read AGGREGATE_I");
> > >> rc = -EIO;
> > >> - goto errout35;
> > >> + goto err_umount_ipbmap;
> > >> }
> > >> sbi->ipaimap2 = ipaimap2;
> > >>
> > >> @@ -151,7 +151,7 @@ int jfs_mount(struct super_block *sb)
> > >> if ((rc = diMount(ipaimap2))) {
> > >> jfs_err("jfs_mount: diMount(ipaimap2) failed, rc = %d",
> > >> rc);
> > >> - goto errout35;
> > >> + goto err_ipaimap2;
> > >> }
> > >> } else
> > >> /* Secondary aggregate inode table is not valid */
> > >> @@ -168,7 +168,7 @@ int jfs_mount(struct super_block *sb)
> > >> jfs_err("jfs_mount: Failed to read FILESYSTEM_I");
> > >> /* open fileset secondary inode allocation map */
> > >> rc = -EIO;
> > >> - goto errout40;
> > >> + goto err_umount_ipaimap2;
> > >> }
> > >> jfs_info("jfs_mount: ipimap:0x%p", ipimap);
> > >>
> > >> @@ -178,41 +178,34 @@ int jfs_mount(struct super_block *sb)
> > >> /* initialize fileset inode allocation map */
> > >> if ((rc = diMount(ipimap))) {
> > >> jfs_err("jfs_mount: diMount failed w/rc = %d", rc);
> > >> - goto errout41;
> > >> + goto err_ipimap;
> > >> }
> > >>
> > >> - goto out;
> > >> + return rc;
> > >>
> > >> /*
> > >> * unwind on error
> > >> */
> > >> - errout41: /* close fileset inode allocation map inode */
> > >> +err_ipimap:
> > >> + /* close fileset inode allocation map inode */
> > >> diFreeSpecial(ipimap);
> > >> -
> > >> - errout40: /* fileset closed */
> > >> -
> > >> +err_umount_ipaimap2:
> > >> /* close secondary aggregate inode allocation map */
> > >> - if (ipaimap2) {
> > >> + if (ipaimap2)
> > >> diUnmount(ipaimap2, 1);
> > >> +err_ipaimap2:
> > >> + /* close aggregate inodes */
> > >> + if (ipaimap2)
> > >> diFreeSpecial(ipaimap2);
> > >> - }
> > >> -
> > >> - errout35:
> > >> -
> > >> - /* close aggregate block allocation map */
> > >> +err_umount_ipbmap: /* close aggregate block allocation map */
> > >> dbUnmount(ipbmap, 1);
> > >> +err_ipbmap: /* close aggregate inodes */
> > >> diFreeSpecial(ipbmap);
> > >> -
> > >> - errout22: /* close aggregate inode allocation map */
> > >> -
> > >> +err_umount_ipaimap: /* close aggregate inode allocation map */
> > >> diUnmount(ipaimap, 1);
> > >> -
> > >> - errout21: /* close aggregate inodes */
> > >> +err_ipaimap: /* close aggregate inodes */
> > >> diFreeSpecial(ipaimap);
> > >> - errout20: /* aggregate closed */
> > >> -
> > >> - out:
> > >> -
> > >> +out:
> > >> if (rc)
> > >> jfs_err("Mount JFS Failure: %d", rc);
> > >>
> > >> --
> > >> 2.25.1
> > >>
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2021-11-08 1:15 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-09-04 2:37 [PATCH v2] JFS: fix memleak in jfs_mount Dongliang Mu
2021-09-23 0:53 ` Dongliang Mu
2021-09-23 14:54 ` Dave Kleikamp
2021-11-08 1:10 ` Dongliang Mu
2021-11-08 1:15 ` Dongliang Mu
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).