lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening PHC | |
Open Source and information security mailing list archives
| ||
|
Date: Fri, 30 Apr 2021 01:05:47 +0300 From: Pavel Skripkin <paskripkin@...il.com> To: "Theodore Ts'o" <tytso@....edu> Cc: Vegard Nossum <vegard.nossum@...cle.com>, akpm@...ux-foundation.org, peterz@...radead.org, axboe@...nel.dk, pmladek@...e.com, adilger.kernel@...ger.ca, linux-ext4@...r.kernel.org, linux-kernel@...r.kernel.org, syzbot+d9e482e303930fa4f6ff@...kaller.appspotmail.com Subject: Re: [PATCH] ext4: fix memory leak in ext4_fill_super On Thu, 29 Apr 2021 17:41:12 -0400 "Theodore Ts'o" <tytso@....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
Powered by blists - more mailing lists