hashed pmcd profile for large ctxnums vulnerable to context history-cardinality OOM

[fche]

Thinking about brolley's improvement to pmcd for profile[]->hashtable further, another problem appears. While pmcd would no longer reject ctxnum>2048, it would add (and only add) entries to the profiles hash table for any new ctxnums that it is told.

So imagine a pcp client that may have a long-standing connection to pmcd. And that same client may have additional short-lived connections (each quickly closed). Then, due to the libpcp socket sharing, pmcd would see one client, with an increasing cardinality of ctxnums it's seen. It would allocate & retain new hash table entries for each of those short-lived connections, even though they were closed by the client. Over time, this can lead to unbounded memory consumption by pmcd, even though the client is doing nothing wrong.

FWIW, that the alternative showstopper workaround I proposed - disabling connection sharing - solves this problem also.

[goodwinos]

the new hashing code uses contexts[contexts_map[i]] so do you mean context_map[] may grow without bound? Surely destroyed context entries could be reused ..? (perhaps only once we reach ctxnum>2048) ?

[fche]

I'm talking about the pmcd-side hash table, not libpcp. It's an elaboration of the point in the middle of https://groups.io/g/pcp/message/15768?p=created,0,,20,2,0,11465260&offset=, that a single client can induce indefinitely large memory use on the server. That's new.

[dbrolley]

fche is referring to the the 'profile' field of ClientInfo (src/pmcd/src/client.h). Before my change, it was an array indexed directly by ctxnum. Old profiles were freed and slots were reused when ctxnum's were reused.

The change to monotonically increasing ctxnum created two problems:

1) The array now grows much larger than before and is much more sparse than before. Even cases where a client has only one profile could end up with a huge array, indexed by a large ctxnum, with no other slots in use.

2) There is no longer any way to know when old profiles can be deleted. So for clients which stay connected for a long time, opening and closing many contexts, such as pmmgr, the array will grow even larger with each new context.

When I changed this array to a hash table, this addressed problem 1. However, the new hash table can still grow arbitrarily due to problem 2. This issue was opened by fche so that we do not forget about problem 2.

[fche]

This little test program indicates the situation. Luckily, these hash tables entries are small, so the memory growth is slow. foo.txt

[kmcdonell]

Sorry to have caused so much pain here and then disappear to Hawaii for 2 weeks.

When I get back I think the correct fix is to use the context slot index from libpcp not the ctxnum when sending the profile pdu to pmcd. This would be no change for pmcd from the state before I changed the ctxnum rules in libpcp ... a bounded and small context numbers at pmcd and the client would still see monotonic increasing context handles.

The existing libpcp code would resend the profile (as required) when a new context re-used a slot in the active contexts array.

[kmcdonell]

@fche I'm pretty sure that as of commit 0d61031 (back in June) the approach I suggested above has been implemented and this issue is now done and dusted.

I'm closing, but please re-open with more info if you don't agree.

[fche]

The OOM possibility remains, because the bulk of that commit was for the pcp client side. A hostile pcp client could still deliberately repeatedly trigger the __pmHashAdd() path in pmcd/src/dupdus.c, and eventually OOM it. But it's a pretty slow OOM, maybe not worth worrying about.