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: Mon, 18 Sep 2017 19:36:56 +0800 From: Yafang Shao <laoar.shao@...il.com> To: Jan Kara <jack@...e.cz> Cc: akpm@...ux-foundation.org, Johannes Weiner <hannes@...xchg.org>, mhocko@...e.com, vdavydov.dev@...il.com, jlayton@...hat.com, nborisov@...e.com, "Theodore Ts'o" <tytso@....edu>, mawilcox@...rosoft.com, linux-mm@...ck.org, linux-kernel@...r.kernel.org Subject: Re: [PATCH] mm: introduce sanity check on dirty ratio sysctl value 2017-09-18 18:22 GMT+08:00 Jan Kara <jack@...e.cz>: > On Mon 18-09-17 01:39:28, Yafang Shao wrote: >> we can find the logic in domain_dirty_limits() that >> when dirty bg_thresh is bigger than dirty thresh, >> bg_thresh will be set as thresh * 1 / 2. >> if (bg_thresh >= thresh) >> bg_thresh = thresh / 2; >> >> But actually we can set dirty_background_raio bigger than >> dirty_ratio successfully. This behavior may mislead us. >> So we should do this sanity check at the beginning. >> >> Signed-off-by: Yafang Shao <laoar.shao@...il.com> > > ... > >> { >> + int old_ratio = dirty_background_ratio; >> + unsigned long bytes; >> int ret; >> >> ret = proc_dointvec_minmax(table, write, buffer, lenp, ppos); >> - if (ret == 0 && write) >> - dirty_background_bytes = 0; >> + >> + if (ret == 0 && write) { >> + if (vm_dirty_ratio > 0) { >> + if (dirty_background_ratio >= vm_dirty_ratio) >> + ret = -EINVAL; >> + } else if (vm_dirty_bytes > 0) { >> + bytes = global_dirtyable_memory() * PAGE_SIZE * >> + dirty_background_ratio / 100; >> + if (bytes >= vm_dirty_bytes) >> + ret = -EINVAL; >> + } >> + >> + if (ret == 0) >> + dirty_background_bytes = 0; >> + else >> + dirty_background_ratio = old_ratio; >> + } >> + > > How about implementing something like > > bool vm_dirty_settings_valid(void) > > helper which would validate whether current dirtiness settings are > consistent. That way we would not have to repeat very similar checks four > times. That seems a smarter way. > Also the arithmetics in: > > global_dirtyable_memory() * PAGE_SIZE * dirty_background_ratio / 100 > > could overflow so I'd prefer to first divide by 100 and then multiply by > dirty_background_ratio... > Oh, yes. It could overflow. > Honza > -- > Jan Kara <jack@...e.com> > SUSE Labs, CR I will reimplement it and submit a new patch. Thanks Yafang
Powered by blists - more mailing lists