| Message ID | 20190628123819.2785504-4-arnd@arndb.de (mailing list archive) |
|---|---|
| State | Not Applicable |
| Headers | show |
| Series | [1/4,v2] structleak: disable STRUCTLEAK_BYREF in combination with KASAN_STACK | expand |
On Fri, Jun 28, 2019 at 8:40 AM Arnd Bergmann <arnd@arndb.de> wrote: > > With the new CONFIG_GCC_PLUGIN_STRUCTLEAK_BYREF_ALL option, the stack > usage in the ipvs debug output grows because each instance of > IP_VS_DBG_BUF() now has its own buffer of 160 bytes that add up > rather than reusing the stack slots: > > net/netfilter/ipvs/ip_vs_core.c: In function 'ip_vs_sched_persist': > net/netfilter/ipvs/ip_vs_core.c:427:1: error: the frame size of 1052 bytes is larger than 1024 bytes [-Werror=frame-larger-than=] > net/netfilter/ipvs/ip_vs_core.c: In function 'ip_vs_new_conn_out': > net/netfilter/ipvs/ip_vs_core.c:1231:1: error: the frame size of 1048 bytes is larger than 1024 bytes [-Werror=frame-larger-than=] > net/netfilter/ipvs/ip_vs_ftp.c: In function 'ip_vs_ftp_out': > net/netfilter/ipvs/ip_vs_ftp.c:397:1: error: the frame size of 1104 bytes is larger than 1024 bytes [-Werror=frame-larger-than=] > net/netfilter/ipvs/ip_vs_ftp.c: In function 'ip_vs_ftp_in': > net/netfilter/ipvs/ip_vs_ftp.c:555:1: error: the frame size of 1200 bytes is larger than 1024 bytes [-Werror=frame-larger-than=] > > Since printk() already has a way to print IPv4/IPv6 addresses using > the %pIS format string, use that instead, since these are sockaddr_in and sockaddr_in6, should that have the 'n' specifier to denote network byteorder? > combined with a macro that > creates a local sockaddr structure on the stack. These will still > add up, but the stack frames are now under 200 bytes. would it make sense to just define a helper function that takes const char * level and msg strings and up to three struct nf_inet_addr* and do the conversion in there? No need for macros and no state on the stack outside error paths at all. > > Signed-off-by: Arnd Bergmann <arnd@arndb.de> > --- > I'm not sure this actually does what I think it does. Someone > needs to verify that we correctly print the addresses here. > I've also only added three files that caused the warning messages > to be reported. There are still a lot of other instances of > IP_VS_DBG_BUF() that could be converted the same way after the > basic idea is confirmed. > --- > include/net/ip_vs.h | 71 +++++++++++++++++++-------------- > net/netfilter/ipvs/ip_vs_core.c | 44 ++++++++++---------- > net/netfilter/ipvs/ip_vs_ftp.c | 20 +++++----- > 3 files changed, 72 insertions(+), 63 deletions(-) > > diff --git a/include/net/ip_vs.h b/include/net/ip_vs.h > index 3759167f91f5..3dfbeef67be6 100644 > --- a/include/net/ip_vs.h > +++ b/include/net/ip_vs.h > @@ -227,6 +227,16 @@ static inline const char *ip_vs_dbg_addr(int af, char *buf, size_t buf_len, > sizeof(ip_vs_dbg_buf), addr, \ > &ip_vs_dbg_idx) > > +#define IP_VS_DBG_SOCKADDR4(fam, addr, port) \ > + (struct sockaddr*)&(struct sockaddr_in) \ > + { .sin_family = (fam), .sin_addr = (addr)->in, .sin_port = (port) } might as well set .sin_family = AF_INET here and AF_INET6 below? > +#define IP_VS_DBG_SOCKADDR6(fam, addr, port) \ > + (struct sockaddr*)&(struct sockaddr_in6) \ > + { .sin6_family = (fam), .sin6_addr = (addr)->in6, .sin6_port = (port) }
Hello, On Fri, 28 Jun 2019, Arnd Bergmann wrote: > With the new CONFIG_GCC_PLUGIN_STRUCTLEAK_BYREF_ALL option, the stack > usage in the ipvs debug output grows because each instance of > IP_VS_DBG_BUF() now has its own buffer of 160 bytes that add up > rather than reusing the stack slots: > > net/netfilter/ipvs/ip_vs_core.c: In function 'ip_vs_sched_persist': > net/netfilter/ipvs/ip_vs_core.c:427:1: error: the frame size of 1052 bytes is larger than 1024 bytes [-Werror=frame-larger-than=] > net/netfilter/ipvs/ip_vs_core.c: In function 'ip_vs_new_conn_out': > net/netfilter/ipvs/ip_vs_core.c:1231:1: error: the frame size of 1048 bytes is larger than 1024 bytes [-Werror=frame-larger-than=] > net/netfilter/ipvs/ip_vs_ftp.c: In function 'ip_vs_ftp_out': > net/netfilter/ipvs/ip_vs_ftp.c:397:1: error: the frame size of 1104 bytes is larger than 1024 bytes [-Werror=frame-larger-than=] > net/netfilter/ipvs/ip_vs_ftp.c: In function 'ip_vs_ftp_in': > net/netfilter/ipvs/ip_vs_ftp.c:555:1: error: the frame size of 1200 bytes is larger than 1024 bytes [-Werror=frame-larger-than=] > > Since printk() already has a way to print IPv4/IPv6 addresses using > the %pIS format string, use that instead, combined with a macro that > creates a local sockaddr structure on the stack. These will still > add up, but the stack frames are now under 200 bytes. > > Signed-off-by: Arnd Bergmann <arnd@arndb.de> > --- > I'm not sure this actually does what I think it does. Someone > needs to verify that we correctly print the addresses here. > I've also only added three files that caused the warning messages > to be reported. There are still a lot of other instances of > IP_VS_DBG_BUF() that could be converted the same way after the > basic idea is confirmed. > --- > include/net/ip_vs.h | 71 +++++++++++++++++++-------------- > net/netfilter/ipvs/ip_vs_core.c | 44 ++++++++++---------- > net/netfilter/ipvs/ip_vs_ftp.c | 20 +++++----- > 3 files changed, 72 insertions(+), 63 deletions(-) > > diff --git a/include/net/ip_vs.h b/include/net/ip_vs.h > index 3759167f91f5..3dfbeef67be6 100644 > --- a/include/net/ip_vs.h > +++ b/include/net/ip_vs.h > @@ -227,6 +227,16 @@ static inline const char *ip_vs_dbg_addr(int af, char *buf, size_t buf_len, > sizeof(ip_vs_dbg_buf), addr, \ > &ip_vs_dbg_idx) > > +#define IP_VS_DBG_SOCKADDR4(fam, addr, port) \ > + (struct sockaddr*)&(struct sockaddr_in) \ > + { .sin_family = (fam), .sin_addr = (addr)->in, .sin_port = (port) } > +#define IP_VS_DBG_SOCKADDR6(fam, addr, port) \ > + (struct sockaddr*)&(struct sockaddr_in6) \ > + { .sin6_family = (fam), .sin6_addr = (addr)->in6, .sin6_port = (port) } > +#define IP_VS_DBG_SOCKADDR(fam, addr, port) (fam == AF_INET ? \ > + IP_VS_DBG_SOCKADDR4(fam, addr, port) : \ > + IP_VS_DBG_SOCKADDR6(fam, addr, port)) > + > #define IP_VS_DBG(level, msg, ...) \ > do { \ > if (level <= ip_vs_get_debug_level()) \ > @@ -251,6 +261,7 @@ static inline const char *ip_vs_dbg_addr(int af, char *buf, size_t buf_len, > #else /* NO DEBUGGING at ALL */ > #define IP_VS_DBG_BUF(level, msg...) do {} while (0) > #define IP_VS_ERR_BUF(msg...) do {} while (0) > +#define IP_VS_DBG_SOCKADDR(fam, addr, port) NULL > #define IP_VS_DBG(level, msg...) do {} while (0) > #define IP_VS_DBG_RL(msg...) do {} while (0) > #define IP_VS_DBG_PKT(level, af, pp, skb, ofs, msg) do {} while (0) > @@ -1244,31 +1255,31 @@ static inline void ip_vs_control_del(struct ip_vs_conn *cp) > { > struct ip_vs_conn *ctl_cp = cp->control; > if (!ctl_cp) { > - IP_VS_ERR_BUF("request control DEL for uncontrolled: " > - "%s:%d to %s:%d\n", > - IP_VS_DBG_ADDR(cp->af, &cp->caddr), > - ntohs(cp->cport), > - IP_VS_DBG_ADDR(cp->af, &cp->vaddr), > - ntohs(cp->vport)); > + pr_err("request control DEL for uncontrolled: " > + "%pISp to %pISp\n", ip_vs_dbg_addr() used compact form (%pI6c), so it would be better to use %pISc and %pISpc everywhere in IPVS... Also, note that before now port was printed with %d and ntohs() was used, now port should be in network order, so: - ntohs() should be removed - htons() should be added, if missing. At first look, this case is not present in IPVS, we have only ntohs() usage Regards -- Julian Anastasov <ja@ssi.bg>
On Sun, Jun 30, 2019 at 10:37 PM Julian Anastasov <ja@ssi.bg> wrote: > On Fri, 28 Jun 2019, Arnd Bergmann wrote: > > struct ip_vs_conn *ctl_cp = cp->control; > > if (!ctl_cp) { > > - IP_VS_ERR_BUF("request control DEL for uncontrolled: " > > - "%s:%d to %s:%d\n", > > - IP_VS_DBG_ADDR(cp->af, &cp->caddr), > > - ntohs(cp->cport), > > - IP_VS_DBG_ADDR(cp->af, &cp->vaddr), > > - ntohs(cp->vport)); > > + pr_err("request control DEL for uncontrolled: " > > + "%pISp to %pISp\n", (replying a bit late) > ip_vs_dbg_addr() used compact form (%pI6c), so it would be > better to use %pISc and %pISpc everywhere in IPVS... done > Also, note that before now port was printed with %d and > ntohs() was used, now port should be in network order, so: > > - ntohs() should be removed done > - htons() should be added, if missing. At first look, this case > is not present in IPVS, we have only ntohs() usage I found one case in ip_vs_ftp_in() that needs it in order to subtract one: IP_VS_DBG(7, "protocol %s %pISpc %pISpc\n", ip_vs_proto_name(ipvsh->protocol), - IP_VS_DBG_SOCKADDR(cp->af, &to, ntohs(port)), + IP_VS_DBG_SOCKADDR(cp->af, &to, port), IP_VS_DBG_SOCKADDR(cp->af, &cp->vaddr, - ntohs(cp->vport)-1)); + htons(ntohs(cp->vport)-1))); Thanks for the review, I'll send a new patch after some more build testing on the new version. Arnd
On Fri, Jun 28, 2019 at 9:59 PM Willem de Bruijn <willemdebruijn.kernel@gmail.com> wrote: > On Fri, Jun 28, 2019 at 8:40 AM Arnd Bergmann <arnd@arndb.de> wrote: > > > > With the new CONFIG_GCC_PLUGIN_STRUCTLEAK_BYREF_ALL option, the stack > > usage in the ipvs debug output grows because each instance of > > IP_VS_DBG_BUF() now has its own buffer of 160 bytes that add up > > rather than reusing the stack slots: > > > > net/netfilter/ipvs/ip_vs_core.c: In function 'ip_vs_sched_persist': > > net/netfilter/ipvs/ip_vs_core.c:427:1: error: the frame size of 1052 bytes is larger than 1024 bytes [-Werror=frame-larger-than=] > > net/netfilter/ipvs/ip_vs_core.c: In function 'ip_vs_new_conn_out': > > net/netfilter/ipvs/ip_vs_core.c:1231:1: error: the frame size of 1048 bytes is larger than 1024 bytes [-Werror=frame-larger-than=] > > net/netfilter/ipvs/ip_vs_ftp.c: In function 'ip_vs_ftp_out': > > net/netfilter/ipvs/ip_vs_ftp.c:397:1: error: the frame size of 1104 bytes is larger than 1024 bytes [-Werror=frame-larger-than=] > > net/netfilter/ipvs/ip_vs_ftp.c: In function 'ip_vs_ftp_in': > > net/netfilter/ipvs/ip_vs_ftp.c:555:1: error: the frame size of 1200 bytes is larger than 1024 bytes [-Werror=frame-larger-than=] > > > > Since printk() already has a way to print IPv4/IPv6 addresses using > > the %pIS format string, use that instead, > > since these are sockaddr_in and sockaddr_in6, should that have the 'n' > specifier to denote network byteorder? I double-checked the implementation, and don't see that make any difference, as 'n' is the default. > > include/net/ip_vs.h | 71 +++++++++++++++++++-------------- > > net/netfilter/ipvs/ip_vs_core.c | 44 ++++++++++---------- > > net/netfilter/ipvs/ip_vs_ftp.c | 20 +++++----- > > 3 files changed, 72 insertions(+), 63 deletions(-) > > > > diff --git a/include/net/ip_vs.h b/include/net/ip_vs.h > > index 3759167f91f5..3dfbeef67be6 100644 > > --- a/include/net/ip_vs.h > > +++ b/include/net/ip_vs.h > > @@ -227,6 +227,16 @@ static inline const char *ip_vs_dbg_addr(int af, char *buf, size_t buf_len, > > sizeof(ip_vs_dbg_buf), addr, \ > > &ip_vs_dbg_idx) > > > > +#define IP_VS_DBG_SOCKADDR4(fam, addr, port) \ > > + (struct sockaddr*)&(struct sockaddr_in) \ > > + { .sin_family = (fam), .sin_addr = (addr)->in, .sin_port = (port) } > > might as well set .sin_family = AF_INET here and AF_INET6 below? That would work just same, but I don't see any advantage. As the family and port members are the same between sockaddr_in and sockaddr_in6, the compiler can decide to set these regardless to the argument values regardless of the condition. Arnd
diff --git a/include/net/ip_vs.h b/include/net/ip_vs.h index 3759167f91f5..3dfbeef67be6 100644 --- a/include/net/ip_vs.h +++ b/include/net/ip_vs.h @@ -227,6 +227,16 @@ static inline const char *ip_vs_dbg_addr(int af, char *buf, size_t buf_len, sizeof(ip_vs_dbg_buf), addr, \ &ip_vs_dbg_idx) +#define IP_VS_DBG_SOCKADDR4(fam, addr, port) \ + (struct sockaddr*)&(struct sockaddr_in) \ + { .sin_family = (fam), .sin_addr = (addr)->in, .sin_port = (port) } +#define IP_VS_DBG_SOCKADDR6(fam, addr, port) \ + (struct sockaddr*)&(struct sockaddr_in6) \ + { .sin6_family = (fam), .sin6_addr = (addr)->in6, .sin6_port = (port) } +#define IP_VS_DBG_SOCKADDR(fam, addr, port) (fam == AF_INET ? \ + IP_VS_DBG_SOCKADDR4(fam, addr, port) : \ + IP_VS_DBG_SOCKADDR6(fam, addr, port)) + #define IP_VS_DBG(level, msg, ...) \ do { \ if (level <= ip_vs_get_debug_level()) \ @@ -251,6 +261,7 @@ static inline const char *ip_vs_dbg_addr(int af, char *buf, size_t buf_len, #else /* NO DEBUGGING at ALL */ #define IP_VS_DBG_BUF(level, msg...) do {} while (0) #define IP_VS_ERR_BUF(msg...) do {} while (0) +#define IP_VS_DBG_SOCKADDR(fam, addr, port) NULL #define IP_VS_DBG(level, msg...) do {} while (0) #define IP_VS_DBG_RL(msg...) do {} while (0) #define IP_VS_DBG_PKT(level, af, pp, skb, ofs, msg) do {} while (0) @@ -1244,31 +1255,31 @@ static inline void ip_vs_control_del(struct ip_vs_conn *cp) { struct ip_vs_conn *ctl_cp = cp->control; if (!ctl_cp) { - IP_VS_ERR_BUF("request control DEL for uncontrolled: " - "%s:%d to %s:%d\n", - IP_VS_DBG_ADDR(cp->af, &cp->caddr), - ntohs(cp->cport), - IP_VS_DBG_ADDR(cp->af, &cp->vaddr), - ntohs(cp->vport)); + pr_err("request control DEL for uncontrolled: " + "%pISp to %pISp\n", + IP_VS_DBG_SOCKADDR(cp->af, &cp->caddr, + ntohs(cp->cport)), + IP_VS_DBG_SOCKADDR(cp->af, &cp->vaddr, + ntohs(cp->vport))); return; } - IP_VS_DBG_BUF(7, "DELeting control for: " - "cp.dst=%s:%d ctl_cp.dst=%s:%d\n", - IP_VS_DBG_ADDR(cp->af, &cp->caddr), - ntohs(cp->cport), - IP_VS_DBG_ADDR(cp->af, &ctl_cp->caddr), - ntohs(ctl_cp->cport)); + IP_VS_DBG(7, "DELeting control for: " + "cp.dst=%pISp ctl_cp.dst=%pISp\n", + IP_VS_DBG_SOCKADDR(cp->af, &cp->caddr, + ntohs(cp->cport)), + IP_VS_DBG_SOCKADDR(cp->af, &ctl_cp->caddr, + ntohs(ctl_cp->cport))); cp->control = NULL; if (atomic_read(&ctl_cp->n_control) == 0) { - IP_VS_ERR_BUF("BUG control DEL with n=0 : " - "%s:%d to %s:%d\n", - IP_VS_DBG_ADDR(cp->af, &cp->caddr), - ntohs(cp->cport), - IP_VS_DBG_ADDR(cp->af, &cp->vaddr), - ntohs(cp->vport)); + pr_err("BUG control DEL with n=0 : " + "%pISp to %pISp\n", + IP_VS_DBG_SOCKADDR(cp->af, &cp->caddr, + ntohs(cp->cport)), + IP_VS_DBG_SOCKADDR(cp->af, &cp->vaddr, + ntohs(cp->vport))); return; } @@ -1279,22 +1290,22 @@ static inline void ip_vs_control_add(struct ip_vs_conn *cp, struct ip_vs_conn *ctl_cp) { if (cp->control) { - IP_VS_ERR_BUF("request control ADD for already controlled: " - "%s:%d to %s:%d\n", - IP_VS_DBG_ADDR(cp->af, &cp->caddr), - ntohs(cp->cport), - IP_VS_DBG_ADDR(cp->af, &cp->vaddr), - ntohs(cp->vport)); + pr_err("request control ADD for already controlled: " + "%pISp to %pISp\n", + IP_VS_DBG_SOCKADDR(cp->af, &cp->caddr, + ntohs(cp->cport)), + IP_VS_DBG_SOCKADDR(cp->af, &cp->vaddr, + ntohs(cp->vport))); ip_vs_control_del(cp); } - IP_VS_DBG_BUF(7, "ADDing control for: " - "cp.dst=%s:%d ctl_cp.dst=%s:%d\n", - IP_VS_DBG_ADDR(cp->af, &cp->caddr), - ntohs(cp->cport), - IP_VS_DBG_ADDR(cp->af, &ctl_cp->caddr), - ntohs(ctl_cp->cport)); + IP_VS_DBG(7, "ADDing control for: " + "cp.dst=%pISp ctl_cp.dst=%pISp\n", + IP_VS_DBG_SOCKADDR(cp->af, &cp->caddr, + ntohs(cp->cport)), + IP_VS_DBG_SOCKADDR(cp->af, &ctl_cp->caddr, + ntohs(ctl_cp->cport))); cp->control = ctl_cp; atomic_inc(&ctl_cp->n_control); diff --git a/net/netfilter/ipvs/ip_vs_core.c b/net/netfilter/ipvs/ip_vs_core.c index f662f198b458..0277cd3c5446 100644 --- a/net/netfilter/ipvs/ip_vs_core.c +++ b/net/netfilter/ipvs/ip_vs_core.c @@ -51,7 +51,6 @@ #include <net/ip_vs.h> #include <linux/indirect_call_wrapper.h> - EXPORT_SYMBOL(register_ip_vs_scheduler); EXPORT_SYMBOL(unregister_ip_vs_scheduler); EXPORT_SYMBOL(ip_vs_proto_name); @@ -294,11 +293,11 @@ ip_vs_sched_persist(struct ip_vs_service *svc, #endif snet.ip = src_addr->ip & svc->netmask; - IP_VS_DBG_BUF(6, "p-schedule: src %s:%u dest %s:%u " - "mnet %s\n", - IP_VS_DBG_ADDR(svc->af, src_addr), ntohs(src_port), - IP_VS_DBG_ADDR(svc->af, dst_addr), ntohs(dst_port), - IP_VS_DBG_ADDR(svc->af, &snet)); + IP_VS_DBG(6, "p-schedule: src %pISp dest %pISp " + "mnet %pIS\n", + IP_VS_DBG_SOCKADDR(svc->af, src_addr, ntohs(src_port)), + IP_VS_DBG_SOCKADDR(svc->af, dst_addr, ntohs(dst_port)), + IP_VS_DBG_SOCKADDR(svc->af, &snet, 0)); /* * As far as we know, FTP is a very complicated network protocol, and @@ -566,12 +565,12 @@ ip_vs_schedule(struct ip_vs_service *svc, struct sk_buff *skb, } } - IP_VS_DBG_BUF(6, "Schedule fwd:%c c:%s:%u v:%s:%u " - "d:%s:%u conn->flags:%X conn->refcnt:%d\n", + IP_VS_DBG(6, "Schedule fwd:%c c:%pISp v:%pISp " + "d:%pISp conn->flags:%X conn->refcnt:%d\n", ip_vs_fwd_tag(cp), - IP_VS_DBG_ADDR(cp->af, &cp->caddr), ntohs(cp->cport), - IP_VS_DBG_ADDR(cp->af, &cp->vaddr), ntohs(cp->vport), - IP_VS_DBG_ADDR(cp->daf, &cp->daddr), ntohs(cp->dport), + IP_VS_DBG_SOCKADDR(cp->af, &cp->caddr, ntohs(cp->cport)), + IP_VS_DBG_SOCKADDR(cp->af, &cp->vaddr, ntohs(cp->vport)), + IP_VS_DBG_SOCKADDR(cp->daf, &cp->daddr, ntohs(cp->dport)), cp->flags, refcount_read(&cp->refcnt)); ip_vs_conn_stats(cp, svc); @@ -885,8 +884,8 @@ static int handle_response_icmp(int af, struct sk_buff *skb, /* Ensure the checksum is correct */ if (!skb_csum_unnecessary(skb) && ip_vs_checksum_complete(skb, ihl)) { /* Failed checksum! */ - IP_VS_DBG_BUF(1, "Forward ICMP: failed checksum from %s!\n", - IP_VS_DBG_ADDR(af, snet)); + IP_VS_DBG(1, "Forward ICMP: failed checksum from %pISp!\n", + IP_VS_DBG_SOCKADDR(af, snet, 0)); goto out; } @@ -1219,13 +1218,13 @@ struct ip_vs_conn *ip_vs_new_conn_out(struct ip_vs_service *svc, ip_vs_conn_stats(cp, svc); /* return connection (will be used to handle outgoing packet) */ - IP_VS_DBG_BUF(6, "New connection RS-initiated:%c c:%s:%u v:%s:%u " - "d:%s:%u conn->flags:%X conn->refcnt:%d\n", - ip_vs_fwd_tag(cp), - IP_VS_DBG_ADDR(cp->af, &cp->caddr), ntohs(cp->cport), - IP_VS_DBG_ADDR(cp->af, &cp->vaddr), ntohs(cp->vport), - IP_VS_DBG_ADDR(cp->af, &cp->daddr), ntohs(cp->dport), - cp->flags, refcount_read(&cp->refcnt)); + IP_VS_DBG(6, "New connection RS-initiated:%c c:%pISp v:%pISp " + "d:%pISp conn->flags:%X conn->refcnt:%d\n", + ip_vs_fwd_tag(cp), + IP_VS_DBG_SOCKADDR(cp->af, &cp->caddr, ntohs(cp->cport)), + IP_VS_DBG_SOCKADDR(cp->af, &cp->vaddr, ntohs(cp->vport)), + IP_VS_DBG_SOCKADDR(cp->af, &cp->daddr, ntohs(cp->dport)), + cp->flags, refcount_read(&cp->refcnt)); LeaveFunction(12); return cp; } @@ -1931,7 +1930,6 @@ static int ip_vs_in_icmp_v6(struct netns_ipvs *ipvs, struct sk_buff *skb, } #endif - /* * Check if it's for virtual services, look it up, * and send it on its way... @@ -1960,10 +1958,10 @@ ip_vs_in(struct netns_ipvs *ipvs, unsigned int hooknum, struct sk_buff *skb, int hooknum != NF_INET_LOCAL_OUT) || !skb_dst(skb))) { ip_vs_fill_iph_skb(af, skb, false, &iph); - IP_VS_DBG_BUF(12, "packet type=%d proto=%d daddr=%s" + IP_VS_DBG(12, "packet type=%d proto=%d daddr=%pIS" " ignored in hook %u\n", skb->pkt_type, iph.protocol, - IP_VS_DBG_ADDR(af, &iph.daddr), hooknum); + IP_VS_DBG_SOCKADDR(af, &iph.daddr, 0), hooknum); return NF_ACCEPT; } /* ipvs enabled in this netns ? */ diff --git a/net/netfilter/ipvs/ip_vs_ftp.c b/net/netfilter/ipvs/ip_vs_ftp.c index cf925906f59b..d57dcc2b4396 100644 --- a/net/netfilter/ipvs/ip_vs_ftp.c +++ b/net/netfilter/ipvs/ip_vs_ftp.c @@ -306,9 +306,9 @@ static int ip_vs_ftp_out(struct ip_vs_app *app, struct ip_vs_conn *cp, &start, &end) != 1) return 1; - IP_VS_DBG_BUF(7, "EPSV response (%s:%u) -> %s:%u detected\n", - IP_VS_DBG_ADDR(cp->af, &from), ntohs(port), - IP_VS_DBG_ADDR(cp->af, &cp->caddr), 0); + IP_VS_DBG(7, "EPSV response (%pISp) -> %pISp detected\n", + IP_VS_DBG_SOCKADDR(cp->af, &from, ntohs(port)), + IP_VS_DBG_SOCKADDR(cp->af, &cp->caddr, 0)); } else { return 1; } @@ -510,15 +510,15 @@ static int ip_vs_ftp_in(struct ip_vs_app *app, struct ip_vs_conn *cp, &to, &port, cp->af, &start, &end) == 1) { - IP_VS_DBG_BUF(7, "EPRT %s:%u detected\n", - IP_VS_DBG_ADDR(cp->af, &to), ntohs(port)); + IP_VS_DBG(7, "EPRT %pISp detected\n", + IP_VS_DBG_SOCKADDR(cp->af, &to, ntohs(port))); /* Now update or create a connection entry for it */ - IP_VS_DBG_BUF(7, "protocol %s %s:%u %s:%u\n", - ip_vs_proto_name(ipvsh->protocol), - IP_VS_DBG_ADDR(cp->af, &to), ntohs(port), - IP_VS_DBG_ADDR(cp->af, &cp->vaddr), - ntohs(cp->vport)-1); + IP_VS_DBG(7, "protocol %s %pISp %pISp\n", + ip_vs_proto_name(ipvsh->protocol), + IP_VS_DBG_SOCKADDR(cp->af, &to, ntohs(port)), + IP_VS_DBG_SOCKADDR(cp->af, &cp->vaddr, + ntohs(cp->vport)-1)); } else { return 1; }
With the new CONFIG_GCC_PLUGIN_STRUCTLEAK_BYREF_ALL option, the stack usage in the ipvs debug output grows because each instance of IP_VS_DBG_BUF() now has its own buffer of 160 bytes that add up rather than reusing the stack slots: net/netfilter/ipvs/ip_vs_core.c: In function 'ip_vs_sched_persist': net/netfilter/ipvs/ip_vs_core.c:427:1: error: the frame size of 1052 bytes is larger than 1024 bytes [-Werror=frame-larger-than=] net/netfilter/ipvs/ip_vs_core.c: In function 'ip_vs_new_conn_out': net/netfilter/ipvs/ip_vs_core.c:1231:1: error: the frame size of 1048 bytes is larger than 1024 bytes [-Werror=frame-larger-than=] net/netfilter/ipvs/ip_vs_ftp.c: In function 'ip_vs_ftp_out': net/netfilter/ipvs/ip_vs_ftp.c:397:1: error: the frame size of 1104 bytes is larger than 1024 bytes [-Werror=frame-larger-than=] net/netfilter/ipvs/ip_vs_ftp.c: In function 'ip_vs_ftp_in': net/netfilter/ipvs/ip_vs_ftp.c:555:1: error: the frame size of 1200 bytes is larger than 1024 bytes [-Werror=frame-larger-than=] Since printk() already has a way to print IPv4/IPv6 addresses using the %pIS format string, use that instead, combined with a macro that creates a local sockaddr structure on the stack. These will still add up, but the stack frames are now under 200 bytes. Signed-off-by: Arnd Bergmann <arnd@arndb.de> --- I'm not sure this actually does what I think it does. Someone needs to verify that we correctly print the addresses here. I've also only added three files that caused the warning messages to be reported. There are still a lot of other instances of IP_VS_DBG_BUF() that could be converted the same way after the basic idea is confirmed. --- include/net/ip_vs.h | 71 +++++++++++++++++++-------------- net/netfilter/ipvs/ip_vs_core.c | 44 ++++++++++---------- net/netfilter/ipvs/ip_vs_ftp.c | 20 +++++----- 3 files changed, 72 insertions(+), 63 deletions(-)