mm: Fix the uninitialized use in overcommit_policy_handler
Subsystems (2)
PATCH 1/1 mm: Fix the uninitialized use in overcommit_policy_handler
An unexpected value of /proc/sys/vm/panic_on_oom we will get, after running the following program
int main()
{
int fd = open("/proc/sys/vm/panic_on_oom", O_RDWR)
write(fd, "1", 1);
write(fd, "2", 1);
close(fd);
}write(fd, "2", 1) will pass *ppos = 1 to proc_dointvec_minmax. proc_dointvec_minmax will return 0 without setting new_policy.
t.data = &new_policy;
ret = proc_dointvec_minmax(&t, write, buffer, lenp, ppos)
-->do_proc_dointvec
-->__do_proc_dointvec
if (write) {
if (proc_first_pos_non_zero_ignore(ppos, table))
goto out;sysctl_overcommit_memory = new_policy;
so sysctl_overcommit_memory will be set to an uninitialized value.
Initialize new_policy with sysctl_overcommit_memory.
--- mm/util.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
mm/util.c +1 -1
@@ -756,7 +756,7 @@ int overcommit_policy_handler(struct ctl_table *table, int write, void *buffer, size_t *lenp, loff_t *ppos) { struct ctl_table t; -int new_policy; +int new_policy = sysctl_overcommit_memory;int ret;/*
Signature · 1 line
2.17.1
Michal Hocko
reviewerQuoted 22 lines · Chen Jun
An unexpected value of
/proc/sys/vm/panic_on_oomwe will get, after running the following programint
main(){ int fd = open("/proc/sys/vm/panic_on_oom", O_RDWR) write(fd, "1", 1); write(fd, "2", 1); close(fd); }
write(fd, "2", 1)will pass *ppos = 1 toproc_dointvec_minmax.proc_dointvec_minmaxwill return 0 without settingnew_policy.t.data =
&new_policy;
ret =proc_dointvec_minmax(&t, write, buffer, lenp, ppos)-->do_proc_dointvec -->__do_proc_dointvec if (write) { if (proc_first_pos_non_zero_ignore(ppos, table)) goto out;
Quoted 3 lines
sysctl_overcommit_memory=new_policy;so
sysctl_overcommit_memorywill be set to an uninitialized value.
The overcommit_policy_handler (ab)use of proc_dointvec_minmax is really an odd one. It is not really great that proc_dointvec_minmax cannot really tell whether the value has been changed but likely nobody really needed that so far.
I strongly suspect the intention was to do all the follow up handling before making the new mode visible to others. Maybe this can be changed so that the handler doesn't really need to do any hops.
Your fix is an easier part I would just initialize the to -1 so that we can tell nothing has been done the handler can bail out without any follow up work.
Signature · 2 lines
Michal Hocko SUSE Labs
Quoted 27 lines · Michal Hocko
Quoted 22 lines · Chen Jun
An unexpected value of
/proc/sys/vm/panic_on_oomwe will get, after running the following programint
main(){ int fd = open("/proc/sys/vm/panic_on_oom", O_RDWR) write(fd, "1", 1); write(fd, "2", 1); close(fd); }
write(fd, "2", 1)will pass *ppos = 1 toproc_dointvec_minmax.proc_dointvec_minmaxwill return 0 without settingnew_policy.t.data =
&new_policy;
ret =proc_dointvec_minmax(&t, write, buffer, lenp, ppos)-->do_proc_dointvec -->__do_proc_dointvec if (write) { if (proc_first_pos_non_zero_ignore(ppos, table)) goto out;Quoted 3 lines
sysctl_overcommit_memory=new_policy;so
sysctl_overcommit_memorywill be set to an uninitialized value.
Quoted 7 lines
The
overcommit_policy_handler(ab)use ofproc_dointvec_minmaxis really an odd one. It is not really great thatproc_dointvec_minmaxcannot really tell whether the value has been changed but likely nobody really needed that so far.I strongly suspect the intention was to do all the follow up handling before making the new mode visible to others.
IIRC, You are right, there was some error caught by ltp tool, that when the overcommit policy get frequently changed while there is stress memory test, so if the policy is changed to OVERCOMMIT_NEVER, it must happen after the batch number is changed.
Thanks,
Feng
Quoted 9 lines
Maybe this can be changed
so that the handler doesn't really need to do any hops.Your fix is an easier part I would just initialize the to -1 so that we can tell nothing has been done the handler can bail out without any follow up work.
Signature · 2 lines
Michal Hocko SUSE Labs