Re: [PATCH net v2] can: isotp: check register_netdevice_notifier() error in module init

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:

Pre-existing issues:
- [High] Missing error checks for can_rx_register() in isotp_bind().
--

--- Patch [1]: [PATCH net v2] can: isotp: check register_netdevice_notifier() error in module init ---
commit 059042597b87e317cf8905dce3a8140167d9528a
Author: Minhong He <[email protected]>

can: isotp: check register_netdevice_notifier() error in module init

This commit registers the netdevice notifier before calling
can_proto_register() and checks its return value. If the protocol
registration fails, it properly unregisters the notifier before
returning the error.

> diff --git a/net/can/isotp.c b/net/can/isotp.c
> index 54becaf6898f1..ae6260e98a7a6 100644
> --- a/net/can/isotp.c
> +++ b/net/can/isotp.c

[ ... ]

This is a pre-existing issue, but I noticed a potential problem in
isotp_bind() when reviewing this file:

net/can/isotp.c:isotp_bind() {
	...
	if (isotp_register_rxid(so))
		can_rx_register(net, dev, rx_id, SINGLE_MASK(rx_id),
				isotp_rcv, sk, "isotp", sk);

	/* no consecutive frame echo skb in flight */
	so->cfecho = 0;

	/* register for echo skb's */
	can_rx_register(net, dev, tx_id, SINGLE_MASK(tx_id),
			isotp_rcv_echo, sk, "isotpe", sk);

	/* switch to new settings */
	so->ifindex = ifindex;
	...
}

[Severity: High]
Does this code silently ignore the return values of can_rx_register()?

If an unprivileged user inside a container calls bind() under memory
pressure, can_rx_register() could fail and return -ENOMEM. Because the
error is not handled, the socket would proceed to be marked as bound.

Later during close, isotp_release() calls can_rx_unregister()
unconditionally. Since the receiver was never successfully registered, this
would trigger the pr_warn("can: receive list entry not found...") inside
af_can.c.

Could a user repeatedly trigger this to generate unbounded kernel log spam,
creating a denial of service vector?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=1