Skip to content

Commit 49af1fa

Browse files
eddyz87Alexei Starovoitov
authored andcommitted
bpf: remove {update,get}_loop_entry functions
The previous patch switched read and precision tracking for iterator-based loops from state-graph-based loop tracking to control-flow-graph-based loop tracking. This patch removes the now-unused `update_loop_entry()` and `get_loop_entry()` functions, which were part of the state-graph-based logic. Signed-off-by: Eduard Zingerman <[email protected]> Link: https://lore.kernel.org/r/[email protected] Signed-off-by: Alexei Starovoitov <[email protected]>
1 parent 264d5eb commit 49af1fa

File tree

2 files changed

+1
-179
lines changed

2 files changed

+1
-179
lines changed

include/linux/bpf_verifier.h

Lines changed: 0 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -449,16 +449,6 @@ struct bpf_verifier_state {
449449
/* first and last insn idx of this verifier state */
450450
u32 first_insn_idx;
451451
u32 last_insn_idx;
452-
/* If this state is a part of states loop this field points to some
453-
* parent of this state such that:
454-
* - it is also a member of the same states loop;
455-
* - DFS states traversal starting from initial state visits loop_entry
456-
* state before this state.
457-
* Used to compute topmost loop entry for state loops.
458-
* State loops might appear because of open coded iterators logic.
459-
* See get_loop_entry() for more information.
460-
*/
461-
struct bpf_verifier_state *loop_entry;
462452
/* if this state is a backedge state then equal_state
463453
* records cached state to which this state is equal.
464454
*/
@@ -473,11 +463,6 @@ struct bpf_verifier_state {
473463
u32 dfs_depth;
474464
u32 callback_unroll_depth;
475465
u32 may_goto_depth;
476-
/* If this state was ever pointed-to by other state's loop_entry field
477-
* this flag would be set to true. Used to avoid freeing such states
478-
* while they are still in use.
479-
*/
480-
u32 used_as_loop_entry;
481466
};
482467

483468
#define bpf_get_spilled_reg(slot, frame, mask) \

kernel/bpf/verifier.c

Lines changed: 1 addition & 164 deletions
Original file line numberDiff line numberDiff line change
@@ -1682,7 +1682,7 @@ static void free_verifier_state(struct bpf_verifier_state *state,
16821682
kfree(state);
16831683
}
16841684

1685-
/* struct bpf_verifier_state->{parent,loop_entry} refer to states
1685+
/* struct bpf_verifier_state->parent refers to states
16861686
* that are in either of env->{expored_states,free_list}.
16871687
* In both cases the state is contained in struct bpf_verifier_state_list.
16881688
*/
@@ -1693,22 +1693,12 @@ static struct bpf_verifier_state_list *state_parent_as_list(struct bpf_verifier_
16931693
return NULL;
16941694
}
16951695

1696-
static struct bpf_verifier_state_list *state_loop_entry_as_list(struct bpf_verifier_state *st)
1697-
{
1698-
if (st->loop_entry)
1699-
return container_of(st->loop_entry, struct bpf_verifier_state_list, state);
1700-
return NULL;
1701-
}
1702-
17031696
static bool incomplete_read_marks(struct bpf_verifier_env *env,
17041697
struct bpf_verifier_state *st);
17051698

17061699
/* A state can be freed if it is no longer referenced:
17071700
* - is in the env->free_list;
17081701
* - has no children states;
1709-
* - is not used as loop_entry.
1710-
*
1711-
* Freeing a state can make it's loop_entry free-able.
17121702
*/
17131703
static void maybe_free_verifier_state(struct bpf_verifier_env *env,
17141704
struct bpf_verifier_state_list *sl)
@@ -1765,9 +1755,7 @@ static int copy_verifier_state(struct bpf_verifier_state *dst_state,
17651755
dst_state->last_insn_idx = src->last_insn_idx;
17661756
dst_state->dfs_depth = src->dfs_depth;
17671757
dst_state->callback_unroll_depth = src->callback_unroll_depth;
1768-
dst_state->used_as_loop_entry = src->used_as_loop_entry;
17691758
dst_state->may_goto_depth = src->may_goto_depth;
1770-
dst_state->loop_entry = src->loop_entry;
17711759
dst_state->equal_state = src->equal_state;
17721760
for (i = 0; i <= src->curframe; i++) {
17731761
dst = dst_state->frame[i];
@@ -1811,157 +1799,6 @@ static bool same_callsites(struct bpf_verifier_state *a, struct bpf_verifier_sta
18111799
return true;
18121800
}
18131801

1814-
/* Open coded iterators allow back-edges in the state graph in order to
1815-
* check unbounded loops that iterators.
1816-
*
1817-
* In is_state_visited() it is necessary to know if explored states are
1818-
* part of some loops in order to decide whether non-exact states
1819-
* comparison could be used:
1820-
* - non-exact states comparison establishes sub-state relation and uses
1821-
* read and precision marks to do so, these marks are propagated from
1822-
* children states and thus are not guaranteed to be final in a loop;
1823-
* - exact states comparison just checks if current and explored states
1824-
* are identical (and thus form a back-edge).
1825-
*
1826-
* Paper "A New Algorithm for Identifying Loops in Decompilation"
1827-
* by Tao Wei, Jian Mao, Wei Zou and Yu Chen [1] presents a convenient
1828-
* algorithm for loop structure detection and gives an overview of
1829-
* relevant terminology. It also has helpful illustrations.
1830-
*
1831-
* [1] https://api.semanticscholar.org/CorpusID:15784067
1832-
*
1833-
* We use a similar algorithm but because loop nested structure is
1834-
* irrelevant for verifier ours is significantly simpler and resembles
1835-
* strongly connected components algorithm from Sedgewick's textbook.
1836-
*
1837-
* Define topmost loop entry as a first node of the loop traversed in a
1838-
* depth first search starting from initial state. The goal of the loop
1839-
* tracking algorithm is to associate topmost loop entries with states
1840-
* derived from these entries.
1841-
*
1842-
* For each step in the DFS states traversal algorithm needs to identify
1843-
* the following situations:
1844-
*
1845-
* initial initial initial
1846-
* | | |
1847-
* V V V
1848-
* ... ... .---------> hdr
1849-
* | | | |
1850-
* V V | V
1851-
* cur .-> succ | .------...
1852-
* | | | | | |
1853-
* V | V | V V
1854-
* succ '-- cur | ... ...
1855-
* | | |
1856-
* | V V
1857-
* | succ <- cur
1858-
* | |
1859-
* | V
1860-
* | ...
1861-
* | |
1862-
* '----'
1863-
*
1864-
* (A) successor state of cur (B) successor state of cur or it's entry
1865-
* not yet traversed are in current DFS path, thus cur and succ
1866-
* are members of the same outermost loop
1867-
*
1868-
* initial initial
1869-
* | |
1870-
* V V
1871-
* ... ...
1872-
* | |
1873-
* V V
1874-
* .------... .------...
1875-
* | | | |
1876-
* V V V V
1877-
* .-> hdr ... ... ...
1878-
* | | | | |
1879-
* | V V V V
1880-
* | succ <- cur succ <- cur
1881-
* | | |
1882-
* | V V
1883-
* | ... ...
1884-
* | | |
1885-
* '----' exit
1886-
*
1887-
* (C) successor state of cur is a part of some loop but this loop
1888-
* does not include cur or successor state is not in a loop at all.
1889-
*
1890-
* Algorithm could be described as the following python code:
1891-
*
1892-
* traversed = set() # Set of traversed nodes
1893-
* entries = {} # Mapping from node to loop entry
1894-
* depths = {} # Depth level assigned to graph node
1895-
* path = set() # Current DFS path
1896-
*
1897-
* # Find outermost loop entry known for n
1898-
* def get_loop_entry(n):
1899-
* h = entries.get(n, None)
1900-
* while h in entries:
1901-
* h = entries[h]
1902-
* return h
1903-
*
1904-
* # Update n's loop entry if h comes before n in current DFS path.
1905-
* def update_loop_entry(n, h):
1906-
* if h in path and depths[entries.get(n, n)] < depths[n]:
1907-
* entries[n] = h1
1908-
*
1909-
* def dfs(n, depth):
1910-
* traversed.add(n)
1911-
* path.add(n)
1912-
* depths[n] = depth
1913-
* for succ in G.successors(n):
1914-
* if succ not in traversed:
1915-
* # Case A: explore succ and update cur's loop entry
1916-
* # only if succ's entry is in current DFS path.
1917-
* dfs(succ, depth + 1)
1918-
* h = entries.get(succ, None)
1919-
* update_loop_entry(n, h)
1920-
* else:
1921-
* # Case B or C depending on `h1 in path` check in update_loop_entry().
1922-
* update_loop_entry(n, succ)
1923-
* path.remove(n)
1924-
*
1925-
* To adapt this algorithm for use with verifier:
1926-
* - use st->branch == 0 as a signal that DFS of succ had been finished
1927-
* and cur's loop entry has to be updated (case A), handle this in
1928-
* update_branch_counts();
1929-
* - use st->branch > 0 as a signal that st is in the current DFS path;
1930-
* - handle cases B and C in is_state_visited().
1931-
*/
1932-
static struct bpf_verifier_state *get_loop_entry(struct bpf_verifier_env *env,
1933-
struct bpf_verifier_state *st)
1934-
{
1935-
struct bpf_verifier_state *topmost = st->loop_entry;
1936-
u32 steps = 0;
1937-
1938-
while (topmost && topmost->loop_entry) {
1939-
if (verifier_bug_if(steps++ > st->dfs_depth, env, "infinite loop"))
1940-
return ERR_PTR(-EFAULT);
1941-
topmost = topmost->loop_entry;
1942-
}
1943-
return topmost;
1944-
}
1945-
1946-
static void update_loop_entry(struct bpf_verifier_env *env,
1947-
struct bpf_verifier_state *cur, struct bpf_verifier_state *hdr)
1948-
{
1949-
/* The hdr->branches check decides between cases B and C in
1950-
* comment for get_loop_entry(). If hdr->branches == 0 then
1951-
* head's topmost loop entry is not in current DFS path,
1952-
* hence 'cur' and 'hdr' are not in the same loop and there is
1953-
* no need to update cur->loop_entry.
1954-
*/
1955-
if (hdr->branches && hdr->dfs_depth < (cur->loop_entry ?: cur)->dfs_depth) {
1956-
if (cur->loop_entry) {
1957-
cur->loop_entry->used_as_loop_entry--;
1958-
maybe_free_verifier_state(env, state_loop_entry_as_list(cur));
1959-
}
1960-
cur->loop_entry = hdr;
1961-
hdr->used_as_loop_entry++;
1962-
}
1963-
}
1964-
19651802
/* Return IP for a given frame in a call stack */
19661803
static u32 frame_insn_idx(struct bpf_verifier_state *st, u32 frame)
19671804
{

0 commit comments

Comments
 (0)