mirror of
https://git.kernel.org/pub/scm/linux/kernel/git/stable/linux.git
synced 2024-10-12 03:26:26 +00:00
4224890edf
commite940e0895aupstream. There are two ref count variables controlling the free()ing of a socket: - struct sock::sk_refcnt - which is changed by sock_hold()/sock_put() - struct sock::sk_wmem_alloc - which accounts the memory allocated by the skbs in the send path. In case there are still TX skbs on the fly and the socket() is closed, the struct sock::sk_refcnt reaches 0. In the TX-path the CAN stack clones an "echo" skb, calls sock_hold() on the original socket and references it. This produces the following back trace: | WARNING: CPU: 0 PID: 280 at lib/refcount.c:25 refcount_warn_saturate+0x114/0x134 | refcount_t: addition on 0; use-after-free. | Modules linked in: coda_vpu(E) v4l2_jpeg(E) videobuf2_vmalloc(E) imx_vdoa(E) | CPU: 0 PID: 280 Comm: test_can.sh Tainted: G E 5.11.0-04577-gf8ff6603c617 #203 | Hardware name: Freescale i.MX6 Quad/DualLite (Device Tree) | Backtrace: | [<80bafea4>] (dump_backtrace) from [<80bb0280>] (show_stack+0x20/0x24) r7:00000000 r6:600f0113 r5:00000000 r4:81441220 | [<80bb0260>] (show_stack) from [<80bb593c>] (dump_stack+0xa0/0xc8) | [<80bb589c>] (dump_stack) from [<8012b268>] (__warn+0xd4/0x114) r9:00000019 r8:80f4a8c2 r7:83e4150c r6:00000000 r5:00000009 r4:80528f90 | [<8012b194>] (__warn) from [<80bb09c4>] (warn_slowpath_fmt+0x88/0xc8) r9:83f26400 r8:80f4a8d1 r7:00000009 r6:80528f90 r5:00000019 r4:80f4a8c2 | [<80bb0940>] (warn_slowpath_fmt) from [<80528f90>] (refcount_warn_saturate+0x114/0x134) r8:00000000 r7:00000000 r6:82b44000 r5:834e5600 r4:83f4d540 | [<80528e7c>] (refcount_warn_saturate) from [<8079a4c8>] (__refcount_add.constprop.0+0x4c/0x50) | [<8079a47c>] (__refcount_add.constprop.0) from [<8079a57c>] (can_put_echo_skb+0xb0/0x13c) | [<8079a4cc>] (can_put_echo_skb) from [<8079ba98>] (flexcan_start_xmit+0x1c4/0x230) r9:00000010 r8:83f48610 r7:0fdc0000 r6:0c080000 r5:82b44000 r4:834e5600 | [<8079b8d4>] (flexcan_start_xmit) from [<80969078>] (netdev_start_xmit+0x44/0x70) r9:814c0ba0 r8:80c8790c r7:00000000 r6:834e5600 r5:82b44000 r4:82ab1f00 | [<80969034>] (netdev_start_xmit) from [<809725a4>] (dev_hard_start_xmit+0x19c/0x318) r9:814c0ba0 r8:00000000 r7:82ab1f00 r6:82b44000 r5:00000000 r4:834e5600 | [<80972408>] (dev_hard_start_xmit) from [<809c6584>] (sch_direct_xmit+0xcc/0x264) r10:834e5600 r9:00000000 r8:00000000 r7:82b44000 r6:82ab1f00 r5:834e5600 r4:83f27400 | [<809c64b8>] (sch_direct_xmit) from [<809c6c0c>] (__qdisc_run+0x4f0/0x534) To fix this problem, only set skb ownership to sockets which have still a ref count > 0. Fixes:0ae89beb28("can: add destructor for self generated skbs") Cc: Oliver Hartkopp <socketcan@hartkopp.net> Cc: Andre Naujoks <nautsch2@gmail.com> Link: https://lore.kernel.org/r/20210226092456.27126-1-o.rempel@pengutronix.de Suggested-by: Eric Dumazet <edumazet@google.com> Signed-off-by: Oleksij Rempel <o.rempel@pengutronix.de> Reviewed-by: Oliver Hartkopp <socketcan@hartkopp.net> Signed-off-by: Marc Kleine-Budde <mkl@pengutronix.de> Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
81 lines
2 KiB
C
81 lines
2 KiB
C
/* SPDX-License-Identifier: (GPL-2.0 OR BSD-3-Clause) */
|
|
/*
|
|
* linux/can/skb.h
|
|
*
|
|
* Definitions for the CAN network socket buffer
|
|
*
|
|
* Copyright (C) 2012 Oliver Hartkopp <socketcan@hartkopp.net>
|
|
*
|
|
*/
|
|
|
|
#ifndef _CAN_SKB_H
|
|
#define _CAN_SKB_H
|
|
|
|
#include <linux/types.h>
|
|
#include <linux/skbuff.h>
|
|
#include <linux/can.h>
|
|
#include <net/sock.h>
|
|
|
|
/*
|
|
* The struct can_skb_priv is used to transport additional information along
|
|
* with the stored struct can(fd)_frame that can not be contained in existing
|
|
* struct sk_buff elements.
|
|
* N.B. that this information must not be modified in cloned CAN sk_buffs.
|
|
* To modify the CAN frame content or the struct can_skb_priv content
|
|
* skb_copy() needs to be used instead of skb_clone().
|
|
*/
|
|
|
|
/**
|
|
* struct can_skb_priv - private additional data inside CAN sk_buffs
|
|
* @ifindex: ifindex of the first interface the CAN frame appeared on
|
|
* @skbcnt: atomic counter to have an unique id together with skb pointer
|
|
* @cf: align to the following CAN frame at skb->data
|
|
*/
|
|
struct can_skb_priv {
|
|
int ifindex;
|
|
int skbcnt;
|
|
struct can_frame cf[];
|
|
};
|
|
|
|
static inline struct can_skb_priv *can_skb_prv(struct sk_buff *skb)
|
|
{
|
|
return (struct can_skb_priv *)(skb->head);
|
|
}
|
|
|
|
static inline void can_skb_reserve(struct sk_buff *skb)
|
|
{
|
|
skb_reserve(skb, sizeof(struct can_skb_priv));
|
|
}
|
|
|
|
static inline void can_skb_set_owner(struct sk_buff *skb, struct sock *sk)
|
|
{
|
|
/* If the socket has already been closed by user space, the
|
|
* refcount may already be 0 (and the socket will be freed
|
|
* after the last TX skb has been freed). So only increase
|
|
* socket refcount if the refcount is > 0.
|
|
*/
|
|
if (sk && refcount_inc_not_zero(&sk->sk_refcnt)) {
|
|
skb->destructor = sock_efree;
|
|
skb->sk = sk;
|
|
}
|
|
}
|
|
|
|
/*
|
|
* returns an unshared skb owned by the original sock to be echo'ed back
|
|
*/
|
|
static inline struct sk_buff *can_create_echo_skb(struct sk_buff *skb)
|
|
{
|
|
struct sk_buff *nskb;
|
|
|
|
nskb = skb_clone(skb, GFP_ATOMIC);
|
|
if (unlikely(!nskb)) {
|
|
kfree_skb(skb);
|
|
return NULL;
|
|
}
|
|
|
|
can_skb_set_owner(nskb, skb->sk);
|
|
consume_skb(skb);
|
|
return nskb;
|
|
}
|
|
|
|
#endif /* !_CAN_SKB_H */
|