This is an AI-generated review of your patch. The human sending this
email has considered the AI review valid, or at least plausible.
Full review at:
https://sashiko.dev/#/patchset/[email protected]?part=3
---
llc: add KUnit tests for the connection state machine bounds
This commit adds a KUnit test suite for the LLC type 2 connection state
machine. It introduces regression tests for state transitions and array
indexing invariants.
> diff --git a/net/llc/tests/conn_kunit.c b/net/llc/tests/conn_kunit.c
> new file mode 100644
> index 0000000000000..86d3f122df0a6
> --- /dev/null
> +++ b/net/llc/tests/conn_kunit.c
> @@ -0,0 +1,146 @@
[ ... ]
> +static struct sk_buff *llc_conn_test_rx_pdu(struct kunit *test, struct sock
> *sk)
> +{
> + struct llc_conn_state_ev *ev;
> + struct llc_pdu_sn *pdu;
> + struct sk_buff *skb;
> +
> + skb = alloc_skb(sizeof(*pdu), GFP_KERNEL);
> + KUNIT_ASSERT_NOT_NULL(test, skb);
[Severity: Medium]
Will this leak unmanaged resources if the assertion fails?
If alloc_skb() fails, KUNIT_ASSERT_NOT_NULL() terminates the test case
abruptly. Any previously allocated resources that are not registered with
KUnit's cleanup mechanism (like kunit_add_action()) will bypass their
manual cleanup routines.
[ ... ]
> +static void llc_conn_state_process_out_of_svc(struct kunit *test)
> +{
> + struct sk_buff *first, *second;
> + struct socket *sock;
> + struct sock *sk;
> +
> + sock = llc_conn_test_socket(test);
> + sk = sock->sk;
> +
> + first = llc_conn_test_rx_pdu(test, sk);
> + second = llc_conn_test_rx_pdu(test, sk);
[Severity: Medium]
For instance, if the second call to llc_conn_test_rx_pdu() triggers an
assertion failure, does this code leak the socket and the first sk_buff?
The sock is allocated in llc_conn_test_socket(), and the first sk_buff is
allocated just above. Because KUNIT_ASSERT_NOT_NULL() causes an immediate
abort on failure, the execution would skip the sock_release(sock) call at the
end of this function, permanently leaking both the socket and the sk_buff.
> +
> + lock_sock(sk);
> + KUNIT_EXPECT_EQ(test, llc_sk(sk)->state, LLC_CONN_STATE_ADM);
