Messages in this thread | | | Date | Fri, 30 Apr 2021 01:05:47 +0300 | From | Pavel Skripkin <> | Subject | Re: [PATCH] ext4: fix memory leak in ext4_fill_super |
| |
On Thu, 29 Apr 2021 17:41:12 -0400 "Theodore Ts'o" <tytso@mit.edu> wrote:
> On Thu, Apr 29, 2021 at 11:09:56PM +0300, Pavel Skripkin wrote: > > > we will exit with -ENOMEM. So at the very least all callers of > > > kthread_stop() also need to check for -ENOMEM as well as -EINTR > > > --- or, be somehow sure that the thread function was successfully > > > called and started. In this particular case, the ext4 mount code > > > had just started the kmmpd thread, and then detected that > > > something else had gone wrong, and failed the mount before the > > > kmmpd thread ever had a chance to run. > > > > There is a small problem about -ENOMEM... > > What I'd suggest is that we simply move > > > exit_thread: > > EXT4_SB(sb)->s_mmp_tsk = NULL; > > kfree(data); > > brelse(bh); > > return retval; > > } > > out of the thread function. That means hanging struct mmpd_data off > the struct ext4_sb_info structure, and then adding a function like > this to fs/ext4/mmp.c > > static void ext4_stop_mmpd(struct ext4_sb_info *sbi) > { > if (sbi->s_mmp_tsk) { > kthread_stop(sbi->s_mmp_tsk); > brelse(sbi->s_mmp_data->bh); > kfree(sbi->s_mmp_data); > sbi->s_mmp_data = NULL; > sbi->s_mmp_tsk = NULL; > } > } > > Basically, just move all of the cleanup so it is done after the > kthread is stopped, so we don't have to do any fancy error checking. > We just do it unconditionally.
This sound much better than my idea. Will do it in v2.
Thanks!
> > - Ted > > P.S. Actually, we could drop the struct mmpd_data altogether, and > just hang the buffer head as sbi->s_mmp_bh. Then we can just pass the > struct super * as the private pointer to kmmpd(). >
With regards, Pavel Skripkin
| |