| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <1370064152.24311.19.camel@edumazet-glaptop>
Date: Fri, 31 May 2013 22:22:32 -0700
From: Eric Dumazet <eric.dumazet@...il.com>
To: Cong Wang <amwang@...hat.com>
Cc: netdev@...r.kernel.org, "David S. Miller" <davem@...emloft.net>
Subject: Re: [Patch net-next] icmp: avoid allocating large struct on stack
On Sat, 2013-06-01 at 11:24 +0800, Cong Wang wrote:
> From: Cong Wang <amwang@...hat.com>
>
> struct icmp_bxm is a large struct, reduce stack usage
> by allocating it on heap.
>
Strange, I posted a patch like that some days ago.
> Cc: David S. Miller <davem@...emloft.net>
> Signed-off-by: Cong Wang <amwang@...hat.com>
>
> ---
> net/ipv4/icmp.c | 37 +++++++++++++++++++++----------------
> 1 files changed, 21 insertions(+), 16 deletions(-)
>
> diff --git a/net/ipv4/icmp.c b/net/ipv4/icmp.c
> index 5d0d379..8157684 100644
> --- a/net/ipv4/icmp.c
> +++ b/net/ipv4/icmp.c
> @@ -482,7 +482,7 @@ void icmp_send(struct sk_buff *skb_in, int type, int code, __be32 info)
> {
> struct iphdr *iph;
> int room;
> - struct icmp_bxm icmp_param;
> + struct icmp_bxm *icmp_param;
> struct rtable *rt = skb_rtable(skb_in);
> struct ipcm_cookie ipc;
> struct flowi4 fl4;
> @@ -558,6 +558,10 @@ void icmp_send(struct sk_buff *skb_in, int type, int code, __be32 info)
> }
> }
>
> + icmp_param = kzalloc(sizeof(*icmp_param), GFP_ATOMIC);
> + if (!icmp_param)
> + return;
> +
> sk = icmp_xmit_lock(net);
> if (sk == NULL)
> return;
Please.
Read carefully this code and tell me if you do not leak memory.
For the record my patch was saving more stack space :
net/ipv4/icmp.c | 72
+++++++++++++++++++++++++++++---------------------------
1 file changed, 38 insertions(+), 34 deletions(-)
diff --git a/net/ipv4/icmp.c b/net/ipv4/icmp.c
index 76e10b4..e33f3b0 100644
--- a/net/ipv4/icmp.c
+++ b/net/ipv4/icmp.c
@@ -208,7 +208,7 @@ static struct sock *icmp_sk(struct net *net)
return net->ipv4.icmp_sk[smp_processor_id()];
}
-static inline struct sock *icmp_xmit_lock(struct net *net)
+static struct sock *icmp_xmit_lock(struct net *net)
{
struct sock *sk;
@@ -226,7 +226,7 @@ static inline struct sock *icmp_xmit_lock(struct net *net)
return sk;
}
-static inline void icmp_xmit_unlock(struct sock *sk)
+static void icmp_xmit_unlock(struct sock *sk)
{
spin_unlock_bh(&sk->sk_lock.slock);
}
@@ -235,8 +235,8 @@ static inline void icmp_xmit_unlock(struct sock *sk)
* Send an ICMP frame.
*/
-static inline bool icmpv4_xrlim_allow(struct net *net, struct rtable *rt,
- struct flowi4 *fl4, int type, int code)
+static bool icmpv4_xrlim_allow(struct net *net, struct rtable *rt,
+ struct flowi4 *fl4, int type, int code)
{
struct dst_entry *dst = &rt->dst;
bool rc = true;
@@ -375,19 +375,22 @@ out_unlock:
icmp_xmit_unlock(sk);
}
-static struct rtable *icmp_route_lookup(struct net *net,
- struct flowi4 *fl4,
- struct sk_buff *skb_in,
- const struct iphdr *iph,
- __be32 saddr, u8 tos,
- int type, int code,
- struct icmp_bxm *param)
+struct icmp_send_data {
+ struct icmp_bxm icmp_param;
+ struct ipcm_cookie ipc;
+ struct flowi4 fl4;
+};
+
+static noinline_for_stack struct rtable *
+icmp_route_lookup(struct net *net, struct flowi4 *fl4,
+ struct sk_buff *skb_in, const struct iphdr *iph,
+ __be32 saddr, u8 tos, int type, int code,
+ struct icmp_bxm *param)
{
struct rtable *rt, *rt2;
struct flowi4 fl4_dec;
int err;
- memset(fl4, 0, sizeof(*fl4));
fl4->daddr = (param->replyopts.opt.opt.srr ?
param->replyopts.opt.opt.faddr : iph->saddr);
fl4->saddr = saddr;
@@ -482,14 +485,12 @@ void icmp_send(struct sk_buff *skb_in, int type, int code, __be32 info)
{
struct iphdr *iph;
int room;
- struct icmp_bxm icmp_param;
struct rtable *rt = skb_rtable(skb_in);
- struct ipcm_cookie ipc;
- struct flowi4 fl4;
__be32 saddr;
u8 tos;
struct net *net;
struct sock *sk;
+ struct icmp_send_data *data = NULL;
if (!rt)
goto out;
@@ -585,7 +586,11 @@ void icmp_send(struct sk_buff *skb_in, int type, int code, __be32 info)
IPTOS_PREC_INTERNETCONTROL) :
iph->tos;
- if (ip_options_echo(&icmp_param.replyopts.opt.opt, skb_in))
+ data = kzalloc(sizeof(*data), GFP_ATOMIC);
+ if (!data)
+ goto out_unlock;
+
+ if (ip_options_echo(&data->icmp_param.replyopts.opt.opt, skb_in))
goto out_unlock;
@@ -593,23 +598,21 @@ void icmp_send(struct sk_buff *skb_in, int type, int code, __be32 info)
* Prepare data for ICMP header.
*/
- icmp_param.data.icmph.type = type;
- icmp_param.data.icmph.code = code;
- icmp_param.data.icmph.un.gateway = info;
- icmp_param.data.icmph.checksum = 0;
- icmp_param.skb = skb_in;
- icmp_param.offset = skb_network_offset(skb_in);
+ data->icmp_param.data.icmph.type = type;
+ data->icmp_param.data.icmph.code = code;
+ data->icmp_param.data.icmph.un.gateway = info;
+ data->icmp_param.skb = skb_in;
+ data->icmp_param.offset = skb_network_offset(skb_in);
inet_sk(sk)->tos = tos;
- ipc.addr = iph->saddr;
- ipc.opt = &icmp_param.replyopts.opt;
- ipc.tx_flags = 0;
+ data->ipc.addr = iph->saddr;
+ data->ipc.opt = &data->icmp_param.replyopts.opt;
- rt = icmp_route_lookup(net, &fl4, skb_in, iph, saddr, tos,
- type, code, &icmp_param);
+ rt = icmp_route_lookup(net, &data->fl4, skb_in, iph, saddr, tos,
+ type, code, &data->icmp_param);
if (IS_ERR(rt))
goto out_unlock;
- if (!icmpv4_xrlim_allow(net, rt, &fl4, type, code))
+ if (!icmpv4_xrlim_allow(net, rt, &data->fl4, type, code))
goto ende;
/* RFC says return as much as we can without exceeding 576 bytes. */
@@ -617,19 +620,20 @@ void icmp_send(struct sk_buff *skb_in, int type, int code, __be32 info)
room = dst_mtu(&rt->dst);
if (room > 576)
room = 576;
- room -= sizeof(struct iphdr) + icmp_param.replyopts.opt.opt.optlen;
+ room -= sizeof(struct iphdr) + data->icmp_param.replyopts.opt.opt.optlen;
room -= sizeof(struct icmphdr);
- icmp_param.data_len = skb_in->len - icmp_param.offset;
- if (icmp_param.data_len > room)
- icmp_param.data_len = room;
- icmp_param.head_len = sizeof(struct icmphdr);
+ data->icmp_param.data_len = skb_in->len - data->icmp_param.offset;
+ if (data->icmp_param.data_len > room)
+ data->icmp_param.data_len = room;
+ data->icmp_param.head_len = sizeof(struct icmphdr);
- icmp_push_reply(&icmp_param, &fl4, &ipc, &rt);
+ icmp_push_reply(&data->icmp_param, &data->fl4, &data->ipc, &rt);
ende:
ip_rt_put(rt);
out_unlock:
icmp_xmit_unlock(sk);
+ kfree(data);
out:;
}
EXPORT_SYMBOL(icmp_send);
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Powered by blists - more mailing lists