On 7/19/21 11:17, Yutian Yang wrote:
> This patch enables accounting for key objects and auth record objects.
> Allocation of the objects are triggerable by syscalls from userspace.
>
> We have written a PoC to show that the missing-charging objects lead to
> breaking memcg limits. The PoC program takes around 2.2GB unaccounted
> memory, while it is charged for only 24MB memory usage. We evaluate the
> PoC on QEMU x86_64 v5.2.90 + Linux kernel v5.10.19 + Debian buster. All
> the limitations including ulimits and sysctl variables are set as default.
> Specifically, we set kernel.keys.maxbytes = 20000 and
> kernel.keys.maxkeys = 200.
>
> /*------------------------- POC code ----------------------------*/
[skipped]
> /*-------------------------- end --------------------------------*/
I experimented with "keyctl request2 user debug: X:Y Z" inside the container
and found that the problem is still relevant and the proposed patch solves it
correctly.
I didn't find any complaints about this patch, could someone explain why
it wasn't applied? If no one objects, I'd like to push it.
> Signed-off-by: Yutian Yang <nglaive@xxxxxxxxx>
Reviewed-by: Vasily Averin <vvs@xxxxxxxxxx>
Thank you,
Vasily Averin
PS. Should I perhaps resend it?
> ---
> security/keys/key.c | 4 ++--
> security/keys/request_key_auth.c | 4 ++--
> 2 files changed, 4 insertions(+), 4 deletions(-)
>
> diff --git a/security/keys/key.c b/security/keys/key.c
> index e282c6179..925d85c2e 100644
> --- a/security/keys/key.c
> +++ b/security/keys/key.c
> @@ -279,7 +279,7 @@ struct key *key_alloc(struct key_type *type, const char *desc,
> goto no_memory_2;
>
> key->index_key.desc_len = desclen;
> - key->index_key.description = kmemdup(desc, desclen + 1, GFP_KERNEL);
> + key->index_key.description = kmemdup(desc, desclen + 1, GFP_KERNEL_ACCOUNT);
> if (!key->index_key.description)
> goto no_memory_3;
> key->index_key.type = type;
> @@ -1198,7 +1198,7 @@ void __init key_init(void)
> {
> /* allocate a slab in which we can store keys */
> key_jar = kmem_cache_create("key_jar", sizeof(struct key),
> - 0, SLAB_HWCACHE_ALIGN|SLAB_PANIC, NULL);
> + 0, SLAB_HWCACHE_ALIGN|SLAB_PANIC|SLAB_ACCOUNT, NULL);
>
> /* add the special key types */
> list_add_tail(&key_type_keyring.link, &key_types_list);
> diff --git a/security/keys/request_key_auth.c b/security/keys/request_key_auth.c
> index 41e973500..ed50a100a 100644
> --- a/security/keys/request_key_auth.c
> +++ b/security/keys/request_key_auth.c
> @@ -171,10 +171,10 @@ struct key *request_key_auth_new(struct key *target, const char *op,
> kenter("%d,", target->serial);
>
> /* allocate a auth record */
> - rka = kzalloc(sizeof(*rka), GFP_KERNEL);
> + rka = kzalloc(sizeof(*rka), GFP_KERNEL_ACCOUNT);
> if (!rka)
> goto error;
> - rka->callout_info = kmemdup(callout_info, callout_len, GFP_KERNEL);
> + rka->callout_info = kmemdup(callout_info, callout_len, GFP_KERNEL_ACCOUNT);
> if (!rka->callout_info)
> goto error_free_rka;
> rka->callout_len = callout_len;
