* [PATCH 1/3] Revert "netfilter: avoid get_random_bytes calls"
2014-01-08 19:13 [PATCH 0/3] Netfilter updates for net-next Pablo Neira Ayuso
@ 2014-01-08 19:13 ` Pablo Neira Ayuso
2014-01-08 19:13 ` [PATCH 2/3] netfilter: nf_conntrack_dccp: use %s format string for buffer Pablo Neira Ayuso
` (2 subsequent siblings)
3 siblings, 0 replies; 5+ messages in thread
From: Pablo Neira Ayuso @ 2014-01-08 19:13 UTC (permalink / raw)
To: netfilter-devel; +Cc: davem, netdev
This reverts commit a42b99a6e329654d376b330de057eff87686d890.
Hannes Frederic Sowa reported some problems with this patch, more specifically
that prandom_u32() may not be ready at boot time, see:
http://marc.info/?l=linux-netdev&m=138896532403533&w=2
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
---
net/netfilter/nfnetlink_log.c | 8 ++++++++
net/netfilter/nft_hash.c | 2 +-
net/netfilter/xt_RATEEST.c | 2 +-
net/netfilter/xt_connlimit.c | 2 +-
net/netfilter/xt_hashlimit.c | 2 +-
net/netfilter/xt_recent.c | 2 +-
6 files changed, 13 insertions(+), 5 deletions(-)
diff --git a/net/netfilter/nfnetlink_log.c b/net/netfilter/nfnetlink_log.c
index 7d4254b..3c4b69e 100644
--- a/net/netfilter/nfnetlink_log.c
+++ b/net/netfilter/nfnetlink_log.c
@@ -28,6 +28,8 @@
#include <linux/proc_fs.h>
#include <linux/security.h>
#include <linux/list.h>
+#include <linux/jhash.h>
+#include <linux/random.h>
#include <linux/slab.h>
#include <net/sock.h>
#include <net/netfilter/nf_log.h>
@@ -73,6 +75,7 @@ struct nfulnl_instance {
};
#define INSTANCE_BUCKETS 16
+static unsigned int hash_init;
static int nfnl_log_net_id __read_mostly;
@@ -1063,6 +1066,11 @@ static int __init nfnetlink_log_init(void)
{
int status = -ENOMEM;
+ /* it's not really all that important to have a random value, so
+ * we can do this from the init function, even if there hasn't
+ * been that much entropy yet */
+ get_random_bytes(&hash_init, sizeof(hash_init));
+
netlink_register_notifier(&nfulnl_rtnl_notifier);
status = nfnetlink_subsys_register(&nfulnl_subsys);
if (status < 0) {
diff --git a/net/netfilter/nft_hash.c b/net/netfilter/nft_hash.c
index 6aae699..3d3f8fc 100644
--- a/net/netfilter/nft_hash.c
+++ b/net/netfilter/nft_hash.c
@@ -164,7 +164,7 @@ static int nft_hash_init(const struct nft_set *set,
unsigned int cnt, i;
if (unlikely(!nft_hash_rnd_initted)) {
- nft_hash_rnd = prandom_u32();
+ get_random_bytes(&nft_hash_rnd, 4);
nft_hash_rnd_initted = true;
}
diff --git a/net/netfilter/xt_RATEEST.c b/net/netfilter/xt_RATEEST.c
index 190854b..370adf6 100644
--- a/net/netfilter/xt_RATEEST.c
+++ b/net/netfilter/xt_RATEEST.c
@@ -100,7 +100,7 @@ static int xt_rateest_tg_checkentry(const struct xt_tgchk_param *par)
int ret;
if (unlikely(!rnd_inited)) {
- jhash_rnd = prandom_u32();
+ get_random_bytes(&jhash_rnd, sizeof(jhash_rnd));
rnd_inited = true;
}
diff --git a/net/netfilter/xt_connlimit.c b/net/netfilter/xt_connlimit.c
index 7671e82..c40b269 100644
--- a/net/netfilter/xt_connlimit.c
+++ b/net/netfilter/xt_connlimit.c
@@ -229,7 +229,7 @@ static int connlimit_mt_check(const struct xt_mtchk_param *par)
u_int32_t rand;
do {
- rand = prandom_u32();
+ get_random_bytes(&rand, sizeof(rand));
} while (!rand);
cmpxchg(&connlimit_rnd, 0, rand);
}
diff --git a/net/netfilter/xt_hashlimit.c b/net/netfilter/xt_hashlimit.c
index d819f62..a3910fc 100644
--- a/net/netfilter/xt_hashlimit.c
+++ b/net/netfilter/xt_hashlimit.c
@@ -177,7 +177,7 @@ dsthash_alloc_init(struct xt_hashlimit_htable *ht,
/* initialize hash with random val at the time we allocate
* the first hashtable entry */
if (unlikely(!ht->rnd_initialized)) {
- ht->rnd = prandom_u32();
+ get_random_bytes(&ht->rnd, sizeof(ht->rnd));
ht->rnd_initialized = true;
}
diff --git a/net/netfilter/xt_recent.c b/net/netfilter/xt_recent.c
index bfdc29f..1e657cf 100644
--- a/net/netfilter/xt_recent.c
+++ b/net/netfilter/xt_recent.c
@@ -334,7 +334,7 @@ static int recent_mt_check(const struct xt_mtchk_param *par,
size_t sz;
if (unlikely(!hash_rnd_inited)) {
- hash_rnd = prandom_u32();
+ get_random_bytes(&hash_rnd, sizeof(hash_rnd));
hash_rnd_inited = true;
}
if (info->check_set & ~XT_RECENT_VALID_FLAGS) {
--
1.7.10.4
^ permalink raw reply related [flat|nested] 5+ messages in thread* [PATCH 2/3] netfilter: nf_conntrack_dccp: use %s format string for buffer
2014-01-08 19:13 [PATCH 0/3] Netfilter updates for net-next Pablo Neira Ayuso
2014-01-08 19:13 ` [PATCH 1/3] Revert "netfilter: avoid get_random_bytes calls" Pablo Neira Ayuso
@ 2014-01-08 19:13 ` Pablo Neira Ayuso
2014-01-08 19:13 ` [PATCH 3/3] netfilter: nf_conntrack_dccp: fix skb_header_pointer API usages Pablo Neira Ayuso
2014-01-08 20:05 ` [PATCH 0/3] Netfilter updates for net-next David Miller
3 siblings, 0 replies; 5+ messages in thread
From: Pablo Neira Ayuso @ 2014-01-08 19:13 UTC (permalink / raw)
To: netfilter-devel; +Cc: davem, netdev
From: Daniel Borkmann <dborkman@redhat.com>
Some invocations of nf_log_packet() use arg buffer directly instead of
"%s" format string with follow-up buffer pointer. Currently, these two
usages are not really critical, but we should fix this up nevertheless
so that we don't run into trouble if that changes one day.
Signed-off-by: Daniel Borkmann <dborkman@redhat.com>
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
---
net/netfilter/nf_conntrack_proto_dccp.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/net/netfilter/nf_conntrack_proto_dccp.c b/net/netfilter/nf_conntrack_proto_dccp.c
index a99b6c3..3841268 100644
--- a/net/netfilter/nf_conntrack_proto_dccp.c
+++ b/net/netfilter/nf_conntrack_proto_dccp.c
@@ -457,7 +457,7 @@ static bool dccp_new(struct nf_conn *ct, const struct sk_buff *skb,
out_invalid:
if (LOG_INVALID(net, IPPROTO_DCCP))
nf_log_packet(net, nf_ct_l3num(ct), 0, skb, NULL, NULL,
- NULL, msg);
+ NULL, "%s", msg);
return false;
}
@@ -614,7 +614,7 @@ static int dccp_error(struct net *net, struct nf_conn *tmpl,
out_invalid:
if (LOG_INVALID(net, IPPROTO_DCCP))
- nf_log_packet(net, pf, 0, skb, NULL, NULL, NULL, msg);
+ nf_log_packet(net, pf, 0, skb, NULL, NULL, NULL, "%s", msg);
return -NF_ACCEPT;
}
--
1.7.10.4
^ permalink raw reply related [flat|nested] 5+ messages in thread
* [PATCH 3/3] netfilter: nf_conntrack_dccp: fix skb_header_pointer API usages
2014-01-08 19:13 [PATCH 0/3] Netfilter updates for net-next Pablo Neira Ayuso
2014-01-08 19:13 ` [PATCH 1/3] Revert "netfilter: avoid get_random_bytes calls" Pablo Neira Ayuso
2014-01-08 19:13 ` [PATCH 2/3] netfilter: nf_conntrack_dccp: use %s format string for buffer Pablo Neira Ayuso
@ 2014-01-08 19:13 ` Pablo Neira Ayuso
2014-01-08 20:05 ` [PATCH 0/3] Netfilter updates for net-next David Miller
3 siblings, 0 replies; 5+ messages in thread
From: Pablo Neira Ayuso @ 2014-01-08 19:13 UTC (permalink / raw)
To: netfilter-devel; +Cc: davem, netdev
From: Daniel Borkmann <dborkman@redhat.com>
Some occurences in the netfilter tree use skb_header_pointer() in
the following way ...
struct dccp_hdr _dh, *dh;
...
skb_header_pointer(skb, dataoff, sizeof(_dh), &dh);
... where dh itself is a pointer that is being passed as the copy
buffer. Instead, we need to use &_dh as the forth argument so that
we're copying the data into an actual buffer that sits on the stack.
Currently, we probably could overwrite memory on the stack (e.g.
with a possibly mal-formed DCCP packet), but unintentionally, as
we only want the buffer to be placed into _dh variable.
Fixes: 2bc780499aa3 ("[NETFILTER]: nf_conntrack: add DCCP protocol support")
Signed-off-by: Daniel Borkmann <dborkman@redhat.com>
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
---
net/netfilter/nf_conntrack_proto_dccp.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/net/netfilter/nf_conntrack_proto_dccp.c b/net/netfilter/nf_conntrack_proto_dccp.c
index 3841268..cb372f9 100644
--- a/net/netfilter/nf_conntrack_proto_dccp.c
+++ b/net/netfilter/nf_conntrack_proto_dccp.c
@@ -428,7 +428,7 @@ static bool dccp_new(struct nf_conn *ct, const struct sk_buff *skb,
const char *msg;
u_int8_t state;
- dh = skb_header_pointer(skb, dataoff, sizeof(_dh), &dh);
+ dh = skb_header_pointer(skb, dataoff, sizeof(_dh), &_dh);
BUG_ON(dh == NULL);
state = dccp_state_table[CT_DCCP_ROLE_CLIENT][dh->dccph_type][CT_DCCP_NONE];
@@ -486,7 +486,7 @@ static int dccp_packet(struct nf_conn *ct, const struct sk_buff *skb,
u_int8_t type, old_state, new_state;
enum ct_dccp_roles role;
- dh = skb_header_pointer(skb, dataoff, sizeof(_dh), &dh);
+ dh = skb_header_pointer(skb, dataoff, sizeof(_dh), &_dh);
BUG_ON(dh == NULL);
type = dh->dccph_type;
@@ -577,7 +577,7 @@ static int dccp_error(struct net *net, struct nf_conn *tmpl,
unsigned int cscov;
const char *msg;
- dh = skb_header_pointer(skb, dataoff, sizeof(_dh), &dh);
+ dh = skb_header_pointer(skb, dataoff, sizeof(_dh), &_dh);
if (dh == NULL) {
msg = "nf_ct_dccp: short packet ";
goto out_invalid;
--
1.7.10.4
^ permalink raw reply related [flat|nested] 5+ messages in thread* Re: [PATCH 0/3] Netfilter updates for net-next
2014-01-08 19:13 [PATCH 0/3] Netfilter updates for net-next Pablo Neira Ayuso
` (2 preceding siblings ...)
2014-01-08 19:13 ` [PATCH 3/3] netfilter: nf_conntrack_dccp: fix skb_header_pointer API usages Pablo Neira Ayuso
@ 2014-01-08 20:05 ` David Miller
3 siblings, 0 replies; 5+ messages in thread
From: David Miller @ 2014-01-08 20:05 UTC (permalink / raw)
To: pablo; +Cc: netfilter-devel, netdev
From: Pablo Neira Ayuso <pablo@netfilter.org>
Date: Wed, 8 Jan 2014 20:13:21 +0100
> The following patchset contains three Netfilter updates, they are:
>
> * Fix wrong usage of skb_header_pointer in the DCCP protocol helper that
> has been there for quite some time. It was resulting in copying the dccp
> header to a pointer allocated in the stack. Fortunately, this pointer
> provides room for the dccp header is 4 bytes long, so no crashes have been
> reported so far. From Daniel Borkmann.
>
> * Use format string to print in the invocation of nf_log_packet(), again
> in the DCCP helper. Also from Daniel Borkmann.
>
> * Revert "netfilter: avoid get_random_bytes call" as prandom32 does not
> guarantee enough entropy when being calling this at boot time, that may
> happen when reloading the rule.
>
> You can pull these changes from:
>
> git://git.kernel.org/pub/scm/linux/kernel/git/pablo/nf-next.git master
Pulled, thanks Pablo.
^ permalink raw reply [flat|nested] 5+ messages in thread