Skip to content

Commit f1712c7

Browse files
Eric Dumazetdavem330
Eric Dumazet
authored and
davem330
committed
can: Fix kernel panic at security_sock_rcv_skb
Zhang Yanmin reported crashes [1] and provided a patch adding a synchronize_rcu() call in can_rx_unregister() The main problem seems that the sockets themselves are not RCU protected. If CAN uses RCU for delivery, then sockets should be freed only after one RCU grace period. Recent kernels could use sock_set_flag(sk, SOCK_RCU_FREE), but let's ease stable backports with the following fix instead. [1] BUG: unable to handle kernel NULL pointer dereference at (null) IP: [<ffffffff81495e25>] selinux_socket_sock_rcv_skb+0x65/0x2a0 Call Trace: <IRQ> [<ffffffff81485d8c>] security_sock_rcv_skb+0x4c/0x60 [<ffffffff81d55771>] sk_filter+0x41/0x210 [<ffffffff81d12913>] sock_queue_rcv_skb+0x53/0x3a0 [<ffffffff81f0a2b3>] raw_rcv+0x2a3/0x3c0 [<ffffffff81f06eab>] can_rcv_filter+0x12b/0x370 [<ffffffff81f07af9>] can_receive+0xd9/0x120 [<ffffffff81f07beb>] can_rcv+0xab/0x100 [<ffffffff81d362ac>] __netif_receive_skb_core+0xd8c/0x11f0 [<ffffffff81d36734>] __netif_receive_skb+0x24/0xb0 [<ffffffff81d37f67>] process_backlog+0x127/0x280 [<ffffffff81d36f7b>] net_rx_action+0x33b/0x4f0 [<ffffffff810c88d4>] __do_softirq+0x184/0x440 [<ffffffff81f9e86c>] do_softirq_own_stack+0x1c/0x30 <EOI> [<ffffffff810c76fb>] do_softirq.part.18+0x3b/0x40 [<ffffffff810c8bed>] do_softirq+0x1d/0x20 [<ffffffff81d30085>] netif_rx_ni+0xe5/0x110 [<ffffffff8199cc87>] slcan_receive_buf+0x507/0x520 [<ffffffff8167ef7c>] flush_to_ldisc+0x21c/0x230 [<ffffffff810e3baf>] process_one_work+0x24f/0x670 [<ffffffff810e44ed>] worker_thread+0x9d/0x6f0 [<ffffffff810e4450>] ? rescuer_thread+0x480/0x480 [<ffffffff810ebafc>] kthread+0x12c/0x150 [<ffffffff81f9ccef>] ret_from_fork+0x3f/0x70 Reported-by: Zhang Yanmin <yanmin.zhang@intel.com> Signed-off-by: Eric Dumazet <edumazet@google.com> Acked-by: Oliver Hartkopp <socketcan@hartkopp.net> Signed-off-by: David S. Miller <davem@davemloft.net>
1 parent dc97a89 commit f1712c7

File tree

6 files changed

+20
-12
lines changed

6 files changed

+20
-12
lines changed

‎include/linux/can/core.h

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -45,10 +45,9 @@ struct can_proto {
4545
extern int can_proto_register(const struct can_proto *cp);
4646
extern void can_proto_unregister(const struct can_proto *cp);
4747

48-
extern int can_rx_register(struct net_device *dev, canid_t can_id,
49-
canid_t mask,
50-
void (*func)(struct sk_buff *, void *),
51-
void *data, char *ident);
48+
int can_rx_register(struct net_device *dev, canid_t can_id, canid_t mask,
49+
void (*func)(struct sk_buff *, void *),
50+
void *data, char *ident, struct sock *sk);
5251

5352
extern void can_rx_unregister(struct net_device *dev, canid_t can_id,
5453
canid_t mask,

‎net/can/af_can.c

Lines changed: 10 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -445,6 +445,7 @@ static struct hlist_head *find_rcv_list(canid_t *can_id, canid_t *mask,
445445
* @func: callback function on filter match
446446
* @data: returned parameter for callback function
447447
* @ident: string for calling module identification
448+
* @sk: socket pointer (might be NULL)
448449
*
449450
* Description:
450451
* Invokes the callback function with the received sk_buff and the given
@@ -468,7 +469,7 @@ static struct hlist_head *find_rcv_list(canid_t *can_id, canid_t *mask,
468469
*/
469470
int can_rx_register(struct net_device *dev, canid_t can_id, canid_t mask,
470471
void (*func)(struct sk_buff *, void *), void *data,
471-
char *ident)
472+
char *ident, struct sock *sk)
472473
{
473474
struct receiver *r;
474475
struct hlist_head *rl;
@@ -496,6 +497,7 @@ int can_rx_register(struct net_device *dev, canid_t can_id, canid_t mask,
496497
r->func = func;
497498
r->data = data;
498499
r->ident = ident;
500+
r->sk = sk;
499501

500502
hlist_add_head_rcu(&r->list, rl);
501503
d->entries++;
@@ -520,8 +522,11 @@ EXPORT_SYMBOL(can_rx_register);
520522
static void can_rx_delete_receiver(struct rcu_head *rp)
521523
{
522524
struct receiver *r = container_of(rp, struct receiver, rcu);
525+
struct sock *sk = r->sk;
523526

524527
kmem_cache_free(rcv_cache, r);
528+
if (sk)
529+
sock_put(sk);
525530
}
526531

527532
/**
@@ -596,8 +601,11 @@ void can_rx_unregister(struct net_device *dev, canid_t can_id, canid_t mask,
596601
spin_unlock(&can_rcvlists_lock);
597602

598603
/* schedule the receiver item for deletion */
599-
if (r)
604+
if (r) {
605+
if (r->sk)
606+
sock_hold(r->sk);
600607
call_rcu(&r->rcu, can_rx_delete_receiver);
608+
}
601609
}
602610
EXPORT_SYMBOL(can_rx_unregister);
603611

‎net/can/af_can.h

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -50,13 +50,14 @@
5050

5151
struct receiver {
5252
struct hlist_node list;
53-
struct rcu_head rcu;
5453
canid_t can_id;
5554
canid_t mask;
5655
unsigned long matches;
5756
void (*func)(struct sk_buff *, void *);
5857
void *data;
5958
char *ident;
59+
struct sock *sk;
60+
struct rcu_head rcu;
6061
};
6162

6263
#define CAN_SFF_RCV_ARRAY_SZ (1 << CAN_SFF_ID_BITS)

‎net/can/bcm.c

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1216,7 +1216,7 @@ static int bcm_rx_setup(struct bcm_msg_head *msg_head, struct msghdr *msg,
12161216
err = can_rx_register(dev, op->can_id,
12171217
REGMASK(op->can_id),
12181218
bcm_rx_handler, op,
1219-
"bcm");
1219+
"bcm", sk);
12201220

12211221
op->rx_reg_dev = dev;
12221222
dev_put(dev);
@@ -1225,7 +1225,7 @@ static int bcm_rx_setup(struct bcm_msg_head *msg_head, struct msghdr *msg,
12251225
} else
12261226
err = can_rx_register(NULL, op->can_id,
12271227
REGMASK(op->can_id),
1228-
bcm_rx_handler, op, "bcm");
1228+
bcm_rx_handler, op, "bcm", sk);
12291229
if (err) {
12301230
/* this bcm rx op is broken -> remove it */
12311231
list_del(&op->list);

‎net/can/gw.c

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -442,7 +442,7 @@ static inline int cgw_register_filter(struct cgw_job *gwj)
442442
{
443443
return can_rx_register(gwj->src.dev, gwj->ccgw.filter.can_id,
444444
gwj->ccgw.filter.can_mask, can_can_gw_rcv,
445-
gwj, "gw");
445+
gwj, "gw", NULL);
446446
}
447447

448448
static inline void cgw_unregister_filter(struct cgw_job *gwj)

‎net/can/raw.c

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -190,7 +190,7 @@ static int raw_enable_filters(struct net_device *dev, struct sock *sk,
190190
for (i = 0; i < count; i++) {
191191
err = can_rx_register(dev, filter[i].can_id,
192192
filter[i].can_mask,
193-
raw_rcv, sk, "raw");
193+
raw_rcv, sk, "raw", sk);
194194
if (err) {
195195
/* clean up successfully registered filters */
196196
while (--i >= 0)
@@ -211,7 +211,7 @@ static int raw_enable_errfilter(struct net_device *dev, struct sock *sk,
211211

212212
if (err_mask)
213213
err = can_rx_register(dev, 0, err_mask | CAN_ERR_FLAG,
214-
raw_rcv, sk, "raw");
214+
raw_rcv, sk, "raw", sk);
215215

216216
return err;
217217
}

0 commit comments

Comments
 (0)