ACK: [D/E][PATCH 0/1] Fix apparmor reference leak via AF_ALG
Colin Ian King
colin.king at canonical.com
Mon Jun 29 20:18:28 UTC 2020
On 29/06/2020 20:41, Mauricio Faria de Oliveira wrote:
> BugLink: https://bugs.launchpad.net/bugs/1883962
>
> Regarding the submission policies for this kernel SRU cycle:
> this patch does not have to be applied, but review/ack would be
> useful, if at all possible.
>
> This has already been submitted/reviewed for B/F but not D/E.
>
> I've recently learned that D's 5.0 kernel and E's 5.3 kernel
> should still be supported for a while on some custom kernels,
> thus sending it for D and E. Apologies for the inconvenience
> and late submission.
>
> [Impact]
>
> * Users of the Crypto (user-space) API (i.e., AF_ALG)
> can trigger refcount errors in AppArmor under high
> load (might lead to memory leak or use after free.)
>
> * There is a reference leak in AppArmor when af_alg_accept()
> calls security_sock_graft() and then security_sk_clone().
>
> * Both acquire a reference to a label, to assign it to the
> same pointer, but the latter does not release the former's
> acquired reference (before overwriting the pointer value.)
>
> * This reference leak builds up over time, and under high
> load can eventually overflow/underflow/saturate refcount,
> depending on which value it has when a program hits that.
>
> * The fix just checks if the pointer has an assigned label,
> then releases its acquired reference.
>
> [Test Case]
>
> * See comment #1 for the test-case 'aa-refcnt-af_alg.c'.
>
> * Exercise that code path indefinitely until it hits
> the refcount_t overflow/underflow/saturate message
> (or not, with the patch.) (see comment #4)
>
> * It's possible to monitor refcount values with kprobes,
> to confirm whether or not the problem is happening.
> (see comments #2 and #3)
>
> [Other Info]
>
> * Patch applied upstream on v5.8-rc1 [1]
> * Applied on Unstable (tag Ubuntu-5.8-5.8.0-0.1)
> * Not required on Groovy (still 5.4; should sync from Unstable)
> * Already sent/reviewed for Bionic and Focal.
> * Now sending for Disco and Eoan (for custom kernels support)
> both have been build tested.
>
> [1] https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit?id=3b646abc5bc6c0df649daea4c2c976bd4d47e4c8
>
> Mauricio Faria de Oliveira (1):
> apparmor: check/put label on apparmor_sk_clone_security()
>
> security/apparmor/lsm.c | 5 +++++
> 1 file changed, 5 insertions(+)
>
Acked-by: Colin Ian King <colin.king at canonical.com>
More information about the kernel-team
mailing list