netdev - [PATCH net-next v3 0/2] Fix dangling pointer at f6i->gc

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [thread-next>] [day] [month] [year] [list]

Message-Id: <20231213213735.434249-1-thinker.li@gmail.com>
Date: Wed, 13 Dec 2023 13:37:33 -0800
From: thinker.li@...il.com
To: netdev@...r.kernel.org,
	martin.lau@...ux.dev,
	kernel-team@...a.com,
	davem@...emloft.net,
	kuba@...nel.org,
	pabeni@...hat.com,
	dsahern@...nel.org,
	edumazet@...gle.com
Cc: sinquersw@...il.com,
	kuifeng@...a.com,
	Kui-Feng Lee <thinker.li@...il.com>
Subject: [PATCH net-next v3 0/2] Fix dangling pointer at f6i->gc_link.

From: Kui-Feng Lee <thinker.li@...il.com>

According to a report [1], f6i->gc_link may point to a free block
causing use-after-free. According the stacktraces in the report, it is
very likely that a f6i was added to the GC list after being removed
from the tree of a fib6_table. The reason this can happen is the
current implementation determines if a f6i is on a tree in a wrong
way. It believes a f6i is on a tree if f6i->fib6_table is not NULL.
However, f6i->fib6_table is not reset when f6i is removed from a tree.

The new solution is to check if f6i->fib6_node is not NULL as well.
f6i->fib6_node is set/or reset when the f6i is added/or removed from
from a tree. It can be used to determines if a f6i is on a tree
reliably.

The other change is to determine if a f6i is on a GC list.  The
current implementation relies on RTF_EXPIRES on fib6_flags. It needs
to consider if a f6i is on a tree as well. The new solution is
checking hlist_unhashed() on f6i->gc_link, a clear evidence, instead.

[1] https://lore.kernel.org/all/20231205173250.2982846-1-edumazet@google.com/

---
Major changes from v2:

 - Ensure dependencies of checks in the test cases.

Major changes from v1:

 - Split fib6_set_expires_locked() and fib6_clean_expires_locked()

 - Use hlist_unhashed() on gc_link instead of checking RTF_EXPIRES to
   determine if a f6i is on a GC list.

 - Add tests on toggling routes between permanent and temporary.

v2: https://lore.kernel.org/all/20231208194523.312416-1-thinker.li@gmail.com/
v1: https://lore.kernel.org/all/20231207221627.746324-1-thinker.li@gmail.com/

Kui-Feng Lee (2):
  net/ipv6: insert a f6i to a GC list only if the f6i is in a fib6_table
    tree.
  selftests: fib_tests: Add tests for toggling between w/ and w/o
    expires.

 include/net/ip6_fib.h                    | 46 +++++++++----
 net/ipv6/route.c                         |  6 +-
 tools/testing/selftests/net/fib_tests.sh | 82 +++++++++++++++++++++++-
 3 files changed, 118 insertions(+), 16 deletions(-)

-- 
2.34.1