Google Groups no longer supports new Usenet posts or subscriptions. Historical content remains viewable.
Dismiss
[PATCH -next 1/3] seq: Add a seq_overflow test.
2 views
Skip to first unread message
Joe Perches
Dec 11, 2013, 12:20:02 AM12/11/13
to
seq_printf and seq_puts returns are often misused.
Instead of checking the seq_printf or seq_puts return,
add a new seq_overflow function to test if a seq_file has
overflowed the available buffer space.
This will eventually allow seq_printf and seq_puts to be
converted to have a void return instead of the int return
that is often assumed to have a size_t value instead of an
error/no-error value.
Signed-off-by: Joe Perches <j...@perches.com>
---
fs/seq_file.c | 15 ++++++++-------
include/linux/seq_file.h | 2 ++
2 files changed, 10 insertions(+), 7 deletions(-)
diff --git a/fs/seq_file.c b/fs/seq_file.c
index 1d641bb..aab0736 100644
--- a/fs/seq_file.c
+++ b/fs/seq_file.c
@@ -14,16 +14,17 @@
#include <asm/uaccess.h>
#include <asm/page.h>
-
-/*
- * seq_files have a buffer which can may overflow. When this happens a larger
- * buffer is reallocated and all the data will be printed again.
- * The overflow state is true when m->count == m->size.
+/**
+ * seq_overflow - test if a seq_file has overflowed the space available
+ * @m: the seq_file handle
+ *
+ * Returns -1 when an overflow has occurred, 0 otherwise.
*/
-static bool seq_overflow(struct seq_file *m)
+int seq_overflow(struct seq_file *m)
{
- return m->count == m->size;
+ return m->count == m->size ? -1 : 0;
}
+EXPORT_SYMBOL(seq_overflow);
static void seq_set_overflow(struct seq_file *m)
{
diff --git a/include/linux/seq_file.h b/include/linux/seq_file.h
index 52e0097..f8f9dc0 100644
--- a/include/linux/seq_file.h
+++ b/include/linux/seq_file.h
@@ -107,6 +107,8 @@ int seq_write(struct seq_file *seq, const void *data, size_t len);
__printf(2, 3) int seq_printf(struct seq_file *, const char *, ...);
__printf(2, 0) int seq_vprintf(struct seq_file *, const char *, va_list args);
+int seq_overflow(struct seq_file *m);
+
int seq_path(struct seq_file *, const struct path *, const char *);
int seq_dentry(struct seq_file *, struct dentry *, const char *);
int seq_path_root(struct seq_file *m, const struct path *path,
--
1.8.1.2.459.gbcd45b4.dirty
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majo...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Instead of checking the seq_printf or seq_puts return,
add a new seq_overflow function to test if a seq_file has
overflowed the available buffer space.
This will eventually allow seq_printf and seq_puts to be
converted to have a void return instead of the int return
that is often assumed to have a size_t value instead of an
error/no-error value.
Signed-off-by: Joe Perches <j...@perches.com>
---
fs/seq_file.c | 15 ++++++++-------
include/linux/seq_file.h | 2 ++
2 files changed, 10 insertions(+), 7 deletions(-)
diff --git a/fs/seq_file.c b/fs/seq_file.c
index 1d641bb..aab0736 100644
--- a/fs/seq_file.c
+++ b/fs/seq_file.c
@@ -14,16 +14,17 @@
#include <asm/uaccess.h>
#include <asm/page.h>
-
-/*
- * seq_files have a buffer which can may overflow. When this happens a larger
- * buffer is reallocated and all the data will be printed again.
- * The overflow state is true when m->count == m->size.
+/**
+ * seq_overflow - test if a seq_file has overflowed the space available
+ * @m: the seq_file handle
+ *
+ * Returns -1 when an overflow has occurred, 0 otherwise.
*/
-static bool seq_overflow(struct seq_file *m)
+int seq_overflow(struct seq_file *m)
{
- return m->count == m->size;
+ return m->count == m->size ? -1 : 0;
}
+EXPORT_SYMBOL(seq_overflow);
static void seq_set_overflow(struct seq_file *m)
{
diff --git a/include/linux/seq_file.h b/include/linux/seq_file.h
index 52e0097..f8f9dc0 100644
--- a/include/linux/seq_file.h
+++ b/include/linux/seq_file.h
@@ -107,6 +107,8 @@ int seq_write(struct seq_file *seq, const void *data, size_t len);
__printf(2, 3) int seq_printf(struct seq_file *, const char *, ...);
__printf(2, 0) int seq_vprintf(struct seq_file *, const char *, va_list args);
+int seq_overflow(struct seq_file *m);
+
int seq_path(struct seq_file *, const struct path *, const char *);
int seq_dentry(struct seq_file *, struct dentry *, const char *);
int seq_path_root(struct seq_file *m, const struct path *path,
--
1.8.1.2.459.gbcd45b4.dirty
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majo...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Joe Perches
Dec 11, 2013, 12:20:02 AM12/11/13
to
Many uses of the return value of seq_printf/seq_puts/seq_putc are
incorrect. Many assume that the return value is the number of
chars emitted into a buffer like printf/puts/putc.
It would be better to make the return value of these functions void
to avoid these misuses.
Start to do so.
Convert seq_overflow to a public function from a static function.
Remove the return uses of seq_printf/seq_puts/seq_putc from net.
Add a seq_overflow function call instead.
Joe Perches (3):
seq: Add a seq_overflow test.
batman-adv: Use seq_overflow
netfilter: Use seq_overflow
fs/seq_file.c | 15 ++++----
include/linux/seq_file.h | 2 +
include/net/netfilter/nf_conntrack_acct.h | 3 +-
net/batman-adv/gateway_client.c | 25 ++++++------
net/ipv4/netfilter/nf_conntrack_l3proto_ipv4.c | 6 ++-
.../netfilter/nf_conntrack_l3proto_ipv4_compat.c | 42 +++++++++++++--------
net/ipv6/netfilter/nf_conntrack_l3proto_ipv6.c | 6 ++-
net/ipv6/netfilter/nf_conntrack_proto_icmpv6.c | 10 +++--
net/netfilter/nf_conntrack_acct.c | 11 +++---
net/netfilter/nf_conntrack_expect.c | 4 +-
net/netfilter/nf_conntrack_proto_dccp.c | 12 ++++--
net/netfilter/nf_conntrack_proto_gre.c | 15 +++++---
net/netfilter/nf_conntrack_proto_sctp.c | 12 ++++--
net/netfilter/nf_conntrack_proto_tcp.c | 11 ++++--
net/netfilter/nf_conntrack_proto_udp.c | 7 ++--
net/netfilter/nf_conntrack_proto_udplite.c | 7 ++--
net/netfilter/nf_conntrack_standalone.c | 44 +++++++++++++---------
net/netfilter/nf_log.c | 26 ++++++-------
net/netfilter/nfnetlink_log.c | 12 +++---
net/netfilter/nfnetlink_queue_core.c | 14 ++++---
net/netfilter/x_tables.c | 8 ++--
net/netfilter/xt_hashlimit.c | 34 +++++++++--------
22 files changed, 191 insertions(+), 135 deletions(-)
incorrect. Many assume that the return value is the number of
chars emitted into a buffer like printf/puts/putc.
It would be better to make the return value of these functions void
to avoid these misuses.
Start to do so.
Convert seq_overflow to a public function from a static function.
Remove the return uses of seq_printf/seq_puts/seq_putc from net.
Add a seq_overflow function call instead.
Joe Perches (3):
seq: Add a seq_overflow test.
batman-adv: Use seq_overflow
netfilter: Use seq_overflow
fs/seq_file.c | 15 ++++----
include/linux/seq_file.h | 2 +
include/net/netfilter/nf_conntrack_acct.h | 3 +-
net/batman-adv/gateway_client.c | 25 ++++++------
net/ipv4/netfilter/nf_conntrack_l3proto_ipv4.c | 6 ++-
.../netfilter/nf_conntrack_l3proto_ipv4_compat.c | 42 +++++++++++++--------
net/ipv6/netfilter/nf_conntrack_l3proto_ipv6.c | 6 ++-
net/ipv6/netfilter/nf_conntrack_proto_icmpv6.c | 10 +++--
net/netfilter/nf_conntrack_acct.c | 11 +++---
net/netfilter/nf_conntrack_expect.c | 4 +-
net/netfilter/nf_conntrack_proto_dccp.c | 12 ++++--
net/netfilter/nf_conntrack_proto_gre.c | 15 +++++---
net/netfilter/nf_conntrack_proto_sctp.c | 12 ++++--
net/netfilter/nf_conntrack_proto_tcp.c | 11 ++++--
net/netfilter/nf_conntrack_proto_udp.c | 7 ++--
net/netfilter/nf_conntrack_proto_udplite.c | 7 ++--
net/netfilter/nf_conntrack_standalone.c | 44 +++++++++++++---------
net/netfilter/nf_log.c | 26 ++++++-------
net/netfilter/nfnetlink_log.c | 12 +++---
net/netfilter/nfnetlink_queue_core.c | 14 ++++---
net/netfilter/x_tables.c | 8 ++--
net/netfilter/xt_hashlimit.c | 34 +++++++++--------
22 files changed, 191 insertions(+), 135 deletions(-)
Joe Perches
Dec 11, 2013, 12:20:02 AM12/11/13
to
Convert the uses of the return of seq_printf to
instead check seq_overflow to determine if a buffer
overflow has occurred.
This will eventually allow seq_printf & seq_puts to
be converted to a void return instead of the often
misused return that is often assumed to be an int for
the number of bytes emitted ala printk.
net/batman-adv/gateway_client.c | 25 ++++++++++++-------------
1 file changed, 12 insertions(+), 13 deletions(-)
diff --git a/net/batman-adv/gateway_client.c b/net/batman-adv/gateway_client.c
index 2449afa..dfa5d2d 100644
--- a/net/batman-adv/gateway_client.c
+++ b/net/batman-adv/gateway_client.c
@@ -517,29 +517,28 @@ static int batadv_write_buffer_text(struct batadv_priv *bat_priv,
{
struct batadv_gw_node *curr_gw;
struct batadv_neigh_node *router;
- int ret = -1;
router = batadv_orig_node_get_router(gw_node->orig_node);
if (!router)
- goto out;
+ return -1;
curr_gw = batadv_gw_get_selected_gw_node(bat_priv);
- ret = seq_printf(seq, "%s %pM (%3i) %pM [%10s]: %u.%u/%u.%u MBit\n",
- (curr_gw == gw_node ? "=>" : " "),
- gw_node->orig_node->orig,
- router->bat_iv.tq_avg, router->addr,
- router->if_incoming->net_dev->name,
- gw_node->bandwidth_down / 10,
- gw_node->bandwidth_down % 10,
- gw_node->bandwidth_up / 10,
- gw_node->bandwidth_up % 10);
+ seq_printf(seq, "%s %pM (%3i) %pM [%10s]: %u.%u/%u.%u MBit\n",
+ (curr_gw == gw_node ? "=>" : " "),
+ gw_node->orig_node->orig,
+ router->bat_iv.tq_avg, router->addr,
+ router->if_incoming->net_dev->name,
+ gw_node->bandwidth_down / 10,
+ gw_node->bandwidth_down % 10,
+ gw_node->bandwidth_up / 10,
+ gw_node->bandwidth_up % 10);
batadv_neigh_node_free_ref(router);
if (curr_gw)
batadv_gw_node_free_ref(curr_gw);
-out:
- return ret;
+
+ return seq_overflow(seq);
}
int batadv_gw_client_seq_print_text(struct seq_file *seq, void *offset)
instead check seq_overflow to determine if a buffer
overflow has occurred.
This will eventually allow seq_printf & seq_puts to
be converted to a void return instead of the often
misused return that is often assumed to be an int for
the number of bytes emitted ala printk.
net/batman-adv/gateway_client.c | 25 ++++++++++++-------------
1 file changed, 12 insertions(+), 13 deletions(-)
diff --git a/net/batman-adv/gateway_client.c b/net/batman-adv/gateway_client.c
index 2449afa..dfa5d2d 100644
--- a/net/batman-adv/gateway_client.c
+++ b/net/batman-adv/gateway_client.c
@@ -517,29 +517,28 @@ static int batadv_write_buffer_text(struct batadv_priv *bat_priv,
{
struct batadv_gw_node *curr_gw;
struct batadv_neigh_node *router;
- int ret = -1;
router = batadv_orig_node_get_router(gw_node->orig_node);
if (!router)
- goto out;
+ return -1;
curr_gw = batadv_gw_get_selected_gw_node(bat_priv);
- ret = seq_printf(seq, "%s %pM (%3i) %pM [%10s]: %u.%u/%u.%u MBit\n",
- (curr_gw == gw_node ? "=>" : " "),
- gw_node->orig_node->orig,
- router->bat_iv.tq_avg, router->addr,
- router->if_incoming->net_dev->name,
- gw_node->bandwidth_down / 10,
- gw_node->bandwidth_down % 10,
- gw_node->bandwidth_up / 10,
- gw_node->bandwidth_up % 10);
+ seq_printf(seq, "%s %pM (%3i) %pM [%10s]: %u.%u/%u.%u MBit\n",
+ (curr_gw == gw_node ? "=>" : " "),
+ gw_node->orig_node->orig,
+ router->bat_iv.tq_avg, router->addr,
+ router->if_incoming->net_dev->name,
+ gw_node->bandwidth_down / 10,
+ gw_node->bandwidth_down % 10,
+ gw_node->bandwidth_up / 10,
+ gw_node->bandwidth_up % 10);
batadv_neigh_node_free_ref(router);
if (curr_gw)
batadv_gw_node_free_ref(curr_gw);
-out:
- return ret;
+
+ return seq_overflow(seq);
}
int batadv_gw_client_seq_print_text(struct seq_file *seq, void *offset)
Joe Perches
Dec 11, 2013, 12:20:02 AM12/11/13
to
Convert the uses of the return of seq_printf/seq_puts/seq_putc to
use check seq_overflow to determine if a buffer overflow has occurred.
This will eventually allow seq_printf & seq_puts to be converted to a
void return instead of the often misused return that is often assumed
to be an int for the number of bytes emitted ala printk.
Convert seq_print_acct to return int.
include/net/netfilter/nf_conntrack_acct.h | 3 +-
diff --git a/include/net/netfilter/nf_conntrack_acct.h b/include/net/netfilter/nf_conntrack_acct.h
index 79d8d16..939bffe 100644
--- a/include/net/netfilter/nf_conntrack_acct.h
+++ b/include/net/netfilter/nf_conntrack_acct.h
@@ -46,8 +46,7 @@ struct nf_conn_acct *nf_ct_acct_ext_add(struct nf_conn *ct, gfp_t gfp)
return acct;
};
-unsigned int seq_print_acct(struct seq_file *s, const struct nf_conn *ct,
- int dir);
+int seq_print_acct(struct seq_file *s, const struct nf_conn *ct, int dir);
/* Check if connection tracking accounting is enabled */
static inline bool nf_ct_acct_enabled(struct net *net)
diff --git a/net/ipv4/netfilter/nf_conntrack_l3proto_ipv4.c b/net/ipv4/netfilter/nf_conntrack_l3proto_ipv4.c
index ecd8bec..aa07f0b 100644
--- a/net/ipv4/netfilter/nf_conntrack_l3proto_ipv4.c
+++ b/net/ipv4/netfilter/nf_conntrack_l3proto_ipv4.c
@@ -59,8 +59,10 @@ static bool ipv4_invert_tuple(struct nf_conntrack_tuple *tuple,
static int ipv4_print_tuple(struct seq_file *s,
const struct nf_conntrack_tuple *tuple)
{
- return seq_printf(s, "src=%pI4 dst=%pI4 ",
- &tuple->src.u3.ip, &tuple->dst.u3.ip);
+ seq_printf(s, "src=%pI4 dst=%pI4 ",
+ &tuple->src.u3.ip, &tuple->dst.u3.ip);
+
+ return seq_overflow(s);
}
static int ipv4_get_l4proto(const struct sk_buff *skb, unsigned int nhoff,
diff --git a/net/ipv4/netfilter/nf_conntrack_l3proto_ipv4_compat.c b/net/ipv4/netfilter/nf_conntrack_l3proto_ipv4_compat.c
index 4c48e43..67ba510 100644
--- a/net/ipv4/netfilter/nf_conntrack_l3proto_ipv4_compat.c
+++ b/net/ipv4/netfilter/nf_conntrack_l3proto_ipv4_compat.c
@@ -104,10 +104,10 @@ static int ct_show_secctx(struct seq_file *s, const struct nf_conn *ct)
if (ret)
return 0;
- ret = seq_printf(s, "secctx=%s ", secctx);
+ seq_printf(s, "secctx=%s ", secctx);
security_release_secctx(secctx, len);
- return ret;
+ return seq_overflow(s);
}
#else
static inline int ct_show_secctx(struct seq_file *s, const struct nf_conn *ct)
@@ -141,10 +141,11 @@ static int ct_seq_show(struct seq_file *s, void *v)
NF_CT_ASSERT(l4proto);
ret = -ENOSPC;
- if (seq_printf(s, "%-8s %u %ld ",
- l4proto->name, nf_ct_protonum(ct),
- timer_pending(&ct->timeout)
- ? (long)(ct->timeout.expires - jiffies)/HZ : 0) != 0)
+ seq_printf(s, "%-8s %u %ld ",
+ l4proto->name, nf_ct_protonum(ct),
+ timer_pending(&ct->timeout)
+ ? (long)(ct->timeout.expires - jiffies)/HZ : 0);
+ if (seq_overflow(s))
goto release;
if (l4proto->print_conntrack && l4proto->print_conntrack(s, ct))
@@ -154,35 +155,44 @@ static int ct_seq_show(struct seq_file *s, void *v)
l3proto, l4proto))
goto release;
- if (seq_print_acct(s, ct, IP_CT_DIR_ORIGINAL))
+ seq_print_acct(s, ct, IP_CT_DIR_ORIGINAL);
+ if (seq_overflow(s))
goto release;
- if (!(test_bit(IPS_SEEN_REPLY_BIT, &ct->status)))
- if (seq_printf(s, "[UNREPLIED] "))
+ if (!(test_bit(IPS_SEEN_REPLY_BIT, &ct->status))) {
+ seq_printf(s, "[UNREPLIED] ");
+ if (seq_overflow(s))
goto release;
+ }
if (print_tuple(s, &ct->tuplehash[IP_CT_DIR_REPLY].tuple,
l3proto, l4proto))
goto release;
- if (seq_print_acct(s, ct, IP_CT_DIR_REPLY))
+ seq_print_acct(s, ct, IP_CT_DIR_REPLY);
+ if (seq_overflow(s))
goto release;
- if (test_bit(IPS_ASSURED_BIT, &ct->status))
- if (seq_printf(s, "[ASSURED] "))
+ if (test_bit(IPS_ASSURED_BIT, &ct->status)) {
+ seq_printf(s, "[ASSURED] ");
+ if (seq_overflow(s))
goto release;
+ }
#ifdef CONFIG_NF_CONNTRACK_MARK
- if (seq_printf(s, "mark=%u ", ct->mark))
+ seq_printf(s, "mark=%u ", ct->mark);
+ if (seq_overflow(s))
goto release;
#endif
if (ct_show_secctx(s, ct))
goto release;
- if (seq_printf(s, "use=%u\n", atomic_read(&ct->ct_general.use)))
+ seq_printf(s, "use=%u\n", atomic_read(&ct->ct_general.use));
+ if (seq_overflow(s))
goto release;
ret = 0;
+
release:
nf_ct_put(ct);
return ret;
@@ -297,7 +307,9 @@ static int exp_seq_show(struct seq_file *s, void *v)
__nf_ct_l3proto_find(exp->tuple.src.l3num),
__nf_ct_l4proto_find(exp->tuple.src.l3num,
exp->tuple.dst.protonum));
- return seq_putc(s, '\n');
+ seq_putc(s, '\n');
+
+ return seq_overflow(s);
}
static const struct seq_operations exp_seq_ops = {
diff --git a/net/ipv6/netfilter/nf_conntrack_l3proto_ipv6.c b/net/ipv6/netfilter/nf_conntrack_l3proto_ipv6.c
index 4cbc6b2..d5b0cd4 100644
--- a/net/ipv6/netfilter/nf_conntrack_l3proto_ipv6.c
+++ b/net/ipv6/netfilter/nf_conntrack_l3proto_ipv6.c
@@ -63,8 +63,10 @@ static bool ipv6_invert_tuple(struct nf_conntrack_tuple *tuple,
static int ipv6_print_tuple(struct seq_file *s,
const struct nf_conntrack_tuple *tuple)
{
- return seq_printf(s, "src=%pI6 dst=%pI6 ",
- tuple->src.u3.ip6, tuple->dst.u3.ip6);
+ seq_printf(s, "src=%pI6 dst=%pI6 ",
+ tuple->src.u3.ip6, tuple->dst.u3.ip6);
+
+ return seq_overflow(s);
}
static int ipv6_get_l4proto(const struct sk_buff *skb, unsigned int nhoff,
diff --git a/net/ipv6/netfilter/nf_conntrack_proto_icmpv6.c b/net/ipv6/netfilter/nf_conntrack_proto_icmpv6.c
index b3807c5..4d6d138 100644
--- a/net/ipv6/netfilter/nf_conntrack_proto_icmpv6.c
+++ b/net/ipv6/netfilter/nf_conntrack_proto_icmpv6.c
@@ -87,10 +87,12 @@ static bool icmpv6_invert_tuple(struct nf_conntrack_tuple *tuple,
static int icmpv6_print_tuple(struct seq_file *s,
const struct nf_conntrack_tuple *tuple)
{
- return seq_printf(s, "type=%u code=%u id=%u ",
- tuple->dst.u.icmp.type,
- tuple->dst.u.icmp.code,
- ntohs(tuple->src.u.icmp.id));
+ seq_printf(s, "type=%u code=%u id=%u ",
+ tuple->dst.u.icmp.type,
+ tuple->dst.u.icmp.code,
+ ntohs(tuple->src.u.icmp.id));
+
+ return seq_overflow(s);
}
static unsigned int *icmpv6_get_timeouts(struct net *net)
diff --git a/net/netfilter/nf_conntrack_acct.c b/net/netfilter/nf_conntrack_acct.c
index a4b5e2a..897d9cc 100644
--- a/net/netfilter/nf_conntrack_acct.c
+++ b/net/netfilter/nf_conntrack_acct.c
@@ -36,8 +36,7 @@ static struct ctl_table acct_sysctl_table[] = {
};
#endif /* CONFIG_SYSCTL */
-unsigned int
-seq_print_acct(struct seq_file *s, const struct nf_conn *ct, int dir)
+int seq_print_acct(struct seq_file *s, const struct nf_conn *ct, int dir)
{
struct nf_conn_acct *acct;
struct nf_conn_counter *counter;
@@ -47,9 +46,11 @@ seq_print_acct(struct seq_file *s, const struct nf_conn *ct, int dir)
return 0;
counter = acct->counter;
- return seq_printf(s, "packets=%llu bytes=%llu ",
- (unsigned long long)atomic64_read(&counter[dir].packets),
- (unsigned long long)atomic64_read(&counter[dir].bytes));
+ seq_printf(s, "packets=%llu bytes=%llu ",
+ (unsigned long long)atomic64_read(&counter[dir].packets),
+ (unsigned long long)atomic64_read(&counter[dir].bytes));
+
+ return seq_overflow(s);
};
EXPORT_SYMBOL_GPL(seq_print_acct);
diff --git a/net/netfilter/nf_conntrack_expect.c b/net/netfilter/nf_conntrack_expect.c
index 4fd1ca9..15bbf36 100644
--- a/net/netfilter/nf_conntrack_expect.c
+++ b/net/netfilter/nf_conntrack_expect.c
@@ -544,7 +544,9 @@ static int exp_seq_show(struct seq_file *s, void *v)
helper->expect_policy[expect->class].name);
}
- return seq_putc(s, '\n');
+ seq_putc(s, '\n');
+
+ return seq_overflow(s);
}
static const struct seq_operations exp_seq_ops = {
diff --git a/net/netfilter/nf_conntrack_proto_dccp.c b/net/netfilter/nf_conntrack_proto_dccp.c
index a99b6c3..a0f95a6 100644
--- a/net/netfilter/nf_conntrack_proto_dccp.c
+++ b/net/netfilter/nf_conntrack_proto_dccp.c
@@ -621,14 +621,18 @@ out_invalid:
static int dccp_print_tuple(struct seq_file *s,
const struct nf_conntrack_tuple *tuple)
{
- return seq_printf(s, "sport=%hu dport=%hu ",
- ntohs(tuple->src.u.dccp.port),
- ntohs(tuple->dst.u.dccp.port));
+ seq_printf(s, "sport=%hu dport=%hu ",
+ ntohs(tuple->src.u.dccp.port),
+ ntohs(tuple->dst.u.dccp.port));
+
+ return seq_overflow(s);
}
static int dccp_print_conntrack(struct seq_file *s, struct nf_conn *ct)
{
- return seq_printf(s, "%s ", dccp_state_names[ct->proto.dccp.state]);
+ seq_printf(s, "%s ", dccp_state_names[ct->proto.dccp.state]);
+
+ return seq_overflow(s);
}
#if IS_ENABLED(CONFIG_NF_CT_NETLINK)
diff --git a/net/netfilter/nf_conntrack_proto_gre.c b/net/netfilter/nf_conntrack_proto_gre.c
index 9d9c0da..234f7f6 100644
--- a/net/netfilter/nf_conntrack_proto_gre.c
+++ b/net/netfilter/nf_conntrack_proto_gre.c
@@ -230,17 +230,20 @@ static bool gre_pkt_to_tuple(const struct sk_buff *skb, unsigned int dataoff,
static int gre_print_tuple(struct seq_file *s,
const struct nf_conntrack_tuple *tuple)
{
- return seq_printf(s, "srckey=0x%x dstkey=0x%x ",
- ntohs(tuple->src.u.gre.key),
- ntohs(tuple->dst.u.gre.key));
+ seq_printf(s, "srckey=0x%x dstkey=0x%x ",
+ ntohs(tuple->src.u.gre.key), ntohs(tuple->dst.u.gre.key));
+
+ return seq_overflow(s);
}
/* print private data for conntrack */
static int gre_print_conntrack(struct seq_file *s, struct nf_conn *ct)
{
- return seq_printf(s, "timeout=%u, stream_timeout=%u ",
- (ct->proto.gre.timeout / HZ),
- (ct->proto.gre.stream_timeout / HZ));
+ seq_printf(s, "timeout=%u, stream_timeout=%u ",
+ ct->proto.gre.timeout / HZ,
+ ct->proto.gre.stream_timeout / HZ);
+
+ return seq_overflow(s);
}
static unsigned int *gre_get_timeouts(struct net *net)
diff --git a/net/netfilter/nf_conntrack_proto_sctp.c b/net/netfilter/nf_conntrack_proto_sctp.c
index 1314d33..b8b5708 100644
--- a/net/netfilter/nf_conntrack_proto_sctp.c
+++ b/net/netfilter/nf_conntrack_proto_sctp.c
@@ -169,9 +169,11 @@ static bool sctp_invert_tuple(struct nf_conntrack_tuple *tuple,
static int sctp_print_tuple(struct seq_file *s,
const struct nf_conntrack_tuple *tuple)
{
- return seq_printf(s, "sport=%hu dport=%hu ",
- ntohs(tuple->src.u.sctp.port),
- ntohs(tuple->dst.u.sctp.port));
+ seq_printf(s, "sport=%hu dport=%hu ",
+ ntohs(tuple->src.u.sctp.port),
+ ntohs(tuple->dst.u.sctp.port));
+
+ return seq_overflow(s);
}
/* Print out the private part of the conntrack. */
@@ -183,7 +185,9 @@ static int sctp_print_conntrack(struct seq_file *s, struct nf_conn *ct)
state = ct->proto.sctp.state;
spin_unlock_bh(&ct->lock);
- return seq_printf(s, "%s ", sctp_conntrack_names[state]);
+ seq_printf(s, "%s ", sctp_conntrack_names[state]);
+
+ return seq_overflow(s);
}
#define for_each_sctp_chunk(skb, sch, _sch, offset, dataoff, count) \
diff --git a/net/netfilter/nf_conntrack_proto_tcp.c b/net/netfilter/nf_conntrack_proto_tcp.c
index 44d1ea3..d21de09e 100644
--- a/net/netfilter/nf_conntrack_proto_tcp.c
+++ b/net/netfilter/nf_conntrack_proto_tcp.c
@@ -305,9 +305,10 @@ static bool tcp_invert_tuple(struct nf_conntrack_tuple *tuple,
static int tcp_print_tuple(struct seq_file *s,
const struct nf_conntrack_tuple *tuple)
{
- return seq_printf(s, "sport=%hu dport=%hu ",
- ntohs(tuple->src.u.tcp.port),
- ntohs(tuple->dst.u.tcp.port));
+ seq_printf(s, "sport=%hu dport=%hu ",
+ ntohs(tuple->src.u.tcp.port), ntohs(tuple->dst.u.tcp.port));
+
+ return seq_overflow(s);
}
/* Print out the private part of the conntrack. */
@@ -319,7 +320,9 @@ static int tcp_print_conntrack(struct seq_file *s, struct nf_conn *ct)
state = ct->proto.tcp.state;
spin_unlock_bh(&ct->lock);
- return seq_printf(s, "%s ", tcp_conntrack_names[state]);
+ seq_printf(s, "%s ", tcp_conntrack_names[state]);
+
+ return seq_overflow(s);
}
static unsigned int get_conntrack_index(const struct tcphdr *tcph)
diff --git a/net/netfilter/nf_conntrack_proto_udp.c b/net/netfilter/nf_conntrack_proto_udp.c
index 9d7721c..c2b52da 100644
--- a/net/netfilter/nf_conntrack_proto_udp.c
+++ b/net/netfilter/nf_conntrack_proto_udp.c
@@ -66,9 +66,10 @@ static bool udp_invert_tuple(struct nf_conntrack_tuple *tuple,
static int udp_print_tuple(struct seq_file *s,
const struct nf_conntrack_tuple *tuple)
{
- return seq_printf(s, "sport=%hu dport=%hu ",
- ntohs(tuple->src.u.udp.port),
- ntohs(tuple->dst.u.udp.port));
+ seq_printf(s, "sport=%hu dport=%hu ",
+ ntohs(tuple->src.u.udp.port), ntohs(tuple->dst.u.udp.port));
+
+ return seq_overflow(s);
}
static unsigned int *udp_get_timeouts(struct net *net)
diff --git a/net/netfilter/nf_conntrack_proto_udplite.c b/net/netfilter/nf_conntrack_proto_udplite.c
index 2750e6c..7c4fb9b 100644
--- a/net/netfilter/nf_conntrack_proto_udplite.c
+++ b/net/netfilter/nf_conntrack_proto_udplite.c
@@ -74,9 +74,10 @@ static bool udplite_invert_tuple(struct nf_conntrack_tuple *tuple,
static int udplite_print_tuple(struct seq_file *s,
const struct nf_conntrack_tuple *tuple)
{
- return seq_printf(s, "sport=%hu dport=%hu ",
- ntohs(tuple->src.u.udp.port),
- ntohs(tuple->dst.u.udp.port));
+ seq_printf(s, "sport=%hu dport=%hu ",
+ ntohs(tuple->src.u.udp.port), ntohs(tuple->dst.u.udp.port));
+
+ return seq_overflow(s);
}
static unsigned int *udplite_get_timeouts(struct net *net)
diff --git a/net/netfilter/nf_conntrack_standalone.c b/net/netfilter/nf_conntrack_standalone.c
index f641751..6666119 100644
--- a/net/netfilter/nf_conntrack_standalone.c
+++ b/net/netfilter/nf_conntrack_standalone.c
@@ -129,10 +129,10 @@ static int ct_show_secctx(struct seq_file *s, const struct nf_conn *ct)
if (ret)
return 0;
- ret = seq_printf(s, "secctx=%s ", secctx);
+ seq_printf(s, "secctx=%s ", secctx);
security_release_secctx(secctx, len);
- return ret;
+ return seq_overflow(s);
}
#else
static inline int ct_show_secctx(struct seq_file *s, const struct nf_conn *ct)
@@ -156,8 +156,9 @@ static int ct_show_delta_time(struct seq_file *s, const struct nf_conn *ct)
else
delta_time = 0;
- return seq_printf(s, "delta-time=%llu ",
- (unsigned long long)delta_time);
+ seq_printf(s, "delta-time=%llu ",
+ (unsigned long long)delta_time);
+ return seq_overflow(s);
}
return 0;
}
@@ -192,11 +193,12 @@ static int ct_seq_show(struct seq_file *s, void *v)
NF_CT_ASSERT(l4proto);
ret = -ENOSPC;
- if (seq_printf(s, "%-8s %u %-8s %u %ld ",
- l3proto->name, nf_ct_l3num(ct),
- l4proto->name, nf_ct_protonum(ct),
- timer_pending(&ct->timeout)
- ? (long)(ct->timeout.expires - jiffies)/HZ : 0) != 0)
+ seq_printf(s, "%-8s %u %-8s %u %ld ",
+ l3proto->name, nf_ct_l3num(ct),
+ l4proto->name, nf_ct_protonum(ct),
+ timer_pending(&ct->timeout)
+ ? (long)(ct->timeout.expires - jiffies) / HZ : 0);
+ if (seq_overflow(s))
goto release;
if (l4proto->print_conntrack && l4proto->print_conntrack(s, ct))
@@ -209,9 +211,11 @@ static int ct_seq_show(struct seq_file *s, void *v)
if (seq_print_acct(s, ct, IP_CT_DIR_ORIGINAL))
goto release;
- if (!(test_bit(IPS_SEEN_REPLY_BIT, &ct->status)))
- if (seq_printf(s, "[UNREPLIED] "))
+ if (!(test_bit(IPS_SEEN_REPLY_BIT, &ct->status))) {
+ seq_printf(s, "[UNREPLIED] ");
+ if (seq_overflow(s))
goto release;
+ }
if (print_tuple(s, &ct->tuplehash[IP_CT_DIR_REPLY].tuple,
l3proto, l4proto))
@@ -220,12 +224,15 @@ static int ct_seq_show(struct seq_file *s, void *v)
if (seq_print_acct(s, ct, IP_CT_DIR_REPLY))
goto release;
- if (test_bit(IPS_ASSURED_BIT, &ct->status))
- if (seq_printf(s, "[ASSURED] "))
+ if (test_bit(IPS_ASSURED_BIT, &ct->status)) {
+ seq_printf(s, "[ASSURED] ");
+ if (seq_overflow(s))
goto release;
+ }
#if defined(CONFIG_NF_CONNTRACK_MARK)
- if (seq_printf(s, "mark=%u ", ct->mark))
+ seq_printf(s, "mark=%u ", ct->mark);
+ if (seq_overflow(s))
goto release;
#endif
@@ -233,17 +240,20 @@ static int ct_seq_show(struct seq_file *s, void *v)
goto release;
#ifdef CONFIG_NF_CONNTRACK_ZONES
- if (seq_printf(s, "zone=%u ", nf_ct_zone(ct)))
+ seq_printf(s, "zone=%u ", nf_ct_zone(ct));
+ if (seq_overflow(s))
goto release;
#endif
if (ct_show_delta_time(s, ct))
goto release;
- if (seq_printf(s, "use=%u\n", atomic_read(&ct->ct_general.use)))
+ seq_printf(s, "use=%u\n", atomic_read(&ct->ct_general.use));
+ if (seq_overflow(s))
goto release;
- ret = 0;
+ return 0;
+
release:
nf_ct_put(ct);
return ret;
diff --git a/net/netfilter/nf_log.c b/net/netfilter/nf_log.c
index 85296d4..386327e 100644
--- a/net/netfilter/nf_log.c
+++ b/net/netfilter/nf_log.c
@@ -189,32 +189,32 @@ static int seq_show(struct seq_file *s, void *v)
loff_t *pos = v;
const struct nf_logger *logger;
struct nf_logger *t;
- int ret;
struct net *net = seq_file_net(s);
logger = rcu_dereference_protected(net->nf.nf_loggers[*pos],
lockdep_is_held(&nf_log_mutex));
if (!logger)
- ret = seq_printf(s, "%2lld NONE (", *pos);
+ seq_printf(s, "%2lld NONE (", *pos);
else
- ret = seq_printf(s, "%2lld %s (", *pos, logger->name);
-
- if (ret < 0)
- return ret;
+ seq_printf(s, "%2lld %s (", *pos, logger->name);
+ if (seq_overflow(s))
+ return seq_overflow(s);
list_for_each_entry(t, &nf_loggers_l[*pos], list[*pos]) {
- ret = seq_printf(s, "%s", t->name);
- if (ret < 0)
- return ret;
+ seq_printf(s, "%s", t->name);
+ if (seq_overflow(s))
+ return seq_overflow(s);
if (&t->list[*pos] != nf_loggers_l[*pos].prev) {
- ret = seq_printf(s, ",");
- if (ret < 0)
- return ret;
+ seq_printf(s, ",");
+ if (seq_overflow(s))
+ return seq_overflow(s);
}
}
- return seq_printf(s, ")\n");
+ seq_printf(s, ")\n");
+
+ return seq_overflow(s);
}
static const struct seq_operations nflog_seq_ops = {
diff --git a/net/netfilter/nfnetlink_log.c b/net/netfilter/nfnetlink_log.c
index 3c4b69e..99d4a6c 100644
--- a/net/netfilter/nfnetlink_log.c
+++ b/net/netfilter/nfnetlink_log.c
@@ -1001,11 +1001,13 @@ static int seq_show(struct seq_file *s, void *v)
{
const struct nfulnl_instance *inst = v;
- return seq_printf(s, "%5d %6d %5d %1d %5d %6d %2d\n",
- inst->group_num,
- inst->peer_portid, inst->qlen,
- inst->copy_mode, inst->copy_range,
- inst->flushtimeout, atomic_read(&inst->use));
+ seq_printf(s, "%5d %6d %5d %1d %5d %6d %2d\n",
+ inst->group_num,
+ inst->peer_portid, inst->qlen,
+ inst->copy_mode, inst->copy_range,
+ inst->flushtimeout, atomic_read(&inst->use));
+
+ return seq_overflow(s);
}
static const struct seq_operations nful_seq_ops = {
diff --git a/net/netfilter/nfnetlink_queue_core.c b/net/netfilter/nfnetlink_queue_core.c
index 21258cf..c3f4762 100644
--- a/net/netfilter/nfnetlink_queue_core.c
+++ b/net/netfilter/nfnetlink_queue_core.c
@@ -1253,12 +1253,14 @@ static int seq_show(struct seq_file *s, void *v)
{
const struct nfqnl_instance *inst = v;
- return seq_printf(s, "%5d %6d %5d %1d %5d %5d %5d %8d %2d\n",
- inst->queue_num,
- inst->peer_portid, inst->queue_total,
- inst->copy_mode, inst->copy_range,
- inst->queue_dropped, inst->queue_user_dropped,
- inst->id_sequence, 1);
+ seq_printf(s, "%5d %6d %5d %1d %5d %5d %5d %8d %2d\n",
+ inst->queue_num,
+ inst->peer_portid, inst->queue_total,
+ inst->copy_mode, inst->copy_range,
+ inst->queue_dropped, inst->queue_user_dropped,
+ inst->id_sequence, 1);
+
+ return seq_overflow(s);
}
static const struct seq_operations nfqnl_seq_ops = {
diff --git a/net/netfilter/x_tables.c b/net/netfilter/x_tables.c
index 227aa11..a74f49f 100644
--- a/net/netfilter/x_tables.c
+++ b/net/netfilter/x_tables.c
@@ -983,10 +983,12 @@ static int xt_table_seq_show(struct seq_file *seq, void *v)
{
struct xt_table *table = list_entry(v, struct xt_table, list);
- if (strlen(table->name))
- return seq_printf(seq, "%s\n", table->name);
- else
+ if (!strlen(table->name))
return 0;
+
+ seq_printf(seq, "%s\n", table->name);
+
+ return seq_overflow(seq);
}
static const struct seq_operations xt_table_seq_ops = {
diff --git a/net/netfilter/xt_hashlimit.c b/net/netfilter/xt_hashlimit.c
index a3910fc..c229655 100644
--- a/net/netfilter/xt_hashlimit.c
+++ b/net/netfilter/xt_hashlimit.c
@@ -797,25 +797,27 @@ static int dl_seq_real_show(struct dsthash_ent *ent, u_int8_t family,
switch (family) {
case NFPROTO_IPV4:
- res = seq_printf(s, "%ld %pI4:%u->%pI4:%u %u %u %u\n",
- (long)(ent->expires - jiffies)/HZ,
- &ent->dst.ip.src,
- ntohs(ent->dst.src_port),
- &ent->dst.ip.dst,
- ntohs(ent->dst.dst_port),
- ent->rateinfo.credit, ent->rateinfo.credit_cap,
- ent->rateinfo.cost);
+ seq_printf(s, "%ld %pI4:%u->%pI4:%u %u %u %u\n",
+ (long)(ent->expires - jiffies)/HZ,
+ &ent->dst.ip.src,
+ ntohs(ent->dst.src_port),
+ &ent->dst.ip.dst,
+ ntohs(ent->dst.dst_port),
+ ent->rateinfo.credit, ent->rateinfo.credit_cap,
+ ent->rateinfo.cost);
+ res = seq_overflow(s);
break;
#if IS_ENABLED(CONFIG_IP6_NF_IPTABLES)
case NFPROTO_IPV6:
- res = seq_printf(s, "%ld %pI6:%u->%pI6:%u %u %u %u\n",
- (long)(ent->expires - jiffies)/HZ,
- &ent->dst.ip6.src,
- ntohs(ent->dst.src_port),
- &ent->dst.ip6.dst,
- ntohs(ent->dst.dst_port),
- ent->rateinfo.credit, ent->rateinfo.credit_cap,
- ent->rateinfo.cost);
+ seq_printf(s, "%ld %pI6:%u->%pI6:%u %u %u %u\n",
+ (long)(ent->expires - jiffies)/HZ,
+ &ent->dst.ip6.src,
+ ntohs(ent->dst.src_port),
+ &ent->dst.ip6.dst,
+ ntohs(ent->dst.dst_port),
+ ent->rateinfo.credit, ent->rateinfo.credit_cap,
+ ent->rateinfo.cost);
+ res = seq_overflow(s);
break;
#endif
default:
use check seq_overflow to determine if a buffer overflow has occurred.
This will eventually allow seq_printf & seq_puts to be converted to a
void return instead of the often misused return that is often assumed
to be an int for the number of bytes emitted ala printk.
include/net/netfilter/nf_conntrack_acct.h | 3 +-
net/ipv4/netfilter/nf_conntrack_l3proto_ipv4.c | 6 ++-
.../netfilter/nf_conntrack_l3proto_ipv4_compat.c | 42 +++++++++++++--------
net/ipv6/netfilter/nf_conntrack_l3proto_ipv6.c | 6 ++-
net/ipv6/netfilter/nf_conntrack_proto_icmpv6.c | 10 +++--
net/netfilter/nf_conntrack_acct.c | 11 +++---
net/netfilter/nf_conntrack_expect.c | 4 +-
net/netfilter/nf_conntrack_proto_dccp.c | 12 ++++--
net/netfilter/nf_conntrack_proto_gre.c | 15 +++++---
net/netfilter/nf_conntrack_proto_sctp.c | 12 ++++--
net/netfilter/nf_conntrack_proto_tcp.c | 11 ++++--
net/netfilter/nf_conntrack_proto_udp.c | 7 ++--
net/netfilter/nf_conntrack_proto_udplite.c | 7 ++--
net/netfilter/nf_conntrack_standalone.c | 44 +++++++++++++---------
net/netfilter/nf_log.c | 26 ++++++-------
net/netfilter/nfnetlink_log.c | 12 +++---
net/netfilter/nfnetlink_queue_core.c | 14 ++++---
net/netfilter/x_tables.c | 8 ++--
net/netfilter/xt_hashlimit.c | 34 +++++++++--------
19 files changed, 169 insertions(+), 115 deletions(-)
.../netfilter/nf_conntrack_l3proto_ipv4_compat.c | 42 +++++++++++++--------
net/ipv6/netfilter/nf_conntrack_l3proto_ipv6.c | 6 ++-
net/ipv6/netfilter/nf_conntrack_proto_icmpv6.c | 10 +++--
net/netfilter/nf_conntrack_acct.c | 11 +++---
net/netfilter/nf_conntrack_expect.c | 4 +-
net/netfilter/nf_conntrack_proto_dccp.c | 12 ++++--
net/netfilter/nf_conntrack_proto_gre.c | 15 +++++---
net/netfilter/nf_conntrack_proto_sctp.c | 12 ++++--
net/netfilter/nf_conntrack_proto_tcp.c | 11 ++++--
net/netfilter/nf_conntrack_proto_udp.c | 7 ++--
net/netfilter/nf_conntrack_proto_udplite.c | 7 ++--
net/netfilter/nf_conntrack_standalone.c | 44 +++++++++++++---------
net/netfilter/nf_log.c | 26 ++++++-------
net/netfilter/nfnetlink_log.c | 12 +++---
net/netfilter/nfnetlink_queue_core.c | 14 ++++---
net/netfilter/x_tables.c | 8 ++--
net/netfilter/xt_hashlimit.c | 34 +++++++++--------
diff --git a/include/net/netfilter/nf_conntrack_acct.h b/include/net/netfilter/nf_conntrack_acct.h
index 79d8d16..939bffe 100644
--- a/include/net/netfilter/nf_conntrack_acct.h
+++ b/include/net/netfilter/nf_conntrack_acct.h
@@ -46,8 +46,7 @@ struct nf_conn_acct *nf_ct_acct_ext_add(struct nf_conn *ct, gfp_t gfp)
return acct;
};
-unsigned int seq_print_acct(struct seq_file *s, const struct nf_conn *ct,
- int dir);
+int seq_print_acct(struct seq_file *s, const struct nf_conn *ct, int dir);
/* Check if connection tracking accounting is enabled */
static inline bool nf_ct_acct_enabled(struct net *net)
diff --git a/net/ipv4/netfilter/nf_conntrack_l3proto_ipv4.c b/net/ipv4/netfilter/nf_conntrack_l3proto_ipv4.c
index ecd8bec..aa07f0b 100644
--- a/net/ipv4/netfilter/nf_conntrack_l3proto_ipv4.c
+++ b/net/ipv4/netfilter/nf_conntrack_l3proto_ipv4.c
@@ -59,8 +59,10 @@ static bool ipv4_invert_tuple(struct nf_conntrack_tuple *tuple,
static int ipv4_print_tuple(struct seq_file *s,
const struct nf_conntrack_tuple *tuple)
{
- return seq_printf(s, "src=%pI4 dst=%pI4 ",
- &tuple->src.u3.ip, &tuple->dst.u3.ip);
+ seq_printf(s, "src=%pI4 dst=%pI4 ",
+ &tuple->src.u3.ip, &tuple->dst.u3.ip);
+
+ return seq_overflow(s);
}
static int ipv4_get_l4proto(const struct sk_buff *skb, unsigned int nhoff,
diff --git a/net/ipv4/netfilter/nf_conntrack_l3proto_ipv4_compat.c b/net/ipv4/netfilter/nf_conntrack_l3proto_ipv4_compat.c
index 4c48e43..67ba510 100644
--- a/net/ipv4/netfilter/nf_conntrack_l3proto_ipv4_compat.c
+++ b/net/ipv4/netfilter/nf_conntrack_l3proto_ipv4_compat.c
@@ -104,10 +104,10 @@ static int ct_show_secctx(struct seq_file *s, const struct nf_conn *ct)
if (ret)
return 0;
- ret = seq_printf(s, "secctx=%s ", secctx);
+ seq_printf(s, "secctx=%s ", secctx);
security_release_secctx(secctx, len);
- return ret;
+ return seq_overflow(s);
}
#else
static inline int ct_show_secctx(struct seq_file *s, const struct nf_conn *ct)
@@ -141,10 +141,11 @@ static int ct_seq_show(struct seq_file *s, void *v)
NF_CT_ASSERT(l4proto);
ret = -ENOSPC;
- if (seq_printf(s, "%-8s %u %ld ",
- l4proto->name, nf_ct_protonum(ct),
- timer_pending(&ct->timeout)
- ? (long)(ct->timeout.expires - jiffies)/HZ : 0) != 0)
+ seq_printf(s, "%-8s %u %ld ",
+ l4proto->name, nf_ct_protonum(ct),
+ timer_pending(&ct->timeout)
+ ? (long)(ct->timeout.expires - jiffies)/HZ : 0);
+ if (seq_overflow(s))
goto release;
if (l4proto->print_conntrack && l4proto->print_conntrack(s, ct))
@@ -154,35 +155,44 @@ static int ct_seq_show(struct seq_file *s, void *v)
l3proto, l4proto))
goto release;
- if (seq_print_acct(s, ct, IP_CT_DIR_ORIGINAL))
+ seq_print_acct(s, ct, IP_CT_DIR_ORIGINAL);
+ if (seq_overflow(s))
goto release;
- if (!(test_bit(IPS_SEEN_REPLY_BIT, &ct->status)))
- if (seq_printf(s, "[UNREPLIED] "))
+ if (!(test_bit(IPS_SEEN_REPLY_BIT, &ct->status))) {
+ seq_printf(s, "[UNREPLIED] ");
+ if (seq_overflow(s))
goto release;
+ }
if (print_tuple(s, &ct->tuplehash[IP_CT_DIR_REPLY].tuple,
l3proto, l4proto))
goto release;
- if (seq_print_acct(s, ct, IP_CT_DIR_REPLY))
+ seq_print_acct(s, ct, IP_CT_DIR_REPLY);
+ if (seq_overflow(s))
goto release;
- if (test_bit(IPS_ASSURED_BIT, &ct->status))
- if (seq_printf(s, "[ASSURED] "))
+ if (test_bit(IPS_ASSURED_BIT, &ct->status)) {
+ seq_printf(s, "[ASSURED] ");
+ if (seq_overflow(s))
goto release;
+ }
#ifdef CONFIG_NF_CONNTRACK_MARK
- if (seq_printf(s, "mark=%u ", ct->mark))
+ seq_printf(s, "mark=%u ", ct->mark);
+ if (seq_overflow(s))
goto release;
#endif
if (ct_show_secctx(s, ct))
goto release;
- if (seq_printf(s, "use=%u\n", atomic_read(&ct->ct_general.use)))
+ seq_printf(s, "use=%u\n", atomic_read(&ct->ct_general.use));
+ if (seq_overflow(s))
goto release;
ret = 0;
+
release:
nf_ct_put(ct);
return ret;
@@ -297,7 +307,9 @@ static int exp_seq_show(struct seq_file *s, void *v)
__nf_ct_l3proto_find(exp->tuple.src.l3num),
__nf_ct_l4proto_find(exp->tuple.src.l3num,
exp->tuple.dst.protonum));
- return seq_putc(s, '\n');
+ seq_putc(s, '\n');
+
+ return seq_overflow(s);
}
static const struct seq_operations exp_seq_ops = {
diff --git a/net/ipv6/netfilter/nf_conntrack_l3proto_ipv6.c b/net/ipv6/netfilter/nf_conntrack_l3proto_ipv6.c
index 4cbc6b2..d5b0cd4 100644
--- a/net/ipv6/netfilter/nf_conntrack_l3proto_ipv6.c
+++ b/net/ipv6/netfilter/nf_conntrack_l3proto_ipv6.c
@@ -63,8 +63,10 @@ static bool ipv6_invert_tuple(struct nf_conntrack_tuple *tuple,
static int ipv6_print_tuple(struct seq_file *s,
const struct nf_conntrack_tuple *tuple)
{
- return seq_printf(s, "src=%pI6 dst=%pI6 ",
- tuple->src.u3.ip6, tuple->dst.u3.ip6);
+ seq_printf(s, "src=%pI6 dst=%pI6 ",
+ tuple->src.u3.ip6, tuple->dst.u3.ip6);
+
+ return seq_overflow(s);
}
static int ipv6_get_l4proto(const struct sk_buff *skb, unsigned int nhoff,
diff --git a/net/ipv6/netfilter/nf_conntrack_proto_icmpv6.c b/net/ipv6/netfilter/nf_conntrack_proto_icmpv6.c
index b3807c5..4d6d138 100644
--- a/net/ipv6/netfilter/nf_conntrack_proto_icmpv6.c
+++ b/net/ipv6/netfilter/nf_conntrack_proto_icmpv6.c
@@ -87,10 +87,12 @@ static bool icmpv6_invert_tuple(struct nf_conntrack_tuple *tuple,
static int icmpv6_print_tuple(struct seq_file *s,
const struct nf_conntrack_tuple *tuple)
{
- return seq_printf(s, "type=%u code=%u id=%u ",
- tuple->dst.u.icmp.type,
- tuple->dst.u.icmp.code,
- ntohs(tuple->src.u.icmp.id));
+ seq_printf(s, "type=%u code=%u id=%u ",
+ tuple->dst.u.icmp.type,
+ tuple->dst.u.icmp.code,
+ ntohs(tuple->src.u.icmp.id));
+
+ return seq_overflow(s);
}
static unsigned int *icmpv6_get_timeouts(struct net *net)
diff --git a/net/netfilter/nf_conntrack_acct.c b/net/netfilter/nf_conntrack_acct.c
index a4b5e2a..897d9cc 100644
--- a/net/netfilter/nf_conntrack_acct.c
+++ b/net/netfilter/nf_conntrack_acct.c
@@ -36,8 +36,7 @@ static struct ctl_table acct_sysctl_table[] = {
};
#endif /* CONFIG_SYSCTL */
-unsigned int
-seq_print_acct(struct seq_file *s, const struct nf_conn *ct, int dir)
+int seq_print_acct(struct seq_file *s, const struct nf_conn *ct, int dir)
{
struct nf_conn_acct *acct;
struct nf_conn_counter *counter;
@@ -47,9 +46,11 @@ seq_print_acct(struct seq_file *s, const struct nf_conn *ct, int dir)
return 0;
counter = acct->counter;
- return seq_printf(s, "packets=%llu bytes=%llu ",
- (unsigned long long)atomic64_read(&counter[dir].packets),
- (unsigned long long)atomic64_read(&counter[dir].bytes));
+ seq_printf(s, "packets=%llu bytes=%llu ",
+ (unsigned long long)atomic64_read(&counter[dir].packets),
+ (unsigned long long)atomic64_read(&counter[dir].bytes));
+
+ return seq_overflow(s);
};
EXPORT_SYMBOL_GPL(seq_print_acct);
diff --git a/net/netfilter/nf_conntrack_expect.c b/net/netfilter/nf_conntrack_expect.c
index 4fd1ca9..15bbf36 100644
--- a/net/netfilter/nf_conntrack_expect.c
+++ b/net/netfilter/nf_conntrack_expect.c
@@ -544,7 +544,9 @@ static int exp_seq_show(struct seq_file *s, void *v)
helper->expect_policy[expect->class].name);
}
- return seq_putc(s, '\n');
+ seq_putc(s, '\n');
+
+ return seq_overflow(s);
}
static const struct seq_operations exp_seq_ops = {
diff --git a/net/netfilter/nf_conntrack_proto_dccp.c b/net/netfilter/nf_conntrack_proto_dccp.c
index a99b6c3..a0f95a6 100644
--- a/net/netfilter/nf_conntrack_proto_dccp.c
+++ b/net/netfilter/nf_conntrack_proto_dccp.c
@@ -621,14 +621,18 @@ out_invalid:
static int dccp_print_tuple(struct seq_file *s,
const struct nf_conntrack_tuple *tuple)
{
- return seq_printf(s, "sport=%hu dport=%hu ",
- ntohs(tuple->src.u.dccp.port),
- ntohs(tuple->dst.u.dccp.port));
+ seq_printf(s, "sport=%hu dport=%hu ",
+ ntohs(tuple->src.u.dccp.port),
+ ntohs(tuple->dst.u.dccp.port));
+
+ return seq_overflow(s);
}
static int dccp_print_conntrack(struct seq_file *s, struct nf_conn *ct)
{
- return seq_printf(s, "%s ", dccp_state_names[ct->proto.dccp.state]);
+ seq_printf(s, "%s ", dccp_state_names[ct->proto.dccp.state]);
+
+ return seq_overflow(s);
}
#if IS_ENABLED(CONFIG_NF_CT_NETLINK)
diff --git a/net/netfilter/nf_conntrack_proto_gre.c b/net/netfilter/nf_conntrack_proto_gre.c
index 9d9c0da..234f7f6 100644
--- a/net/netfilter/nf_conntrack_proto_gre.c
+++ b/net/netfilter/nf_conntrack_proto_gre.c
@@ -230,17 +230,20 @@ static bool gre_pkt_to_tuple(const struct sk_buff *skb, unsigned int dataoff,
static int gre_print_tuple(struct seq_file *s,
const struct nf_conntrack_tuple *tuple)
{
- return seq_printf(s, "srckey=0x%x dstkey=0x%x ",
- ntohs(tuple->src.u.gre.key),
- ntohs(tuple->dst.u.gre.key));
+ seq_printf(s, "srckey=0x%x dstkey=0x%x ",
+ ntohs(tuple->src.u.gre.key), ntohs(tuple->dst.u.gre.key));
+
+ return seq_overflow(s);
}
/* print private data for conntrack */
static int gre_print_conntrack(struct seq_file *s, struct nf_conn *ct)
{
- return seq_printf(s, "timeout=%u, stream_timeout=%u ",
- (ct->proto.gre.timeout / HZ),
- (ct->proto.gre.stream_timeout / HZ));
+ seq_printf(s, "timeout=%u, stream_timeout=%u ",
+ ct->proto.gre.timeout / HZ,
+ ct->proto.gre.stream_timeout / HZ);
+
+ return seq_overflow(s);
}
static unsigned int *gre_get_timeouts(struct net *net)
diff --git a/net/netfilter/nf_conntrack_proto_sctp.c b/net/netfilter/nf_conntrack_proto_sctp.c
index 1314d33..b8b5708 100644
--- a/net/netfilter/nf_conntrack_proto_sctp.c
+++ b/net/netfilter/nf_conntrack_proto_sctp.c
@@ -169,9 +169,11 @@ static bool sctp_invert_tuple(struct nf_conntrack_tuple *tuple,
static int sctp_print_tuple(struct seq_file *s,
const struct nf_conntrack_tuple *tuple)
{
- return seq_printf(s, "sport=%hu dport=%hu ",
- ntohs(tuple->src.u.sctp.port),
- ntohs(tuple->dst.u.sctp.port));
+ seq_printf(s, "sport=%hu dport=%hu ",
+ ntohs(tuple->src.u.sctp.port),
+ ntohs(tuple->dst.u.sctp.port));
+
+ return seq_overflow(s);
}
/* Print out the private part of the conntrack. */
@@ -183,7 +185,9 @@ static int sctp_print_conntrack(struct seq_file *s, struct nf_conn *ct)
state = ct->proto.sctp.state;
spin_unlock_bh(&ct->lock);
- return seq_printf(s, "%s ", sctp_conntrack_names[state]);
+ seq_printf(s, "%s ", sctp_conntrack_names[state]);
+
+ return seq_overflow(s);
}
#define for_each_sctp_chunk(skb, sch, _sch, offset, dataoff, count) \
diff --git a/net/netfilter/nf_conntrack_proto_tcp.c b/net/netfilter/nf_conntrack_proto_tcp.c
index 44d1ea3..d21de09e 100644
--- a/net/netfilter/nf_conntrack_proto_tcp.c
+++ b/net/netfilter/nf_conntrack_proto_tcp.c
@@ -305,9 +305,10 @@ static bool tcp_invert_tuple(struct nf_conntrack_tuple *tuple,
static int tcp_print_tuple(struct seq_file *s,
const struct nf_conntrack_tuple *tuple)
{
- return seq_printf(s, "sport=%hu dport=%hu ",
- ntohs(tuple->src.u.tcp.port),
- ntohs(tuple->dst.u.tcp.port));
+ seq_printf(s, "sport=%hu dport=%hu ",
+ ntohs(tuple->src.u.tcp.port), ntohs(tuple->dst.u.tcp.port));
+
+ return seq_overflow(s);
}
/* Print out the private part of the conntrack. */
@@ -319,7 +320,9 @@ static int tcp_print_conntrack(struct seq_file *s, struct nf_conn *ct)
state = ct->proto.tcp.state;
spin_unlock_bh(&ct->lock);
- return seq_printf(s, "%s ", tcp_conntrack_names[state]);
+ seq_printf(s, "%s ", tcp_conntrack_names[state]);
+
+ return seq_overflow(s);
}
static unsigned int get_conntrack_index(const struct tcphdr *tcph)
diff --git a/net/netfilter/nf_conntrack_proto_udp.c b/net/netfilter/nf_conntrack_proto_udp.c
index 9d7721c..c2b52da 100644
--- a/net/netfilter/nf_conntrack_proto_udp.c
+++ b/net/netfilter/nf_conntrack_proto_udp.c
@@ -66,9 +66,10 @@ static bool udp_invert_tuple(struct nf_conntrack_tuple *tuple,
static int udp_print_tuple(struct seq_file *s,
const struct nf_conntrack_tuple *tuple)
{
- return seq_printf(s, "sport=%hu dport=%hu ",
- ntohs(tuple->src.u.udp.port),
- ntohs(tuple->dst.u.udp.port));
+ seq_printf(s, "sport=%hu dport=%hu ",
+ ntohs(tuple->src.u.udp.port), ntohs(tuple->dst.u.udp.port));
+
+ return seq_overflow(s);
}
static unsigned int *udp_get_timeouts(struct net *net)
diff --git a/net/netfilter/nf_conntrack_proto_udplite.c b/net/netfilter/nf_conntrack_proto_udplite.c
index 2750e6c..7c4fb9b 100644
--- a/net/netfilter/nf_conntrack_proto_udplite.c
+++ b/net/netfilter/nf_conntrack_proto_udplite.c
@@ -74,9 +74,10 @@ static bool udplite_invert_tuple(struct nf_conntrack_tuple *tuple,
static int udplite_print_tuple(struct seq_file *s,
const struct nf_conntrack_tuple *tuple)
{
- return seq_printf(s, "sport=%hu dport=%hu ",
- ntohs(tuple->src.u.udp.port),
- ntohs(tuple->dst.u.udp.port));
+ seq_printf(s, "sport=%hu dport=%hu ",
+ ntohs(tuple->src.u.udp.port), ntohs(tuple->dst.u.udp.port));
+
+ return seq_overflow(s);
}
static unsigned int *udplite_get_timeouts(struct net *net)
diff --git a/net/netfilter/nf_conntrack_standalone.c b/net/netfilter/nf_conntrack_standalone.c
index f641751..6666119 100644
--- a/net/netfilter/nf_conntrack_standalone.c
+++ b/net/netfilter/nf_conntrack_standalone.c
@@ -129,10 +129,10 @@ static int ct_show_secctx(struct seq_file *s, const struct nf_conn *ct)
if (ret)
return 0;
- ret = seq_printf(s, "secctx=%s ", secctx);
+ seq_printf(s, "secctx=%s ", secctx);
security_release_secctx(secctx, len);
- return ret;
+ return seq_overflow(s);
}
#else
static inline int ct_show_secctx(struct seq_file *s, const struct nf_conn *ct)
@@ -156,8 +156,9 @@ static int ct_show_delta_time(struct seq_file *s, const struct nf_conn *ct)
else
delta_time = 0;
- return seq_printf(s, "delta-time=%llu ",
- (unsigned long long)delta_time);
+ seq_printf(s, "delta-time=%llu ",
+ (unsigned long long)delta_time);
+ return seq_overflow(s);
}
return 0;
}
@@ -192,11 +193,12 @@ static int ct_seq_show(struct seq_file *s, void *v)
NF_CT_ASSERT(l4proto);
ret = -ENOSPC;
- if (seq_printf(s, "%-8s %u %-8s %u %ld ",
- l3proto->name, nf_ct_l3num(ct),
- l4proto->name, nf_ct_protonum(ct),
- timer_pending(&ct->timeout)
- ? (long)(ct->timeout.expires - jiffies)/HZ : 0) != 0)
+ seq_printf(s, "%-8s %u %-8s %u %ld ",
+ l3proto->name, nf_ct_l3num(ct),
+ l4proto->name, nf_ct_protonum(ct),
+ timer_pending(&ct->timeout)
+ ? (long)(ct->timeout.expires - jiffies) / HZ : 0);
+ if (seq_overflow(s))
goto release;
if (l4proto->print_conntrack && l4proto->print_conntrack(s, ct))
@@ -209,9 +211,11 @@ static int ct_seq_show(struct seq_file *s, void *v)
if (seq_print_acct(s, ct, IP_CT_DIR_ORIGINAL))
goto release;
- if (!(test_bit(IPS_SEEN_REPLY_BIT, &ct->status)))
- if (seq_printf(s, "[UNREPLIED] "))
+ if (!(test_bit(IPS_SEEN_REPLY_BIT, &ct->status))) {
+ seq_printf(s, "[UNREPLIED] ");
+ if (seq_overflow(s))
goto release;
+ }
if (print_tuple(s, &ct->tuplehash[IP_CT_DIR_REPLY].tuple,
l3proto, l4proto))
@@ -220,12 +224,15 @@ static int ct_seq_show(struct seq_file *s, void *v)
if (seq_print_acct(s, ct, IP_CT_DIR_REPLY))
goto release;
- if (test_bit(IPS_ASSURED_BIT, &ct->status))
- if (seq_printf(s, "[ASSURED] "))
+ if (test_bit(IPS_ASSURED_BIT, &ct->status)) {
+ seq_printf(s, "[ASSURED] ");
+ if (seq_overflow(s))
goto release;
+ }
#if defined(CONFIG_NF_CONNTRACK_MARK)
- if (seq_printf(s, "mark=%u ", ct->mark))
+ seq_printf(s, "mark=%u ", ct->mark);
+ if (seq_overflow(s))
goto release;
#endif
@@ -233,17 +240,20 @@ static int ct_seq_show(struct seq_file *s, void *v)
goto release;
#ifdef CONFIG_NF_CONNTRACK_ZONES
- if (seq_printf(s, "zone=%u ", nf_ct_zone(ct)))
+ seq_printf(s, "zone=%u ", nf_ct_zone(ct));
+ if (seq_overflow(s))
goto release;
#endif
if (ct_show_delta_time(s, ct))
goto release;
- if (seq_printf(s, "use=%u\n", atomic_read(&ct->ct_general.use)))
+ seq_printf(s, "use=%u\n", atomic_read(&ct->ct_general.use));
+ if (seq_overflow(s))
goto release;
- ret = 0;
+ return 0;
+
release:
nf_ct_put(ct);
return ret;
diff --git a/net/netfilter/nf_log.c b/net/netfilter/nf_log.c
index 85296d4..386327e 100644
--- a/net/netfilter/nf_log.c
+++ b/net/netfilter/nf_log.c
@@ -189,32 +189,32 @@ static int seq_show(struct seq_file *s, void *v)
loff_t *pos = v;
const struct nf_logger *logger;
struct nf_logger *t;
- int ret;
struct net *net = seq_file_net(s);
logger = rcu_dereference_protected(net->nf.nf_loggers[*pos],
lockdep_is_held(&nf_log_mutex));
if (!logger)
- ret = seq_printf(s, "%2lld NONE (", *pos);
+ seq_printf(s, "%2lld NONE (", *pos);
else
- ret = seq_printf(s, "%2lld %s (", *pos, logger->name);
-
- if (ret < 0)
- return ret;
+ seq_printf(s, "%2lld %s (", *pos, logger->name);
+ if (seq_overflow(s))
+ return seq_overflow(s);
list_for_each_entry(t, &nf_loggers_l[*pos], list[*pos]) {
- ret = seq_printf(s, "%s", t->name);
- if (ret < 0)
- return ret;
+ seq_printf(s, "%s", t->name);
+ if (seq_overflow(s))
+ return seq_overflow(s);
if (&t->list[*pos] != nf_loggers_l[*pos].prev) {
- ret = seq_printf(s, ",");
- if (ret < 0)
- return ret;
+ seq_printf(s, ",");
+ if (seq_overflow(s))
+ return seq_overflow(s);
}
}
- return seq_printf(s, ")\n");
+ seq_printf(s, ")\n");
+
+ return seq_overflow(s);
}
static const struct seq_operations nflog_seq_ops = {
diff --git a/net/netfilter/nfnetlink_log.c b/net/netfilter/nfnetlink_log.c
index 3c4b69e..99d4a6c 100644
--- a/net/netfilter/nfnetlink_log.c
+++ b/net/netfilter/nfnetlink_log.c
@@ -1001,11 +1001,13 @@ static int seq_show(struct seq_file *s, void *v)
{
const struct nfulnl_instance *inst = v;
- return seq_printf(s, "%5d %6d %5d %1d %5d %6d %2d\n",
- inst->group_num,
- inst->peer_portid, inst->qlen,
- inst->copy_mode, inst->copy_range,
- inst->flushtimeout, atomic_read(&inst->use));
+ seq_printf(s, "%5d %6d %5d %1d %5d %6d %2d\n",
+ inst->group_num,
+ inst->peer_portid, inst->qlen,
+ inst->copy_mode, inst->copy_range,
+ inst->flushtimeout, atomic_read(&inst->use));
+
+ return seq_overflow(s);
}
static const struct seq_operations nful_seq_ops = {
diff --git a/net/netfilter/nfnetlink_queue_core.c b/net/netfilter/nfnetlink_queue_core.c
index 21258cf..c3f4762 100644
--- a/net/netfilter/nfnetlink_queue_core.c
+++ b/net/netfilter/nfnetlink_queue_core.c
@@ -1253,12 +1253,14 @@ static int seq_show(struct seq_file *s, void *v)
{
const struct nfqnl_instance *inst = v;
- return seq_printf(s, "%5d %6d %5d %1d %5d %5d %5d %8d %2d\n",
- inst->queue_num,
- inst->peer_portid, inst->queue_total,
- inst->copy_mode, inst->copy_range,
- inst->queue_dropped, inst->queue_user_dropped,
- inst->id_sequence, 1);
+ seq_printf(s, "%5d %6d %5d %1d %5d %5d %5d %8d %2d\n",
+ inst->queue_num,
+ inst->peer_portid, inst->queue_total,
+ inst->copy_mode, inst->copy_range,
+ inst->queue_dropped, inst->queue_user_dropped,
+ inst->id_sequence, 1);
+
+ return seq_overflow(s);
}
static const struct seq_operations nfqnl_seq_ops = {
diff --git a/net/netfilter/x_tables.c b/net/netfilter/x_tables.c
index 227aa11..a74f49f 100644
--- a/net/netfilter/x_tables.c
+++ b/net/netfilter/x_tables.c
@@ -983,10 +983,12 @@ static int xt_table_seq_show(struct seq_file *seq, void *v)
{
struct xt_table *table = list_entry(v, struct xt_table, list);
- if (strlen(table->name))
- return seq_printf(seq, "%s\n", table->name);
- else
+ if (!strlen(table->name))
return 0;
+
+ seq_printf(seq, "%s\n", table->name);
+
+ return seq_overflow(seq);
}
static const struct seq_operations xt_table_seq_ops = {
diff --git a/net/netfilter/xt_hashlimit.c b/net/netfilter/xt_hashlimit.c
index a3910fc..c229655 100644
--- a/net/netfilter/xt_hashlimit.c
+++ b/net/netfilter/xt_hashlimit.c
@@ -797,25 +797,27 @@ static int dl_seq_real_show(struct dsthash_ent *ent, u_int8_t family,
switch (family) {
case NFPROTO_IPV4:
- res = seq_printf(s, "%ld %pI4:%u->%pI4:%u %u %u %u\n",
- (long)(ent->expires - jiffies)/HZ,
- &ent->dst.ip.src,
- ntohs(ent->dst.src_port),
- &ent->dst.ip.dst,
- ntohs(ent->dst.dst_port),
- ent->rateinfo.credit, ent->rateinfo.credit_cap,
- ent->rateinfo.cost);
+ seq_printf(s, "%ld %pI4:%u->%pI4:%u %u %u %u\n",
+ (long)(ent->expires - jiffies)/HZ,
+ &ent->dst.ip.src,
+ ntohs(ent->dst.src_port),
+ &ent->dst.ip.dst,
+ ntohs(ent->dst.dst_port),
+ ent->rateinfo.credit, ent->rateinfo.credit_cap,
+ ent->rateinfo.cost);
+ res = seq_overflow(s);
break;
#if IS_ENABLED(CONFIG_IP6_NF_IPTABLES)
case NFPROTO_IPV6:
- res = seq_printf(s, "%ld %pI6:%u->%pI6:%u %u %u %u\n",
- (long)(ent->expires - jiffies)/HZ,
- &ent->dst.ip6.src,
- ntohs(ent->dst.src_port),
- &ent->dst.ip6.dst,
- ntohs(ent->dst.dst_port),
- ent->rateinfo.credit, ent->rateinfo.credit_cap,
- ent->rateinfo.cost);
+ seq_printf(s, "%ld %pI6:%u->%pI6:%u %u %u %u\n",
+ (long)(ent->expires - jiffies)/HZ,
+ &ent->dst.ip6.src,
+ ntohs(ent->dst.src_port),
+ &ent->dst.ip6.dst,
+ ntohs(ent->dst.dst_port),
+ ent->rateinfo.credit, ent->rateinfo.credit_cap,
+ ent->rateinfo.cost);
+ res = seq_overflow(s);
break;
#endif
default:
David Miller
Dec 11, 2013, 12:30:02 AM12/11/13
to
From: Joe Perches <j...@perches.com>
Date: Tue, 10 Dec 2013 21:12:41 -0800
> Many uses of the return value of seq_printf/seq_puts/seq_putc are
> incorrect. Many assume that the return value is the number of
> chars emitted into a buffer like printf/puts/putc.
>
> It would be better to make the return value of these functions void
> to avoid these misuses.
>
> Start to do so.
> Convert seq_overflow to a public function from a static function.
>
> Remove the return uses of seq_printf/seq_puts/seq_putc from net.
> Add a seq_overflow function call instead.
I'm fine with this going in whatever tree is appropriate for
the seq_overflow un-static change:
Acked-by: David S. Miller <da...@davemloft.net>
Date: Tue, 10 Dec 2013 21:12:41 -0800
> Many uses of the return value of seq_printf/seq_puts/seq_putc are
> incorrect. Many assume that the return value is the number of
> chars emitted into a buffer like printf/puts/putc.
>
> It would be better to make the return value of these functions void
> to avoid these misuses.
>
> Start to do so.
> Convert seq_overflow to a public function from a static function.
>
> Remove the return uses of seq_printf/seq_puts/seq_putc from net.
> Add a seq_overflow function call instead.
the seq_overflow un-static change:
Acked-by: David S. Miller <da...@davemloft.net>
Antonio Quartulli
Dec 11, 2013, 2:40:01 AM12/11/13
to
On 11/12/13 06:12, Joe Perches wrote:
> Convert the uses of the return of seq_printf to
> instead check seq_overflow to determine if a buffer
> overflow has occurred.
>
> This will eventually allow seq_printf & seq_puts to
> be converted to a void return instead of the often
> misused return that is often assumed to be an int for
> the number of bytes emitted ala printk.
>
> Signed-off-by: Joe Perches <j...@perches.com>
I assume this patch is going to be merged with the others in some tree.
> Convert the uses of the return of seq_printf to
> instead check seq_overflow to determine if a buffer
> overflow has occurred.
>
> This will eventually allow seq_printf & seq_puts to
> be converted to a void return instead of the often
> misused return that is often assumed to be an int for
> the number of bytes emitted ala printk.
>
> Signed-off-by: Joe Perches <j...@perches.com>
In that case:
Acked-by: Antonio Quartulli <ant...@meshcoding.com>
Thanks,
--
Antonio Quartulli
Antonio Quartulli
Dec 11, 2013, 2:40:01 AM12/11/13
to
Joe,
we have other places in the batman-adv code where we use seq_printf, but
at the moment we don't check the return value and we always return 0 at
the end of the function.
I think we could use seq_overflow here as well?
Thanks,
--
Antonio Quartulli
we have other places in the batman-adv code where we use seq_printf, but
at the moment we don't check the return value and we always return 0 at
the end of the function.
I think we could use seq_overflow here as well?
Thanks,
--
Antonio Quartulli
Al Viro
Dec 11, 2013, 3:00:02 AM12/11/13
to
On Tue, Dec 10, 2013 at 09:12:43PM -0800, Joe Perches wrote:
> diff --git a/net/batman-adv/gateway_client.c b/net/batman-adv/gateway_client.c
> index 2449afa..dfa5d2d 100644
> --- a/net/batman-adv/gateway_client.c
> +++ b/net/batman-adv/gateway_client.c
> @@ -517,29 +517,28 @@ static int batadv_write_buffer_text(struct batadv_priv *bat_priv,
> {
> struct batadv_gw_node *curr_gw;
> struct batadv_neigh_node *router;
> - int ret = -1;
>
> router = batadv_orig_node_get_router(gw_node->orig_node);
> if (!router)
> - goto out;
> + return -1;
This (as well as the original) means "fail read(2) with -EINVAL", which
> diff --git a/net/batman-adv/gateway_client.c b/net/batman-adv/gateway_client.c
> index 2449afa..dfa5d2d 100644
> --- a/net/batman-adv/gateway_client.c
> +++ b/net/batman-adv/gateway_client.c
> @@ -517,29 +517,28 @@ static int batadv_write_buffer_text(struct batadv_priv *bat_priv,
> {
> struct batadv_gw_node *curr_gw;
> struct batadv_neigh_node *router;
> - int ret = -1;
>
> router = batadv_orig_node_get_router(gw_node->orig_node);
> if (!router)
> - goto out;
> + return -1;
might or might not be correct behaviour.
This sucker should return 0. Insufficiently large buffer will be handled
by caller, TYVM, if you give that caller a chance to do so. Returning 1
from ->show() is a bug in almost all cases, and definitely so in this one.
Just in case somebody decides that above is worth copying: It Is Not.
Original code is buggy, plain and simple. This one trades the older
bug ("fail with -EINVAL whenever the buffer is too small") with just as buggy
"silently skip an entry entirely whenever the buffer is too small".
Don't Do That.
Al Viro
Dec 11, 2013, 3:00:02 AM12/11/13
to
On Wed, Dec 11, 2013 at 08:31:35AM +0100, Antonio Quartulli wrote:
> Joe,
>
> we have other places in the batman-adv code where we use seq_printf, but
> at the moment we don't check the return value and we always return 0 at
> the end of the function.
>
> I think we could use seq_overflow here as well?
Not if you want correctly behaving code...
> Joe,
>
> we have other places in the batman-adv code where we use seq_printf, but
> at the moment we don't check the return value and we always return 0 at
> the end of the function.
>
> I think we could use seq_overflow here as well?
Al Viro
Dec 11, 2013, 3:10:01 AM12/11/13
to
On Wed, Dec 11, 2013 at 07:55:26AM +0000, Al Viro wrote:
> This sucker should return 0. Insufficiently large buffer will be handled
> by caller, TYVM, if you give that caller a chance to do so. Returning 1
> from ->show() is a bug in almost all cases, and definitely so in this one.
>
> Just in case somebody decides that above is worth copying: It Is Not.
> Original code is buggy, plain and simple. This one trades the older
> bug ("fail with -EINVAL whenever the buffer is too small") with just as buggy
> "silently skip an entry entirely whenever the buffer is too small".
>
> Don't Do That.
Pardon - Joe has made seq_overflow return -1 instead of true. Correction
> This sucker should return 0. Insufficiently large buffer will be handled
> by caller, TYVM, if you give that caller a chance to do so. Returning 1
> from ->show() is a bug in almost all cases, and definitely so in this one.
>
> Just in case somebody decides that above is worth copying: It Is Not.
> Original code is buggy, plain and simple. This one trades the older
> bug ("fail with -EINVAL whenever the buffer is too small") with just as buggy
> "silently skip an entry entirely whenever the buffer is too small".
>
> Don't Do That.
to the above, then - s/This trades.*\./This is just as buggy./
Conclusion is still the same - Don't Do That. Returning -1 on insufficiently
large buffer is a bug, plain and simple.
And this patch series is completely misguided - it doesn't fix any bugs
*and* it provides a misleading example for everyone. See the reaction
right in this thread, proposing to spread the same bug to currently
working iterators.
Al Viro
Dec 11, 2013, 3:20:01 AM12/11/13
to
On Tue, Dec 10, 2013 at 09:12:44PM -0800, Joe Perches wrote:
> --- a/net/ipv4/netfilter/nf_conntrack_l3proto_ipv4.c
> +++ b/net/ipv4/netfilter/nf_conntrack_l3proto_ipv4.c
> @@ -59,8 +59,10 @@ static bool ipv4_invert_tuple(struct nf_conntrack_tuple *tuple,
> static int ipv4_print_tuple(struct seq_file *s,
> const struct nf_conntrack_tuple *tuple)
> {
> - return seq_printf(s, "src=%pI4 dst=%pI4 ",
> - &tuple->src.u3.ip, &tuple->dst.u3.ip);
> + seq_printf(s, "src=%pI4 dst=%pI4 ",
> + &tuple->src.u3.ip, &tuple->dst.u3.ip);
> +
> + return seq_overflow(s);
> }
I'm not at all sure we want ->print_tuple() to return anything;
> --- a/net/ipv4/netfilter/nf_conntrack_l3proto_ipv4.c
> +++ b/net/ipv4/netfilter/nf_conntrack_l3proto_ipv4.c
> @@ -59,8 +59,10 @@ static bool ipv4_invert_tuple(struct nf_conntrack_tuple *tuple,
> static int ipv4_print_tuple(struct seq_file *s,
> const struct nf_conntrack_tuple *tuple)
> {
> - return seq_printf(s, "src=%pI4 dst=%pI4 ",
> - &tuple->src.u3.ip, &tuple->dst.u3.ip);
> + seq_printf(s, "src=%pI4 dst=%pI4 ",
> + &tuple->src.u3.ip, &tuple->dst.u3.ip);
> +
> + return seq_overflow(s);
> }
if we *really* want to check overflow and skip some expensive
output, we can bloody well do that in callers of print_tuple().
> @@ -104,10 +104,10 @@ static int ct_show_secctx(struct seq_file *s, const struct nf_conn *ct)
> if (ret)
> return 0;
>
> - ret = seq_printf(s, "secctx=%s ", secctx);
> + seq_printf(s, "secctx=%s ", secctx);
>
> security_release_secctx(secctx, len);
> - return ret;
> + return seq_overflow(s);
> }
In any case, returning non-zero in those cases is a bug.
> @@ -297,7 +307,9 @@ static int exp_seq_show(struct seq_file *s, void *v)
> __nf_ct_l3proto_find(exp->tuple.src.l3num),
> __nf_ct_l4proto_find(exp->tuple.src.l3num,
> exp->tuple.dst.protonum));
> - return seq_putc(s, '\n');
> + seq_putc(s, '\n');
> +
> + return seq_overflow(s);
> }
The rest is no better, AFAICS. Joe, you are doing it from the wrong end;
the right approach would be something like "make ->print_tuple() return
void", etc. As it is, you are just providing a lousy pattern to anybody
reading that. The last thing we want is people blindly copying these
bugs just because they'd seen a "proper" way of producing that kind of
broken behaviour. Monkey see, monkey do and all such...
Joe Perches
Dec 11, 2013, 3:30:01 AM12/11/13
to
On Wed, 2013-12-11 at 08:05 +0000, Al Viro wrote:
> On Wed, Dec 11, 2013 at 07:55:26AM +0000, Al Viro wrote:
>
> > This sucker should return 0. Insufficiently large buffer will be handled
> > by caller, TYVM, if you give that caller a chance to do so. Returning 1
> > from ->show() is a bug in almost all cases, and definitely so in this one.
> >
> > Just in case somebody decides that above is worth copying: It Is Not.
> > Original code is buggy, plain and simple. This one trades the older
> > bug ("fail with -EINVAL whenever the buffer is too small") with just as buggy
> > "silently skip an entry entirely whenever the buffer is too small".
> >
> > Don't Do That.
>
> Pardon - Joe has made seq_overflow return -1 instead of true. Correction
> to the above, then - s/This trades.*\./This is just as buggy./
Yeah, I started to use true/false, 0/1, but thought
> On Wed, Dec 11, 2013 at 07:55:26AM +0000, Al Viro wrote:
>
> > This sucker should return 0. Insufficiently large buffer will be handled
> > by caller, TYVM, if you give that caller a chance to do so. Returning 1
> > from ->show() is a bug in almost all cases, and definitely so in this one.
> >
> > Just in case somebody decides that above is worth copying: It Is Not.
> > Original code is buggy, plain and simple. This one trades the older
> > bug ("fail with -EINVAL whenever the buffer is too small") with just as buggy
> > "silently skip an entry entirely whenever the buffer is too small".
> >
> > Don't Do That.
>
> Pardon - Joe has made seq_overflow return -1 instead of true. Correction
> to the above, then - s/This trades.*\./This is just as buggy./
I needed to match what seq_printf/seq_vprintf does.
> Conclusion is still the same - Don't Do That. Returning -1 on insufficiently
> large buffer is a bug, plain and simple.
{
int len;
if (m->count < m->size) {
len = vsnprintf(m->buf + m->count, m->size - m->count, f, args);
if (m->count + len < m->size) {
m->count += len;
return 0;
}
}
seq_set_overflow(m);
return -1;
}
EXPORT_SYMBOL(seq_vprintf);
int seq_printf(struct seq_file *m, const char *f, ...)
{
int ret;
va_list args;
va_start(args, f);
ret = seq_vprintf(m, f, args);
va_end(args);
return ret;
}
EXPORT_SYMBOL(seq_printf);
> And this patch series is completely misguided - it doesn't fix any bugs
> *and* it provides a misleading example for everyone. See the reaction
> right in this thread, proposing to spread the same bug to currently
> working iterators.
You prefer this?
bool seq_overflow(struct seq_file *seq)
{
return m->count == m->size;
}
}
Al Viro
Dec 11, 2013, 3:40:02 AM12/11/13
to
where we return seq_printf/seq_puts/seq_putc return value from ->show()).
All such places should return 0. Then we need to look at the remaining
places that check return value of seq_printf() et.al. And decide whether
the callers really care about it.
Theoretically, there is a legitimate case when we want to look at that
return value. Namely,
seq_print(...)
if (!overflowed)
do tons of expensive calculations
generate more output
return 0
That is the reason why those guys hadn't been returning void to start with.
And yes, it was inviting bugs with ->show() returning -1 on overflows.
Bad API design, plain and simple.
I'm not sure we actually have any instances of that legitimate case, TBH.
_IF_ we do, we ought to expose seq_overflow() (with saner name - this one
invites the same "it's an error, need to report it" kind of bugs) and use
it in such places. But that needs to be decided on per-caller basis. And
I'd expect that there would be few enough such places after we kill the
obvious bugs.
Ryan Mallon
Dec 11, 2013, 6:30:02 PM12/11/13
to
it return int encourages people to do:
return seq_overflow(s);
When used in seq_file functions will return -EPERM (-1) to user-space,
which is confusing. It should probably return bool and let the caller
sort out the correct error to return.
~Ryan
Joe Perches
Dec 11, 2013, 6:40:01 PM12/11/13
to
Anyway, the patch series is out of date (see Al's comments).
I'll update it later unless you or someone else does it first.
0 new messages