On 2022/10/28 15:13, Oliver Hartkopp wrote:
Hello,
On 28.10.22 05:33, Zhengchao Shao wrote:
It causes NULL pointer dereference when testing as following:
(a) use syscall(__NR_socket, 0x10ul, 3ul, 0) to create netlink socket.
(b) use syscall(__NR_sendmsg, ...) to create bond link device and vxcan
link device, and bind vxcan device to bond device (can also use
ifenslave command to bind vxcan device to bond device).
(c) use syscall(__NR_socket, 0x1dul, 3ul, 1) to create CAN socket.
(d) use syscall(__NR_bind, ...) to bind the bond device to CAN socket.
The bond device invokes the can-raw protocol registration interface to
receive CAN packets. However, ml_priv is not allocated to the dev,
dev_rcv_lists is assigned to NULL in can_rx_register(). In this case,
it will occur the NULL pointer dereference issue.
I can see the problem and see that the patch makes sense for
can_rx_register().
But for me the problem seems to be located in the bonding device.
A CAN interface with dev->type == ARPHRD_CAN *always* has the
dev->ml_priv and dev->ml_priv_type set correctly.
I'm not sure if a bonding device does the right thing by just 'claiming'
to be a CAN device (by setting dev->type to ARPHRD_CAN) but not taking
care of being a CAN device and taking care of ml_priv specifics.
This might also be the case in other ml_priv use cases.
Would it probably make sense to blacklist CAN devices in bonding devices?
Thanks & best regards,
Oliver
Hi Oliver:
Thank you for your review. The bond device inherits the type of
the slave device, but the bond device does not allocate ml_priv. I can
try to add ARPHRD_CAN to the blocklist of slave_dev. But I was
wondering, could there be other scenes with the same problem?
Zhengchao Shao
The following is the stack information:
BUG: kernel NULL pointer dereference, address: 0000000000000008
PGD 122a4067 P4D 122a4067 PUD 1223c067 PMD 0
Oops: 0000 [#1] PREEMPT SMP
RIP: 0010:can_rx_register+0x12d/0x1e0
Call Trace:
<TASK>
raw_enable_filters+0x8d/0x120
raw_enable_allfilters+0x3b/0x130
raw_bind+0x118/0x4f0
__sys_bind+0x163/0x1a0
__x64_sys_bind+0x1e/0x30
do_syscall_64+0x35/0x80
entry_SYSCALL_64_after_hwframe+0x63/0xcd
</TASK>
Fixes: 4e096a18867a ("net: introduce CAN specific pointer in the
struct net_device")
Signed-off-by: Zhengchao Shao <shaozhengchao@xxxxxxxxxx>
---
net/can/af_can.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/net/can/af_can.c b/net/can/af_can.c
index 9503ab10f9b8..ef2697f3ebcb 100644
--- a/net/can/af_can.c
+++ b/net/can/af_can.c
@@ -450,7 +450,7 @@ int can_rx_register(struct net *net, struct
net_device *dev, canid_t can_id,
/* insert new receiver (dev,canid,mask) -> (func,data) */
- if (dev && dev->type != ARPHRD_CAN)
+ if (dev && (dev->type != ARPHRD_CAN || dev->ml_priv_type !=
ML_PRIV_CAN))
return -ENODEV;
if (dev && !net_eq(net, dev_net(dev)))
