Skip to content

Commit 8d0caed

Browse files
Tetsuo Handamarckleinebudde
Tetsuo Handa
authored and
marckleinebudde
committed
can: bcm/raw/isotp: use per module netdevice notifier
syzbot is reporting hung task at register_netdevice_notifier() [1] and unregister_netdevice_notifier() [2], for cleanup_net() might perform time consuming operations while CAN driver's raw/bcm/isotp modules are calling {register,unregister}_netdevice_notifier() on each socket. Change raw/bcm/isotp modules to call register_netdevice_notifier() from module's __init function and call unregister_netdevice_notifier() from module's __exit function, as with gw/j1939 modules are doing. Link: https://syzkaller.appspot.com/bug?id=391b9498827788b3cc6830226d4ff5be87107c30 [1] Link: https://syzkaller.appspot.com/bug?id=1724d278c83ca6e6df100a2e320c10d991cf2bce [2] Link: https://lore.kernel.org/r/[email protected] Cc: linux-stable <[email protected]> Reported-by: syzbot <[email protected]> Reported-by: syzbot <[email protected]> Reviewed-by: Kirill Tkhai <[email protected]> Tested-by: syzbot <[email protected]> Tested-by: Oliver Hartkopp <[email protected]> Signed-off-by: Tetsuo Handa <[email protected]> Signed-off-by: Marc Kleine-Budde <[email protected]>
1 parent 2030043 commit 8d0caed

File tree

3 files changed

+142
-40
lines changed

3 files changed

+142
-40
lines changed

‎net/can/bcm.c

Lines changed: 46 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -125,14 +125,18 @@ struct bcm_sock {
125125
struct sock sk;
126126
int bound;
127127
int ifindex;
128-
struct notifier_block notifier;
128+
struct list_head notifier;
129129
struct list_head rx_ops;
130130
struct list_head tx_ops;
131131
unsigned long dropped_usr_msgs;
132132
struct proc_dir_entry *bcm_proc_read;
133133
char procname [32]; /* inode number in decimal with \0 */
134134
};
135135

136+
static LIST_HEAD(bcm_notifier_list);
137+
static DEFINE_SPINLOCK(bcm_notifier_lock);
138+
static struct bcm_sock *bcm_busy_notifier;
139+
136140
static inline struct bcm_sock *bcm_sk(const struct sock *sk)
137141
{
138142
return (struct bcm_sock *)sk;
@@ -1378,20 +1382,15 @@ static int bcm_sendmsg(struct socket *sock, struct msghdr *msg, size_t size)
13781382
/*
13791383
* notification handler for netdevice status changes
13801384
*/
1381-
static int bcm_notifier(struct notifier_block *nb, unsigned long msg,
1382-
void *ptr)
1385+
static void bcm_notify(struct bcm_sock *bo, unsigned long msg,
1386+
struct net_device *dev)
13831387
{
1384-
struct net_device *dev = netdev_notifier_info_to_dev(ptr);
1385-
struct bcm_sock *bo = container_of(nb, struct bcm_sock, notifier);
13861388
struct sock *sk = &bo->sk;
13871389
struct bcm_op *op;
13881390
int notify_enodev = 0;
13891391

13901392
if (!net_eq(dev_net(dev), sock_net(sk)))
1391-
return NOTIFY_DONE;
1392-
1393-
if (dev->type != ARPHRD_CAN)
1394-
return NOTIFY_DONE;
1393+
return;
13951394

13961395
switch (msg) {
13971396

@@ -1426,7 +1425,28 @@ static int bcm_notifier(struct notifier_block *nb, unsigned long msg,
14261425
sk->sk_error_report(sk);
14271426
}
14281427
}
1428+
}
14291429

1430+
static int bcm_notifier(struct notifier_block *nb, unsigned long msg,
1431+
void *ptr)
1432+
{
1433+
struct net_device *dev = netdev_notifier_info_to_dev(ptr);
1434+
1435+
if (dev->type != ARPHRD_CAN)
1436+
return NOTIFY_DONE;
1437+
if (msg != NETDEV_UNREGISTER && msg != NETDEV_DOWN)
1438+
return NOTIFY_DONE;
1439+
if (unlikely(bcm_busy_notifier)) /* Check for reentrant bug. */
1440+
return NOTIFY_DONE;
1441+
1442+
spin_lock(&bcm_notifier_lock);
1443+
list_for_each_entry(bcm_busy_notifier, &bcm_notifier_list, notifier) {
1444+
spin_unlock(&bcm_notifier_lock);
1445+
bcm_notify(bcm_busy_notifier, msg, dev);
1446+
spin_lock(&bcm_notifier_lock);
1447+
}
1448+
bcm_busy_notifier = NULL;
1449+
spin_unlock(&bcm_notifier_lock);
14301450
return NOTIFY_DONE;
14311451
}
14321452

@@ -1446,9 +1466,9 @@ static int bcm_init(struct sock *sk)
14461466
INIT_LIST_HEAD(&bo->rx_ops);
14471467

14481468
/* set notifier */
1449-
bo->notifier.notifier_call = bcm_notifier;
1450-
1451-
register_netdevice_notifier(&bo->notifier);
1469+
spin_lock(&bcm_notifier_lock);
1470+
list_add_tail(&bo->notifier, &bcm_notifier_list);
1471+
spin_unlock(&bcm_notifier_lock);
14521472

14531473
return 0;
14541474
}
@@ -1471,7 +1491,14 @@ static int bcm_release(struct socket *sock)
14711491

14721492
/* remove bcm_ops, timer, rx_unregister(), etc. */
14731493

1474-
unregister_netdevice_notifier(&bo->notifier);
1494+
spin_lock(&bcm_notifier_lock);
1495+
while (bcm_busy_notifier == bo) {
1496+
spin_unlock(&bcm_notifier_lock);
1497+
schedule_timeout_uninterruptible(1);
1498+
spin_lock(&bcm_notifier_lock);
1499+
}
1500+
list_del(&bo->notifier);
1501+
spin_unlock(&bcm_notifier_lock);
14751502

14761503
lock_sock(sk);
14771504

@@ -1692,6 +1719,10 @@ static struct pernet_operations canbcm_pernet_ops __read_mostly = {
16921719
.exit = canbcm_pernet_exit,
16931720
};
16941721

1722+
static struct notifier_block canbcm_notifier = {
1723+
.notifier_call = bcm_notifier
1724+
};
1725+
16951726
static int __init bcm_module_init(void)
16961727
{
16971728
int err;
@@ -1705,12 +1736,14 @@ static int __init bcm_module_init(void)
17051736
}
17061737

17071738
register_pernet_subsys(&canbcm_pernet_ops);
1739+
register_netdevice_notifier(&canbcm_notifier);
17081740
return 0;
17091741
}
17101742

17111743
static void __exit bcm_module_exit(void)
17121744
{
17131745
can_proto_unregister(&bcm_can_proto);
1746+
unregister_netdevice_notifier(&canbcm_notifier);
17141747
unregister_pernet_subsys(&canbcm_pernet_ops);
17151748
}
17161749

‎net/can/isotp.c

Lines changed: 48 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -143,10 +143,14 @@ struct isotp_sock {
143143
u32 force_tx_stmin;
144144
u32 force_rx_stmin;
145145
struct tpcon rx, tx;
146-
struct notifier_block notifier;
146+
struct list_head notifier;
147147
wait_queue_head_t wait;
148148
};
149149

150+
static LIST_HEAD(isotp_notifier_list);
151+
static DEFINE_SPINLOCK(isotp_notifier_lock);
152+
static struct isotp_sock *isotp_busy_notifier;
153+
150154
static inline struct isotp_sock *isotp_sk(const struct sock *sk)
151155
{
152156
return (struct isotp_sock *)sk;
@@ -1013,7 +1017,14 @@ static int isotp_release(struct socket *sock)
10131017
/* wait for complete transmission of current pdu */
10141018
wait_event_interruptible(so->wait, so->tx.state == ISOTP_IDLE);
10151019

1016-
unregister_netdevice_notifier(&so->notifier);
1020+
spin_lock(&isotp_notifier_lock);
1021+
while (isotp_busy_notifier == so) {
1022+
spin_unlock(&isotp_notifier_lock);
1023+
schedule_timeout_uninterruptible(1);
1024+
spin_lock(&isotp_notifier_lock);
1025+
}
1026+
list_del(&so->notifier);
1027+
spin_unlock(&isotp_notifier_lock);
10171028

10181029
lock_sock(sk);
10191030

@@ -1317,21 +1328,16 @@ static int isotp_getsockopt(struct socket *sock, int level, int optname,
13171328
return 0;
13181329
}
13191330

1320-
static int isotp_notifier(struct notifier_block *nb, unsigned long msg,
1321-
void *ptr)
1331+
static void isotp_notify(struct isotp_sock *so, unsigned long msg,
1332+
struct net_device *dev)
13221333
{
1323-
struct net_device *dev = netdev_notifier_info_to_dev(ptr);
1324-
struct isotp_sock *so = container_of(nb, struct isotp_sock, notifier);
13251334
struct sock *sk = &so->sk;
13261335

13271336
if (!net_eq(dev_net(dev), sock_net(sk)))
1328-
return NOTIFY_DONE;
1329-
1330-
if (dev->type != ARPHRD_CAN)
1331-
return NOTIFY_DONE;
1337+
return;
13321338

13331339
if (so->ifindex != dev->ifindex)
1334-
return NOTIFY_DONE;
1340+
return;
13351341

13361342
switch (msg) {
13371343
case NETDEV_UNREGISTER:
@@ -1357,7 +1363,28 @@ static int isotp_notifier(struct notifier_block *nb, unsigned long msg,
13571363
sk->sk_error_report(sk);
13581364
break;
13591365
}
1366+
}
13601367

1368+
static int isotp_notifier(struct notifier_block *nb, unsigned long msg,
1369+
void *ptr)
1370+
{
1371+
struct net_device *dev = netdev_notifier_info_to_dev(ptr);
1372+
1373+
if (dev->type != ARPHRD_CAN)
1374+
return NOTIFY_DONE;
1375+
if (msg != NETDEV_UNREGISTER && msg != NETDEV_DOWN)
1376+
return NOTIFY_DONE;
1377+
if (unlikely(isotp_busy_notifier)) /* Check for reentrant bug. */
1378+
return NOTIFY_DONE;
1379+
1380+
spin_lock(&isotp_notifier_lock);
1381+
list_for_each_entry(isotp_busy_notifier, &isotp_notifier_list, notifier) {
1382+
spin_unlock(&isotp_notifier_lock);
1383+
isotp_notify(isotp_busy_notifier, msg, dev);
1384+
spin_lock(&isotp_notifier_lock);
1385+
}
1386+
isotp_busy_notifier = NULL;
1387+
spin_unlock(&isotp_notifier_lock);
13611388
return NOTIFY_DONE;
13621389
}
13631390

@@ -1394,8 +1421,9 @@ static int isotp_init(struct sock *sk)
13941421

13951422
init_waitqueue_head(&so->wait);
13961423

1397-
so->notifier.notifier_call = isotp_notifier;
1398-
register_netdevice_notifier(&so->notifier);
1424+
spin_lock(&isotp_notifier_lock);
1425+
list_add_tail(&so->notifier, &isotp_notifier_list);
1426+
spin_unlock(&isotp_notifier_lock);
13991427

14001428
return 0;
14011429
}
@@ -1442,6 +1470,10 @@ static const struct can_proto isotp_can_proto = {
14421470
.prot = &isotp_proto,
14431471
};
14441472

1473+
static struct notifier_block canisotp_notifier = {
1474+
.notifier_call = isotp_notifier
1475+
};
1476+
14451477
static __init int isotp_module_init(void)
14461478
{
14471479
int err;
@@ -1451,13 +1483,16 @@ static __init int isotp_module_init(void)
14511483
err = can_proto_register(&isotp_can_proto);
14521484
if (err < 0)
14531485
pr_err("can: registration of isotp protocol failed\n");
1486+
else
1487+
register_netdevice_notifier(&canisotp_notifier);
14541488

14551489
return err;
14561490
}
14571491

14581492
static __exit void isotp_module_exit(void)
14591493
{
14601494
can_proto_unregister(&isotp_can_proto);
1495+
unregister_netdevice_notifier(&canisotp_notifier);
14611496
}
14621497

14631498
module_init(isotp_module_init);

‎net/can/raw.c

Lines changed: 48 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -83,7 +83,7 @@ struct raw_sock {
8383
struct sock sk;
8484
int bound;
8585
int ifindex;
86-
struct notifier_block notifier;
86+
struct list_head notifier;
8787
int loopback;
8888
int recv_own_msgs;
8989
int fd_frames;
@@ -95,6 +95,10 @@ struct raw_sock {
9595
struct uniqframe __percpu *uniq;
9696
};
9797

98+
static LIST_HEAD(raw_notifier_list);
99+
static DEFINE_SPINLOCK(raw_notifier_lock);
100+
static struct raw_sock *raw_busy_notifier;
101+
98102
/* Return pointer to store the extra msg flags for raw_recvmsg().
99103
* We use the space of one unsigned int beyond the 'struct sockaddr_can'
100104
* in skb->cb.
@@ -263,21 +267,16 @@ static int raw_enable_allfilters(struct net *net, struct net_device *dev,
263267
return err;
264268
}
265269

266-
static int raw_notifier(struct notifier_block *nb,
267-
unsigned long msg, void *ptr)
270+
static void raw_notify(struct raw_sock *ro, unsigned long msg,
271+
struct net_device *dev)
268272
{
269-
struct net_device *dev = netdev_notifier_info_to_dev(ptr);
270-
struct raw_sock *ro = container_of(nb, struct raw_sock, notifier);
271273
struct sock *sk = &ro->sk;
272274

273275
if (!net_eq(dev_net(dev), sock_net(sk)))
274-
return NOTIFY_DONE;
275-
276-
if (dev->type != ARPHRD_CAN)
277-
return NOTIFY_DONE;
276+
return;
278277

279278
if (ro->ifindex != dev->ifindex)
280-
return NOTIFY_DONE;
279+
return;
281280

282281
switch (msg) {
283282
case NETDEV_UNREGISTER:
@@ -305,7 +304,28 @@ static int raw_notifier(struct notifier_block *nb,
305304
sk->sk_error_report(sk);
306305
break;
307306
}
307+
}
308+
309+
static int raw_notifier(struct notifier_block *nb, unsigned long msg,
310+
void *ptr)
311+
{
312+
struct net_device *dev = netdev_notifier_info_to_dev(ptr);
313+
314+
if (dev->type != ARPHRD_CAN)
315+
return NOTIFY_DONE;
316+
if (msg != NETDEV_UNREGISTER && msg != NETDEV_DOWN)
317+
return NOTIFY_DONE;
318+
if (unlikely(raw_busy_notifier)) /* Check for reentrant bug. */
319+
return NOTIFY_DONE;
308320

321+
spin_lock(&raw_notifier_lock);
322+
list_for_each_entry(raw_busy_notifier, &raw_notifier_list, notifier) {
323+
spin_unlock(&raw_notifier_lock);
324+
raw_notify(raw_busy_notifier, msg, dev);
325+
spin_lock(&raw_notifier_lock);
326+
}
327+
raw_busy_notifier = NULL;
328+
spin_unlock(&raw_notifier_lock);
309329
return NOTIFY_DONE;
310330
}
311331

@@ -334,9 +354,9 @@ static int raw_init(struct sock *sk)
334354
return -ENOMEM;
335355

336356
/* set notifier */
337-
ro->notifier.notifier_call = raw_notifier;
338-
339-
register_netdevice_notifier(&ro->notifier);
357+
spin_lock(&raw_notifier_lock);
358+
list_add_tail(&ro->notifier, &raw_notifier_list);
359+
spin_unlock(&raw_notifier_lock);
340360

341361
return 0;
342362
}
@@ -351,7 +371,14 @@ static int raw_release(struct socket *sock)
351371

352372
ro = raw_sk(sk);
353373

354-
unregister_netdevice_notifier(&ro->notifier);
374+
spin_lock(&raw_notifier_lock);
375+
while (raw_busy_notifier == ro) {
376+
spin_unlock(&raw_notifier_lock);
377+
schedule_timeout_uninterruptible(1);
378+
spin_lock(&raw_notifier_lock);
379+
}
380+
list_del(&ro->notifier);
381+
spin_unlock(&raw_notifier_lock);
355382

356383
lock_sock(sk);
357384

@@ -889,6 +916,10 @@ static const struct can_proto raw_can_proto = {
889916
.prot = &raw_proto,
890917
};
891918

919+
static struct notifier_block canraw_notifier = {
920+
.notifier_call = raw_notifier
921+
};
922+
892923
static __init int raw_module_init(void)
893924
{
894925
int err;
@@ -898,13 +929,16 @@ static __init int raw_module_init(void)
898929
err = can_proto_register(&raw_can_proto);
899930
if (err < 0)
900931
pr_err("can: registration of raw protocol failed\n");
932+
else
933+
register_netdevice_notifier(&canraw_notifier);
901934

902935
return err;
903936
}
904937

905938
static __exit void raw_module_exit(void)
906939
{
907940
can_proto_unregister(&raw_can_proto);
941+
unregister_netdevice_notifier(&canraw_notifier);
908942
}
909943

910944
module_init(raw_module_init);

0 commit comments

Comments
 (0)