Skip to content

Commit

Permalink
can: Fix kernel panic at security_sock_rcv_skb
Browse files Browse the repository at this point in the history 
[ Upstream commit f1712c7 ]

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>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
  • Loading branch information
@gregkh
Eric Dumazet authored and gregkh committed Feb 18, 2017
1 parent 702c0ce commit e6c654c
Show file tree
Hide file tree
Showing 6 changed files with 20 additions and 12 deletions.
7 changes: 3 additions & 4 deletions include/linux/can/core.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -45,10 +45,9 @@ struct can_proto {
extern int can_proto_register(const struct can_proto *cp);
extern void can_proto_unregister(const struct can_proto *cp);

extern int can_rx_register(struct net_device *dev, canid_t can_id,
canid_t mask,
void (*func)(struct sk_buff *, void *),
void *data, char *ident);
int can_rx_register(struct net_device *dev, canid_t can_id, canid_t mask,
void (*func)(struct sk_buff *, void *),
void *data, char *ident, struct sock *sk);

extern void can_rx_unregister(struct net_device *dev, canid_t can_id,
canid_t mask,
Expand Down
12 changes: 10 additions & 2 deletions net/can/af_can.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -445,6 +445,7 @@ static struct hlist_head *find_rcv_list(canid_t *can_id, canid_t *mask,
* @func: callback function on filter match
* @data: returned parameter for callback function
* @ident: string for calling module identification
* @sk: socket pointer (might be NULL)
*
* Description:
* Invokes the callback function with the received sk_buff and the given
Expand All @@ -468,7 +469,7 @@ static struct hlist_head *find_rcv_list(canid_t *can_id, canid_t *mask,
*/
int can_rx_register(struct net_device *dev, canid_t can_id, canid_t mask,
void (*func)(struct sk_buff *, void *), void *data,
char *ident)
char *ident, struct sock *sk)
{
struct receiver *r;
struct hlist_head *rl;
Expand Down Expand Up @@ -496,6 +497,7 @@ int can_rx_register(struct net_device *dev, canid_t can_id, canid_t mask,
r->func = func;
r->data = data;
r->ident = ident;
r->sk = sk;

hlist_add_head_rcu(&r->list, rl);
d->entries++;
Expand All @@ -520,8 +522,11 @@ EXPORT_SYMBOL(can_rx_register);
static void can_rx_delete_receiver(struct rcu_head *rp)
{
struct receiver *r = container_of(rp, struct receiver, rcu);
struct sock *sk = r->sk;

kmem_cache_free(rcv_cache, r);
if (sk)
sock_put(sk);
}

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

/* schedule the receiver item for deletion */
if (r)
if (r) {
if (r->sk)
sock_hold(r->sk);
call_rcu(&r->rcu, can_rx_delete_receiver);
}
}
EXPORT_SYMBOL(can_rx_unregister);

Expand Down
3 changes: 2 additions & 1 deletion net/can/af_can.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -50,13 +50,14 @@

struct receiver {
struct hlist_node list;
struct rcu_head rcu;
canid_t can_id;
canid_t mask;
unsigned long matches;
void (*func)(struct sk_buff *, void *);
void *data;
char *ident;
struct sock *sk;
struct rcu_head rcu;
};

#define CAN_SFF_RCV_ARRAY_SZ (1 << CAN_SFF_ID_BITS)
Expand Down
4 changes: 2 additions & 2 deletions net/can/bcm.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1179,7 +1179,7 @@ static int bcm_rx_setup(struct bcm_msg_head *msg_head, struct msghdr *msg,
err = can_rx_register(dev, op->can_id,
REGMASK(op->can_id),
bcm_rx_handler, op,
"bcm");
"bcm", sk);

op->rx_reg_dev = dev;
dev_put(dev);
Expand All @@ -1188,7 +1188,7 @@ static int bcm_rx_setup(struct bcm_msg_head *msg_head, struct msghdr *msg,
} else
err = can_rx_register(NULL, op->can_id,
REGMASK(op->can_id),
bcm_rx_handler, op, "bcm");
bcm_rx_handler, op, "bcm", sk);
if (err) {
/* this bcm rx op is broken -> remove it */
list_del(&op->list);
Expand Down
2 changes: 1 addition & 1 deletion net/can/gw.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -442,7 +442,7 @@ static inline int cgw_register_filter(struct cgw_job *gwj)
{
return can_rx_register(gwj->src.dev, gwj->ccgw.filter.can_id,
gwj->ccgw.filter.can_mask, can_can_gw_rcv,
gwj, "gw");
gwj, "gw", NULL);
}

static inline void cgw_unregister_filter(struct cgw_job *gwj)
Expand Down
4 changes: 2 additions & 2 deletions net/can/raw.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -190,7 +190,7 @@ static int raw_enable_filters(struct net_device *dev, struct sock *sk,
for (i = 0; i < count; i++) {
err = can_rx_register(dev, filter[i].can_id,
filter[i].can_mask,
raw_rcv, sk, "raw");
raw_rcv, sk, "raw", sk);
if (err) {
/* clean up successfully registered filters */
while (--i >= 0)
Expand All @@ -211,7 +211,7 @@ static int raw_enable_errfilter(struct net_device *dev, struct sock *sk,

if (err_mask)
err = can_rx_register(dev, 0, err_mask | CAN_ERR_FLAG,
raw_rcv, sk, "raw");
raw_rcv, sk, "raw", sk);

return err;
}
Expand Down

0 comments on commit e6c654c

Please sign in to comment.