Messages in this thread | | | Date | Thu, 5 Jan 2017 12:49:50 +0100 | From | Michal Hocko <> | Subject | Re: [PATCH] proc: Fix integer overflow of VmLib |
| |
On Thu 05-01-17 12:03:47, Richard Weinberger wrote: > Michal, > > Am 05.01.2017 um 11:53 schrieb Michal Hocko: > > I guess you meant s@overflow@underflow@ right? > > Yep, of course. > > > On Thu 05-01-17 00:29:18, Richard Weinberger wrote: > >> /proc/<pid>/status can report extremely high VmLib values which > >> will confuse monitoring tools. > >> VmLib is mm->exec_vm minus text size, where exec_vm is the number of > >> bytes backed by an executable memory mapping and text size is > >> mm->end_code - mm->start_code as set up by binfmt. > >> > >> For the vast majority of all programs text size is smaller than exec_vm. > >> But if a program interprets binaries on its own the calculation result > >> can be negative. > >> UserModeLinux is such an example. It installs and removes lots of PROT_EXEC > >> mappings but mm->start_code and mm->start_code remain and VmLib turns > >> negative. > >> > >> Fix this by detecting the overflow and just return 0. > >> For interpreting the value reported by VmLib is anyway useless but > >> returning 0 does at least not confuse userspace. > > > > Is really 0 what the userspace expects? Why shouldn't we just report > > exec_vm unconditionally? Btw. we used to do something that many years > > back https://lkml.org/lkml/2004/8/24/47. We are exporting the text size > > so the calculation can be done by the userspace. > > Strictly speaking both values, 0 and exec_vm are wrong. > Userspace expects VmLib to be 0 when an application has no libs loaded, > i.e. for statically linked binaries. > > So, either we report 0 as "I don't know" or exec_vm, which is also wrong.
Yes unfortunately.
> I thought 0 is the better choice since it will not lead to wrong results > when userspace tools compute the sum of values reported by /proc/<pid>/status.
Dunno. If somebody translates 0 to statically linked library then it could be wrong.
That being said, the underflow is _clearly_ wrong. I am not sure what the right way is to fix this but whatever we do it might just break somebody's usecase. Sad... -- Michal Hocko SUSE Labs
| |