Module: xenomai-3 Branch: arm64 Commit: 45474093d65bfa43968815520059dd530b4d9b78 URL: http://git.xenomai.org/?p=xenomai-3.git;a=commit;h=45474093d65bfa43968815520059dd530b4d9b78
Author: Gilles Chanteperdrix <gilles.chanteperd...@xenomai.org> Date: Tue Sep 29 23:17:18 2015 +0200 rtnet/socket: rework reference counting With socket pools allocations locking sockets, when a received packet switches from the device pool to the socket pool and is queued in a socket incoming queue, the socket is locked preventing its destruction until all packets have been received. This is probably the reason for this bug report: http://xenomai.org/pipermail/xenomai/2015-June/034345.html Furthermore, when looking at the code in af_packet.c when a packet has been acquired by the socket pool, but is not yet queued, the socket is locked, so, the socket can not disappear and the packet leak, so the locking is redundant. The case of ip_input.c is a bit different as there is a small window where a packet is in flight and a socket can be closed, but this can been fixed. What can happen however, is that a module can be removed in the middle of the packet reception, causing all sorts of trouble. So, this commit: - suppresses the locking of sockets by the socket pool allocation; - gets sockets creation to lock the module in which they are created; - fixes packet delivery in ip_input.c to lock the destination socket as long as the packet has not been delivered. --- kernel/drivers/net/stack/include/rtnet_socket.h | 16 +++++-- kernel/drivers/net/stack/ipv4/icmp.c | 8 ++-- kernel/drivers/net/stack/ipv4/ip_fragment.c | 19 ++++++-- kernel/drivers/net/stack/ipv4/ip_input.c | 10 ++-- kernel/drivers/net/stack/packet/af_packet.c | 4 -- kernel/drivers/net/stack/rtmac/rtmac_vnic.c | 17 +------ kernel/drivers/net/stack/rtskb.c | 17 ++++++- kernel/drivers/net/stack/socket.c | 58 +++++++++++------------ 8 files changed, 82 insertions(+), 67 deletions(-) diff --git a/kernel/drivers/net/stack/include/rtnet_socket.h b/kernel/drivers/net/stack/include/rtnet_socket.h index 363267e..cddf328 100644 --- a/kernel/drivers/net/stack/include/rtnet_socket.h +++ b/kernel/drivers/net/stack/include/rtnet_socket.h @@ -78,6 +78,8 @@ struct rtsocket { int ifindex; } packet; } prot; + + struct module *owner; }; @@ -91,7 +93,11 @@ static inline struct rtdm_fd *rt_socket_fd(struct rtsocket *sock) #define rt_socket_dereference(sock) \ rtdm_fd_unlock(rt_socket_fd(sock)) -int rt_socket_init(struct rtdm_fd *fd, unsigned short protocol); +int __rt_socket_init(struct rtdm_fd *fd, unsigned short protocol, + struct module *module); +#define rt_socket_init(fd, proto) \ + __rt_socket_init(fd, proto, THIS_MODULE) + void rt_socket_cleanup(struct rtdm_fd *fd); int rt_socket_common_ioctl(struct rtdm_fd *fd, int request, void *arg); int rt_socket_if_ioctl(struct rtdm_fd *fd, int request, void *arg); @@ -100,12 +106,16 @@ int rt_socket_select_bind(struct rtdm_fd *fd, enum rtdm_selecttype type, unsigned fd_index); -int rt_bare_socket_init(struct rtdm_fd *fd, unsigned short protocol, - unsigned int priority, unsigned int pool_size); +int __rt_bare_socket_init(struct rtdm_fd *fd, unsigned short protocol, + unsigned int priority, unsigned int pool_size, + struct module *module); +#define rt_bare_socket_init(fd, proto, prio, pool_sz) \ + __rt_bare_socket_init(fd, proto, prio, pool_sz, THIS_MODULE) static inline void rt_bare_socket_cleanup(struct rtsocket *sock) { rtskb_pool_release(&sock->skb_pool); + module_put(sock->owner); } #endif /* __RTNET_SOCKET_H_ */ diff --git a/kernel/drivers/net/stack/ipv4/icmp.c b/kernel/drivers/net/stack/ipv4/icmp.c index b6ae736..58d97cd 100644 --- a/kernel/drivers/net/stack/ipv4/icmp.c +++ b/kernel/drivers/net/stack/ipv4/icmp.c @@ -524,11 +524,11 @@ static struct rtinet_protocol icmp_protocol = { */ void __init rt_icmp_init(void) { - unsigned int skbs; + int skbs; - - skbs = rt_bare_socket_init(icmp_fd, IPPROTO_ICMP, RT_ICMP_PRIO, - ICMP_REPLY_POOL_SIZE); + skbs = __rt_bare_socket_init(icmp_fd, IPPROTO_ICMP, RT_ICMP_PRIO, + ICMP_REPLY_POOL_SIZE, NULL); + BUG_ON(skbs < 0); if (skbs < ICMP_REPLY_POOL_SIZE) printk("RTnet: allocated only %d icmp rtskbs\n", skbs); diff --git a/kernel/drivers/net/stack/ipv4/ip_fragment.c b/kernel/drivers/net/stack/ipv4/ip_fragment.c index 31f3e0a..593aec2 100644 --- a/kernel/drivers/net/stack/ipv4/ip_fragment.c +++ b/kernel/drivers/net/stack/ipv4/ip_fragment.c @@ -112,7 +112,7 @@ static void alloc_collector(struct rtskb *skb, struct rtsocket *sock) * */ static struct rtskb *add_to_collector(struct rtskb *skb, unsigned int offset, int more_frags) { - int i; + int i, err; rtdm_lockctx_t context; struct ip_collector *p_coll; struct iphdr *iph = skb->nh.iph; @@ -174,7 +174,15 @@ static struct rtskb *add_to_collector(struct rtskb *skb, unsigned int offset, in if (!more_frags) { p_coll->in_use = 0; + err = rt_socket_reference(p_coll->sock); + rtdm_lock_put_irqrestore(&p_coll->frags.lock, context); + + if (err < 0) { + kfree_rtskb(first_skb); + return NULL; + } + return first_skb; } else { rtdm_lock_put_irqrestore(&p_coll->frags.lock, context); @@ -297,12 +305,9 @@ struct rtskb *rt_ip_defrag(struct rtskb *skb, struct rtinet_protocol *ipprot) return NULL; } - /* Acquire the rtskb at the expense of the socket's pool */ + /* Acquire the rtskb, to unlock the device skb pool */ ret = rtskb_acquire(skb, &sock->skb_pool); - /* socket is now implicitely locked by the missing rtskb */ - rt_socket_dereference(sock); - if (ret != 0) { /* Drop the rtskb */ kfree_rtskb(skb); @@ -310,6 +315,10 @@ struct rtskb *rt_ip_defrag(struct rtskb *skb, struct rtinet_protocol *ipprot) /* Allocates a new collector */ alloc_collector(skb, sock); } + + /* Packet is queued or freed, socket can be released */ + rt_socket_dereference(sock); + return NULL; } else diff --git a/kernel/drivers/net/stack/ipv4/ip_input.c b/kernel/drivers/net/stack/ipv4/ip_input.c index 9705846..c9c871e 100644 --- a/kernel/drivers/net/stack/ipv4/ip_input.c +++ b/kernel/drivers/net/stack/ipv4/ip_input.c @@ -67,6 +67,8 @@ static inline void rt_ip_local_deliver(struct rtskb *skb) skb = rt_ip_defrag(skb, ipprot); if (!skb) return; + + sock = skb->sk; } else { /* Get the destination socket */ if ((sock = ipprot->dest_socket(skb)) == NULL) { @@ -81,12 +83,9 @@ static inline void rt_ip_local_deliver(struct rtskb *skb) return; } - /* Acquire the rtskb at the expense of the protocol pool */ + /* Acquire the rtskb, to unlock the device skb pool */ err = rtskb_acquire(skb, &sock->skb_pool); - /* Socket is now implicitely locked by the rtskb */ - rt_socket_dereference(sock); - if (err) { kfree_rtskb(skb); return; @@ -95,6 +94,9 @@ static inline void rt_ip_local_deliver(struct rtskb *skb) /* Deliver the packet to the next layer */ ipprot->rcv_handler(skb); + + /* Packet is queued, socket can be released */ + rt_socket_dereference(sock); #if IS_ENABLED(CONFIG_XENO_DRIVERS_NET_ADDON_PROXY) } else if (rt_ip_fallback_handler) { /* If a fallback handler for IP protocol has been installed, diff --git a/kernel/drivers/net/stack/packet/af_packet.c b/kernel/drivers/net/stack/packet/af_packet.c index 4c7ff57..b4a18c2 100644 --- a/kernel/drivers/net/stack/packet/af_packet.c +++ b/kernel/drivers/net/stack/packet/af_packet.c @@ -48,9 +48,6 @@ static int rt_packet_rcv(struct rtskb *skb, struct rtpacket_type *pt) if (unlikely((ifindex != 0) && (ifindex != skb->rtdev->ifindex))) return -EUNATCH; - if (rt_socket_reference(sock) < 0) - return -EUNATCH; - #ifdef CONFIG_XENO_DRIVERS_NET_ETH_P_ALL if (pt->type == htons(ETH_P_ALL)) { struct rtskb *clone_skb = rtskb_clone(skb, &sock->skb_pool); @@ -76,7 +73,6 @@ static int rt_packet_rcv(struct rtskb *skb, struct rtpacket_type *pt) callback_func(rt_socket_fd(sock), callback_arg); out: - rt_socket_dereference(sock); return 0; } diff --git a/kernel/drivers/net/stack/rtmac/rtmac_vnic.c b/kernel/drivers/net/stack/rtmac/rtmac_vnic.c index e17405e..510e714 100644 --- a/kernel/drivers/net/stack/rtmac/rtmac_vnic.c +++ b/kernel/drivers/net/stack/rtmac/rtmac_vnic.c @@ -231,20 +231,6 @@ static void rtmac_vnic_setup(struct net_device *dev) dev->flags &= ~IFF_MULTICAST; } -static int rtmac_vnic_pool_trylock(void *cookie) -{ - return 1; -} - -static void rtmac_vnic_pool_unlock(void *cookie) -{ -} - -static const struct rtskb_pool_lock_ops rtmac_vnic_pool_lock_ops = { - .trylock = rtmac_vnic_pool_trylock, - .unlock = rtmac_vnic_pool_unlock, -}; - int rtmac_vnic_add(struct rtnet_device *rtdev, vnic_xmit_handler vnic_xmit) { int res; @@ -263,8 +249,7 @@ int rtmac_vnic_add(struct rtnet_device *rtdev, vnic_xmit_handler vnic_xmit) /* create the rtskb pool */ if (rtskb_pool_init(&mac_priv->vnic_skb_pool, - vnic_rtskbs, &rtmac_vnic_pool_lock_ops, - NULL) < vnic_rtskbs) { + vnic_rtskbs, NULL, NULL) < vnic_rtskbs) { res = -ENOMEM; goto error; } diff --git a/kernel/drivers/net/stack/rtskb.c b/kernel/drivers/net/stack/rtskb.c index 23d6332..8afd6a6 100644 --- a/kernel/drivers/net/stack/rtskb.c +++ b/kernel/drivers/net/stack/rtskb.c @@ -295,6 +295,21 @@ void kfree_rtskb(struct rtskb *skb) EXPORT_SYMBOL_GPL(kfree_rtskb); +static int rtskb_nop_pool_trylock(void *cookie) +{ + return 1; +} + +static void rtskb_nop_pool_unlock(void *cookie) +{ +} + +static const struct rtskb_pool_lock_ops rtskb_nop_pool_lock_ops = { + .trylock = rtskb_nop_pool_trylock, + .unlock = rtskb_nop_pool_unlock, +}; + + /*** * rtskb_pool_init * @pool: pool to be initialized @@ -316,7 +331,7 @@ unsigned int rtskb_pool_init(struct rtskb_pool *pool, if (rtskb_pools > rtskb_pools_max) rtskb_pools_max = rtskb_pools; - pool->lock_ops = lock_ops; + pool->lock_ops = lock_ops ?: &rtskb_nop_pool_lock_ops; pool->lock_count = 0; pool->lock_cookie = lock_cookie; diff --git a/kernel/drivers/net/stack/socket.c b/kernel/drivers/net/stack/socket.c index a38f560..0c028ab 100644 --- a/kernel/drivers/net/stack/socket.c +++ b/kernel/drivers/net/stack/socket.c @@ -49,38 +49,36 @@ MODULE_PARM_DESC(socket_rtskbs, "Default number of realtime socket buffers in so * internal socket functions * ************************************************************************/ -static int rtskb_socket_pool_trylock(void *cookie) +int __rt_bare_socket_init(struct rtdm_fd *fd, unsigned short protocol, + unsigned int priority, unsigned int pool_size, + struct module *module) { - return rtdm_fd_lock(cookie) >= 0; -} + struct rtsocket *sock = rtdm_fd_to_private(fd); + int err; -static void rtskb_socket_pool_unlock(void *cookie) -{ - rtdm_fd_unlock(cookie); -} + err = try_module_get(module); + if (!err) + return -EAFNOSUPPORT; -static const struct rtskb_pool_lock_ops rtskb_socket_pool_ops = { - .trylock = rtskb_socket_pool_trylock, - .unlock = rtskb_socket_pool_unlock, -}; + err = rtskb_pool_init(&sock->skb_pool, pool_size, NULL, fd); + if (err < 0) { + module_put(module); + return err; + } -int rt_bare_socket_init(struct rtdm_fd *fd, - unsigned short protocol, - unsigned int priority, unsigned int pool_size) -{ - struct rtsocket *sock = rtdm_fd_to_private(fd); sock->protocol = protocol; sock->priority = priority; + sock->owner = module; - return rtskb_pool_init(&sock->skb_pool, - pool_size, &rtskb_socket_pool_ops, fd); + return err; } -EXPORT_SYMBOL_GPL(rt_bare_socket_init); +EXPORT_SYMBOL_GPL(__rt_bare_socket_init); /*** * rt_socket_init - initialises a new socket structure */ -int rt_socket_init(struct rtdm_fd *fd, unsigned short protocol) +int __rt_socket_init(struct rtdm_fd *fd, unsigned short protocol, + struct module *module) { struct rtsocket *sock = rtdm_fd_to_private(fd); unsigned int pool_size; @@ -95,9 +93,10 @@ int rt_socket_init(struct rtdm_fd *fd, unsigned short protocol) rtdm_lock_init(&sock->param_lock); rtdm_sem_init(&sock->pending_sem, 0); - pool_size = rt_bare_socket_init(fd, protocol, - RTSKB_PRIO_VALUE(SOCK_DEF_PRIO, - RTSKB_DEF_RT_CHANNEL), socket_rtskbs); + pool_size = __rt_bare_socket_init(fd, protocol, + RTSKB_PRIO_VALUE(SOCK_DEF_PRIO, + RTSKB_DEF_RT_CHANNEL), + socket_rtskbs, module); sock->pool_size = pool_size; mutex_init(&sock->pool_nrt_lock); @@ -112,7 +111,7 @@ int rt_socket_init(struct rtdm_fd *fd, unsigned short protocol) return 0; } - +EXPORT_SYMBOL_GPL(__rt_socket_init); /*** @@ -133,8 +132,10 @@ void rt_socket_cleanup(struct rtdm_fd *fd) rtskb_pool_release(&sock->skb_pool); mutex_unlock(&sock->pool_nrt_lock); -} + module_put(sock->owner); +} +EXPORT_SYMBOL_GPL(rt_socket_cleanup); /*** @@ -217,6 +218,7 @@ int rt_socket_common_ioctl(struct rtdm_fd *fd, int request, void *arg) return ret; } +EXPORT_SYMBOL_GPL(rt_socket_common_ioctl); @@ -313,6 +315,7 @@ int rt_socket_if_ioctl(struct rtdm_fd *fd, int request, void *arg) rtdev_dereference(rtdev); return ret; } +EXPORT_SYMBOL_GPL(rt_socket_if_ioctl); int rt_socket_select_bind(struct rtdm_fd *fd, @@ -332,9 +335,4 @@ int rt_socket_select_bind(struct rtdm_fd *fd, return -EINVAL; } - EXPORT_SYMBOL_GPL(rt_socket_select_bind); -EXPORT_SYMBOL_GPL(rt_socket_init); -EXPORT_SYMBOL_GPL(rt_socket_cleanup); -EXPORT_SYMBOL_GPL(rt_socket_common_ioctl); -EXPORT_SYMBOL_GPL(rt_socket_if_ioctl); _______________________________________________ Xenomai-git mailing list Xenomai-git@xenomai.org http://xenomai.org/mailman/listinfo/xenomai-git
