NACK/Cmnt: [SRU][F][PATCH 0/2] CVE-2022-48740

[Mehmet Basaran]

As is, without any patches, this problem only exists in
"duplicate_policydb_cond_list()" because it does not set
"p->cond_list = NULL;" unlike "cond_read_list()".

However, for focal, bionic, and xenial we don't even have
"duplicate_policydb_cond_list()" yet. And cond_read_list() doesn't have
double free problem because it sets "p->cond_list = NULL;".

Here:
https://lore.kernel.org/linux-cve-announce/2024062002-CVE-2022-48740-a623@gregkh/T/
it mentions the affected versions. Following that, both
 - https://www.cve.org/CVERecord/?id=CVE-2022-48740 and
 - https://ubuntu.com/security/CVE-2022-48740
made a mistake as marking the break commit as 1da177e4c3f4. Break commit
should have been 60abd3181db29ea81742106cc0ac2e27fd05b418.

CVE card will be updated as "not vulnerable" for focal, bionic, xenial.

Nacked-by: Mehmet Basaran <mehmet.basaran at canonical.com>

-------------- next part --------------
Koichiro Den <koichiro.den at canonical.com> writes:

> [Impact]
>
> selinux: fix double free of cond_list on error paths
>
> On error path from cond_read_list() and duplicate_policydb_cond_list()
> the cond_list_destroy() gets called a second time in caller functions,
> resulting in NULL pointer deref.  Fix this by resetting the
> cond_list_len to 0 in cond_list_destroy(), making subsequent calls a
> noop.
>
> Also consistently reset the cond_list pointer to NULL after freeing.
>
> [Backport]
>
> Before the primary fix commit, I backported commit 60abd3181db2
> ("selinux: convert cond_list to array") seperately since otherwise
> the primary fix commit's message description would not make sense,
> and also it does not introduce any new features. To backport it,
> I adjusted the context due to another missing commit 06c2efe2cf3a
> ("selinux: simplify evaluate_cond_node()"). After that, the primary
> fix could be cleanly applied (i.e., just cherry-picked it).
>
> Note that the double free issue seems to have been also present in
> the older linked list version of cond_list in a different way.
>
> [Fix]
>
> Noble:  not affected
> Jammy:  not affected
> Focal:  Backport - one dependent commit backported as well, see [Backport]
> Bionic: fix sent to esm ML
> Xenial: fix sent to esm ML
> Trusty: won't fix
>
> [Test Case]
>
> Compile and boot tested
>
> [Where problems could occur]
>
> This fix affects selinux enabled environment, an issue with this fix would
> be visible to user via unpredicted system behavior or a system crash.
>
>
> Ondrej Mosnacek (1):
>   selinux: convert cond_list to array
>
> Vratislav Bendel (1):
>   selinux: fix double free of cond_list on error paths
>
>  security/selinux/include/conditional.h |  6 +--
>  security/selinux/selinuxfs.c           |  4 +-
>  security/selinux/ss/conditional.c      | 57 ++++++++++----------------
>  security/selinux/ss/conditional.h      |  3 +-
>  security/selinux/ss/policydb.c         |  2 +-
>  security/selinux/ss/policydb.h         |  3 +-
>  security/selinux/ss/services.c         | 28 ++++++-------
>  7 files changed, 45 insertions(+), 58 deletions(-)
>
> --
> 2.43.0
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 873 bytes
Desc: not available
URL: <https://lists.ubuntu.com/archives/kernel-team/attachments/20240912/a54d335d/attachment.sig>