On Mon, 2010-03-15 at 20:42 +1100, Greg Banks wrote:
...
> Patch looks good to me too.
>
> >>
> >> The memory allocation based on the ntohl(pduProfile->numprof), or
> >> instprof->profile_len, value looks like it could still use some
> >> kind of ceiling sanity test? (as per Gregs bug)
> >
> > Yeah, it was that comment that struck me as the more important point.
> > Letting an arbitrary network-connected client allocate arbitrary amounts
> > of memory as root on the pmcd machine seems pretty terrible.
>
> What they said. While memory is plentiful today, so are malicious people.
>
Can we have a short discussion on these possible limits?
I'll buy limiting profile_len to 1024 (of the possible 2^31 instance
domains) because as of Mar 2010 there are fewer than 100 different
instance domains in the open source version of PCP.
But instances_len occurs with every instance domain in the profile, and
the largest instance domain is likely to be for "proc" metrics so does
anyone want to put a limit on this? And I've checked, at least pmtop
enumerates the proc indom and adds it to the profile, pmlogger could end
up with a long list of explicit instances to be logged, pminfo will do
the same as pmtop for any indom that is non-enumerable (all right, the
IRIX proc PMDA is probably the only place this was ever used aside from
the sample PMDA). Other uses are probably more benign as the size of
the instance domain in the profile is driven by configuration files or
interactive user input, so legitimate long lists are less likely.
So when I enforce a limit, clients will see PM_ERR_IPC if the profile is
too large ... so anyone want to guess at a safe value for instances_len?
If profile_len is limited to 1024 as above then for all practical
purposes the largest profile would be 4*1024*N bytes if instances_len
was constrained to be no larger than N.
|