* [PATCH nf-next 0/7] xtables: use dedicated copy_to_user helpers
@ 2017-01-02 22:19 Willem de Bruijn
2017-01-02 22:19 ` [PATCH nf-next 1/7] xtables: add xt_match, xt_target and data copy_to_user functions Willem de Bruijn
` (7 more replies)
0 siblings, 8 replies; 9+ messages in thread
From: Willem de Bruijn @ 2017-01-02 22:19 UTC (permalink / raw)
To: netfilter-devel; +Cc: netdev, davem, fw, dborkman, pablo, Willem de Bruijn
From: Willem de Bruijn <willemb@google.com>
xtables list and save interfaces share xt_match and xt_target state
with userspace. The kernel and userspace definitions of these structs
differ. Currently, the structs are copied wholesale, then patched up.
The match and target structs contain a kernel pointer. Type-specific
data may contain additional kernel-only state.
Introduce xt_match_to_user and xt_target_to_user helper functions to
copy only fields intended to be shared with userspace.
Introduce xt_data_to_user to do the same for type-specific state. Add
a field .usersize to xt_match and xt_target to define the range of
bytes in .matchsize that should be shared with userspace. All matches
and targets that define kernel-only data store this at the tail of
their struct.
Tested:
Ran iptables-test.py from iptables.git, with both a 64-bit and
32-bit compat binary. 603/603 tests passed both before and after
the patches (out of 705, but some CONFIGs were not enabled).
Also ran the following example queries manually, again using 64-bit
and 32-bit compat paths:
iptables -A INPUT -m string --algo bm --string 'xxx' -j LOG
iptables -L
iptables-save
ip6tables -A INPUT -m string --algo bm --string 'xxx' -j LOG
ip6tables -L
ip6tables-save
ebtables -A INPUT --limit 3 -j ACCEPT
ebtables -L
arptables -A INPUT --source-mac 00:11:22:33:44:55 -j ACCEPT
arptables -L
An instrumented binary that initializes its buffer with 0x66 bytes
shows the result of the patchset.
iptables LOG target in hex before and after. The xt_target struct
only has its size, name and revision specified. Trailing bytes in
the name field are not zeroed:
40 00 4c 4f 47 00 00 00
40 e1 0a a0 ff ff ff ff
00 00 00 00 00 00 00 00
00 00 00 00 00 00 00 00
04 00 00 00 00 00 00 00
00 00 00 00 00 00 00 00
00 00 00 00 00 00 00 00
00 00 00 00 00 00 00 00
40 00 4c 4f 47 00 66 66
66 66 66 66 66 66 66 66
66 66 66 66 66 66 66 66
66 66 66 66 66 66 66 00
04 00 00 00 00 00 00 00
00 00 00 00 00 00 00 00
00 00 00 00 00 00 00 00
00 00 00 00 00 00 00 00
ebtables limit match in hex before and after. Only the avg and burst
fields of ebt_limit_info are shared.
6c 69 6d 69 74 00 00 00
00 00 00 00 00 00 00 00
00 00 00 00 00 00 00 00
00 00 00 00 00 00 00 00
20 00 00 00 00 00 00 00
05 0d 00 00 05 00 00 00
66 de fc ff 00 00 00 00
50 d0 00 00 50 d0 00 00
a9 29 00 00 00 00 00 00
6c 69 6d 69 74 00 00 00
00 00 00 00 00 00 00 00
00 00 00 00 00 00 00 00
00 00 00 00 00 00 00 00
20 00 00 00 00 00 00 00
05 0d 00 00 05 00 00 00
00 00 00 00 00 00 00 00
00 00 00 00 00 00 00 00
00 00 00 00 00 00 00 00
Willem de Bruijn (7):
xtables: add xt_match, xt_target and data copy_to_user functions
iptables: use match, target and data copy_to_user helpers
ip6tables: use match, target and data copy_to_user helpers
arptables: use match, target and data copy_to_user helpers
ebtables: use match, target and data copy_to_user helpers
xtables: use match, target and data copy_to_user helpers in compat
xtables: extend matches and targets with .usersize
include/linux/netfilter/x_tables.h | 9 +++++
net/bridge/netfilter/ebt_limit.c | 1 +
net/bridge/netfilter/ebtables.c | 78 +++++++++++++++++++++++---------------
net/ipv4/netfilter/arp_tables.c | 15 +++-----
net/ipv4/netfilter/ip_tables.c | 21 +++-------
net/ipv4/netfilter/ipt_CLUSTERIP.c | 1 +
net/ipv6/netfilter/ip6_tables.c | 21 +++-------
net/ipv6/netfilter/ip6t_NPT.c | 2 +
net/netfilter/x_tables.c | 68 ++++++++++++++++++++++++++++-----
net/netfilter/xt_CT.c | 3 ++
net/netfilter/xt_RATEEST.c | 1 +
net/netfilter/xt_TEE.c | 2 +
net/netfilter/xt_bpf.c | 2 +
net/netfilter/xt_cgroup.c | 1 +
net/netfilter/xt_connlimit.c | 1 +
net/netfilter/xt_hashlimit.c | 4 ++
net/netfilter/xt_limit.c | 2 +
net/netfilter/xt_quota.c | 1 +
net/netfilter/xt_rateest.c | 1 +
net/netfilter/xt_string.c | 1 +
20 files changed, 154 insertions(+), 81 deletions(-)
--
2.8.0.rc3.226.g39d4020
^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH nf-next 1/7] xtables: add xt_match, xt_target and data copy_to_user functions
2017-01-02 22:19 [PATCH nf-next 0/7] xtables: use dedicated copy_to_user helpers Willem de Bruijn
@ 2017-01-02 22:19 ` Willem de Bruijn
2017-01-02 22:19 ` [PATCH nf-next 2/7] iptables: use match, target and data copy_to_user helpers Willem de Bruijn
` (6 subsequent siblings)
7 siblings, 0 replies; 9+ messages in thread
From: Willem de Bruijn @ 2017-01-02 22:19 UTC (permalink / raw)
To: netfilter-devel; +Cc: netdev, davem, fw, dborkman, pablo, Willem de Bruijn
From: Willem de Bruijn <willemb@google.com>
xt_entry_target, xt_entry_match and their private data may contain
kernel data.
Introduce helper functions xt_match_to_user, xt_target_to_user and
xt_data_to_user that copy only the expected fields. These replace
existing logic that calls copy_to_user on entire structs, then
overwrites select fields.
Private data is defined in xt_match and xt_target. All matches and
targets that maintain kernel data store this at the tail of their
private structure. Extend xt_match and xt_target with .usersize to
limit how many bytes of data are copied. The remainder is cleared.
If compatsize is specified, usersize can only safely be used if all
fields up to usersize use platform-independent types. Otherwise, the
compat_to_user callback must be defined.
This patch does not yet enable the support logic.
Signed-off-by: Willem de Bruijn <willemb@google.com>
---
include/linux/netfilter/x_tables.h | 9 +++++++
net/netfilter/x_tables.c | 54 ++++++++++++++++++++++++++++++++++++++
2 files changed, 63 insertions(+)
diff --git a/include/linux/netfilter/x_tables.h b/include/linux/netfilter/x_tables.h
index 5117e4d..be378cf 100644
--- a/include/linux/netfilter/x_tables.h
+++ b/include/linux/netfilter/x_tables.h
@@ -167,6 +167,7 @@ struct xt_match {
const char *table;
unsigned int matchsize;
+ unsigned int usersize;
#ifdef CONFIG_COMPAT
unsigned int compatsize;
#endif
@@ -207,6 +208,7 @@ struct xt_target {
const char *table;
unsigned int targetsize;
+ unsigned int usersize;
#ifdef CONFIG_COMPAT
unsigned int compatsize;
#endif
@@ -287,6 +289,13 @@ int xt_check_match(struct xt_mtchk_param *, unsigned int size, u_int8_t proto,
int xt_check_target(struct xt_tgchk_param *, unsigned int size, u_int8_t proto,
bool inv_proto);
+int xt_match_to_user(const struct xt_entry_match *m,
+ struct xt_entry_match __user *u);
+int xt_target_to_user(const struct xt_entry_target *t,
+ struct xt_entry_target __user *u);
+int xt_data_to_user(void __user *dst, const void *src,
+ int usersize, int size);
+
void *xt_copy_counters_from_user(const void __user *user, unsigned int len,
struct xt_counters_info *info, bool compat);
diff --git a/net/netfilter/x_tables.c b/net/netfilter/x_tables.c
index 2ff4996..feccf52 100644
--- a/net/netfilter/x_tables.c
+++ b/net/netfilter/x_tables.c
@@ -262,6 +262,60 @@ struct xt_target *xt_request_find_target(u8 af, const char *name, u8 revision)
}
EXPORT_SYMBOL_GPL(xt_request_find_target);
+
+static int xt_obj_to_user(u16 __user *psize, u16 size,
+ void __user *pname, const char *name,
+ u8 __user *prev, u8 rev)
+{
+ if (put_user(size, psize))
+ return -EFAULT;
+ if (copy_to_user(pname, name, strlen(name) + 1))
+ return -EFAULT;
+ if (put_user(rev, prev))
+ return -EFAULT;
+
+ return 0;
+}
+
+#define XT_OBJ_TO_USER(U, K, TYPE, C_SIZE) \
+ xt_obj_to_user(&U->u.TYPE##_size, C_SIZE ? : K->u.TYPE##_size, \
+ U->u.user.name, K->u.kernel.TYPE->name, \
+ &U->u.user.revision, K->u.kernel.TYPE->revision)
+
+int xt_data_to_user(void __user *dst, const void *src,
+ int usersize, int size)
+{
+ usersize = usersize ? : size;
+ if (copy_to_user(dst, src, usersize))
+ return -EFAULT;
+ if (usersize != size && clear_user(dst + usersize, size - usersize))
+ return -EFAULT;
+
+ return 0;
+}
+EXPORT_SYMBOL_GPL(xt_data_to_user);
+
+#define XT_DATA_TO_USER(U, K, TYPE, C_SIZE) \
+ xt_data_to_user(U->data, K->data, \
+ K->u.kernel.TYPE->usersize, \
+ C_SIZE ? : K->u.kernel.TYPE->TYPE##size)
+
+int xt_match_to_user(const struct xt_entry_match *m,
+ struct xt_entry_match __user *u)
+{
+ return XT_OBJ_TO_USER(u, m, match, 0) ||
+ XT_DATA_TO_USER(u, m, match, 0);
+}
+EXPORT_SYMBOL_GPL(xt_match_to_user);
+
+int xt_target_to_user(const struct xt_entry_target *t,
+ struct xt_entry_target __user *u)
+{
+ return XT_OBJ_TO_USER(u, t, target, 0) ||
+ XT_DATA_TO_USER(u, t, target, 0);
+}
+EXPORT_SYMBOL_GPL(xt_target_to_user);
+
static int match_revfn(u8 af, const char *name, u8 revision, int *bestp)
{
const struct xt_match *m;
--
2.8.0.rc3.226.g39d4020
^ permalink raw reply related [flat|nested] 9+ messages in thread
* [PATCH nf-next 2/7] iptables: use match, target and data copy_to_user helpers
2017-01-02 22:19 [PATCH nf-next 0/7] xtables: use dedicated copy_to_user helpers Willem de Bruijn
2017-01-02 22:19 ` [PATCH nf-next 1/7] xtables: add xt_match, xt_target and data copy_to_user functions Willem de Bruijn
@ 2017-01-02 22:19 ` Willem de Bruijn
2017-01-02 22:19 ` [PATCH nf-next 3/7] ip6tables: " Willem de Bruijn
` (5 subsequent siblings)
7 siblings, 0 replies; 9+ messages in thread
From: Willem de Bruijn @ 2017-01-02 22:19 UTC (permalink / raw)
To: netfilter-devel; +Cc: netdev, davem, fw, dborkman, pablo, Willem de Bruijn
From: Willem de Bruijn <willemb@google.com>
Convert iptables to copying entries, matches and targets one by one,
using the xt_match_to_user and xt_target_to_user helper functions.
Signed-off-by: Willem de Bruijn <willemb@google.com>
---
net/ipv4/netfilter/ip_tables.c | 21 ++++++---------------
1 file changed, 6 insertions(+), 15 deletions(-)
diff --git a/net/ipv4/netfilter/ip_tables.c b/net/ipv4/netfilter/ip_tables.c
index 91656a1..384b857 100644
--- a/net/ipv4/netfilter/ip_tables.c
+++ b/net/ipv4/netfilter/ip_tables.c
@@ -826,10 +826,6 @@ copy_entries_to_user(unsigned int total_size,
return PTR_ERR(counters);
loc_cpu_entry = private->entries;
- if (copy_to_user(userptr, loc_cpu_entry, total_size) != 0) {
- ret = -EFAULT;
- goto free_counters;
- }
/* FIXME: use iterator macros --RR */
/* ... then go back and fix counters and names */
@@ -839,6 +835,10 @@ copy_entries_to_user(unsigned int total_size,
const struct xt_entry_target *t;
e = (struct ipt_entry *)(loc_cpu_entry + off);
+ if (copy_to_user(userptr + off, e, sizeof(*e))) {
+ ret = -EFAULT;
+ goto free_counters;
+ }
if (copy_to_user(userptr + off
+ offsetof(struct ipt_entry, counters),
&counters[num],
@@ -852,23 +852,14 @@ copy_entries_to_user(unsigned int total_size,
i += m->u.match_size) {
m = (void *)e + i;
- if (copy_to_user(userptr + off + i
- + offsetof(struct xt_entry_match,
- u.user.name),
- m->u.kernel.match->name,
- strlen(m->u.kernel.match->name)+1)
- != 0) {
+ if (xt_match_to_user(m, userptr + off + i)) {
ret = -EFAULT;
goto free_counters;
}
}
t = ipt_get_target_c(e);
- if (copy_to_user(userptr + off + e->target_offset
- + offsetof(struct xt_entry_target,
- u.user.name),
- t->u.kernel.target->name,
- strlen(t->u.kernel.target->name)+1) != 0) {
+ if (xt_target_to_user(t, userptr + off + e->target_offset)) {
ret = -EFAULT;
goto free_counters;
}
--
2.8.0.rc3.226.g39d4020
^ permalink raw reply related [flat|nested] 9+ messages in thread
* [PATCH nf-next 3/7] ip6tables: use match, target and data copy_to_user helpers
2017-01-02 22:19 [PATCH nf-next 0/7] xtables: use dedicated copy_to_user helpers Willem de Bruijn
2017-01-02 22:19 ` [PATCH nf-next 1/7] xtables: add xt_match, xt_target and data copy_to_user functions Willem de Bruijn
2017-01-02 22:19 ` [PATCH nf-next 2/7] iptables: use match, target and data copy_to_user helpers Willem de Bruijn
@ 2017-01-02 22:19 ` Willem de Bruijn
2017-01-02 22:19 ` [PATCH nf-next 4/7] arptables: " Willem de Bruijn
` (4 subsequent siblings)
7 siblings, 0 replies; 9+ messages in thread
From: Willem de Bruijn @ 2017-01-02 22:19 UTC (permalink / raw)
To: netfilter-devel; +Cc: netdev, davem, fw, dborkman, pablo, Willem de Bruijn
From: Willem de Bruijn <willemb@google.com>
Convert ip6tables to copying entries, matches and targets one by one,
using the xt_match_to_user and xt_target_to_user helper functions.
Signed-off-by: Willem de Bruijn <willemb@google.com>
---
net/ipv6/netfilter/ip6_tables.c | 21 ++++++---------------
1 file changed, 6 insertions(+), 15 deletions(-)
diff --git a/net/ipv6/netfilter/ip6_tables.c b/net/ipv6/netfilter/ip6_tables.c
index 25a022d..1e15c54 100644
--- a/net/ipv6/netfilter/ip6_tables.c
+++ b/net/ipv6/netfilter/ip6_tables.c
@@ -855,10 +855,6 @@ copy_entries_to_user(unsigned int total_size,
return PTR_ERR(counters);
loc_cpu_entry = private->entries;
- if (copy_to_user(userptr, loc_cpu_entry, total_size) != 0) {
- ret = -EFAULT;
- goto free_counters;
- }
/* FIXME: use iterator macros --RR */
/* ... then go back and fix counters and names */
@@ -868,6 +864,10 @@ copy_entries_to_user(unsigned int total_size,
const struct xt_entry_target *t;
e = (struct ip6t_entry *)(loc_cpu_entry + off);
+ if (copy_to_user(userptr + off, e, sizeof(*e))) {
+ ret = -EFAULT;
+ goto free_counters;
+ }
if (copy_to_user(userptr + off
+ offsetof(struct ip6t_entry, counters),
&counters[num],
@@ -881,23 +881,14 @@ copy_entries_to_user(unsigned int total_size,
i += m->u.match_size) {
m = (void *)e + i;
- if (copy_to_user(userptr + off + i
- + offsetof(struct xt_entry_match,
- u.user.name),
- m->u.kernel.match->name,
- strlen(m->u.kernel.match->name)+1)
- != 0) {
+ if (xt_match_to_user(m, userptr + off + i)) {
ret = -EFAULT;
goto free_counters;
}
}
t = ip6t_get_target_c(e);
- if (copy_to_user(userptr + off + e->target_offset
- + offsetof(struct xt_entry_target,
- u.user.name),
- t->u.kernel.target->name,
- strlen(t->u.kernel.target->name)+1) != 0) {
+ if (xt_target_to_user(t, userptr + off + e->target_offset)) {
ret = -EFAULT;
goto free_counters;
}
--
2.8.0.rc3.226.g39d4020
^ permalink raw reply related [flat|nested] 9+ messages in thread
* [PATCH nf-next 4/7] arptables: use match, target and data copy_to_user helpers
2017-01-02 22:19 [PATCH nf-next 0/7] xtables: use dedicated copy_to_user helpers Willem de Bruijn
` (2 preceding siblings ...)
2017-01-02 22:19 ` [PATCH nf-next 3/7] ip6tables: " Willem de Bruijn
@ 2017-01-02 22:19 ` Willem de Bruijn
2017-01-02 22:19 ` [PATCH nf-next 5/7] ebtables: " Willem de Bruijn
` (3 subsequent siblings)
7 siblings, 0 replies; 9+ messages in thread
From: Willem de Bruijn @ 2017-01-02 22:19 UTC (permalink / raw)
To: netfilter-devel; +Cc: netdev, davem, fw, dborkman, pablo, Willem de Bruijn
From: Willem de Bruijn <willemb@google.com>
Convert arptables to copying entries, matches and targets one by one,
using the xt_match_to_user and xt_target_to_user helper functions.
Signed-off-by: Willem de Bruijn <willemb@google.com>
---
net/ipv4/netfilter/arp_tables.c | 15 +++++----------
1 file changed, 5 insertions(+), 10 deletions(-)
diff --git a/net/ipv4/netfilter/arp_tables.c b/net/ipv4/netfilter/arp_tables.c
index a467e12..6241a81 100644
--- a/net/ipv4/netfilter/arp_tables.c
+++ b/net/ipv4/netfilter/arp_tables.c
@@ -677,11 +677,6 @@ static int copy_entries_to_user(unsigned int total_size,
return PTR_ERR(counters);
loc_cpu_entry = private->entries;
- /* ... then copy entire thing ... */
- if (copy_to_user(userptr, loc_cpu_entry, total_size) != 0) {
- ret = -EFAULT;
- goto free_counters;
- }
/* FIXME: use iterator macros --RR */
/* ... then go back and fix counters and names */
@@ -689,6 +684,10 @@ static int copy_entries_to_user(unsigned int total_size,
const struct xt_entry_target *t;
e = (struct arpt_entry *)(loc_cpu_entry + off);
+ if (copy_to_user(userptr + off, e, sizeof(*e))) {
+ ret = -EFAULT;
+ goto free_counters;
+ }
if (copy_to_user(userptr + off
+ offsetof(struct arpt_entry, counters),
&counters[num],
@@ -698,11 +697,7 @@ static int copy_entries_to_user(unsigned int total_size,
}
t = arpt_get_target_c(e);
- if (copy_to_user(userptr + off + e->target_offset
- + offsetof(struct xt_entry_target,
- u.user.name),
- t->u.kernel.target->name,
- strlen(t->u.kernel.target->name)+1) != 0) {
+ if (xt_target_to_user(t, userptr + off + e->target_offset)) {
ret = -EFAULT;
goto free_counters;
}
--
2.8.0.rc3.226.g39d4020
^ permalink raw reply related [flat|nested] 9+ messages in thread
* [PATCH nf-next 5/7] ebtables: use match, target and data copy_to_user helpers
2017-01-02 22:19 [PATCH nf-next 0/7] xtables: use dedicated copy_to_user helpers Willem de Bruijn
` (3 preceding siblings ...)
2017-01-02 22:19 ` [PATCH nf-next 4/7] arptables: " Willem de Bruijn
@ 2017-01-02 22:19 ` Willem de Bruijn
2017-01-02 22:19 ` [PATCH nf-next 6/7] xtables: use match, target and data copy_to_user helpers in compat Willem de Bruijn
` (2 subsequent siblings)
7 siblings, 0 replies; 9+ messages in thread
From: Willem de Bruijn @ 2017-01-02 22:19 UTC (permalink / raw)
To: netfilter-devel; +Cc: netdev, davem, fw, dborkman, pablo, Willem de Bruijn
From: Willem de Bruijn <willemb@google.com>
Convert ebtables to copying entries, matches and targets one by one.
The solution is analogous to that of generic xt_(match|target)_to_user
helpers, but is applied to different structs.
Convert existing helpers ebt_make_XXXname helpers that overwrite
fields of an already copy_to_user'd struct with ebt_XXX_to_user
helpers that copy all relevant fields of the struct from scratch.
Signed-off-by: Willem de Bruijn <willemb@google.com>
---
net/bridge/netfilter/ebtables.c | 78 +++++++++++++++++++++++++----------------
1 file changed, 47 insertions(+), 31 deletions(-)
diff --git a/net/bridge/netfilter/ebtables.c b/net/bridge/netfilter/ebtables.c
index 537e3d5..79b6991 100644
--- a/net/bridge/netfilter/ebtables.c
+++ b/net/bridge/netfilter/ebtables.c
@@ -1346,56 +1346,72 @@ static int update_counters(struct net *net, const void __user *user,
hlp.num_counters, user, len);
}
-static inline int ebt_make_matchname(const struct ebt_entry_match *m,
- const char *base, char __user *ubase)
+static inline int ebt_obj_to_user(char __user *um, const char *_name,
+ const char *data, int entrysize,
+ int usersize, int datasize)
{
- char __user *hlp = ubase + ((char *)m - base);
- char name[EBT_FUNCTION_MAXNAMELEN] = {};
+ char name[EBT_FUNCTION_MAXNAMELEN] = {0};
/* ebtables expects 32 bytes long names but xt_match names are 29 bytes
* long. Copy 29 bytes and fill remaining bytes with zeroes.
*/
- strlcpy(name, m->u.match->name, sizeof(name));
- if (copy_to_user(hlp, name, EBT_FUNCTION_MAXNAMELEN))
+ strlcpy(name, _name, sizeof(name));
+ if (copy_to_user(um, name, EBT_FUNCTION_MAXNAMELEN) ||
+ put_user(datasize, (int __user *)(um + EBT_FUNCTION_MAXNAMELEN)) ||
+ xt_data_to_user(um + entrysize, data, usersize, datasize))
return -EFAULT;
+
return 0;
}
-static inline int ebt_make_watchername(const struct ebt_entry_watcher *w,
- const char *base, char __user *ubase)
+static inline int ebt_match_to_user(const struct ebt_entry_match *m,
+ const char *base, char __user *ubase)
{
- char __user *hlp = ubase + ((char *)w - base);
- char name[EBT_FUNCTION_MAXNAMELEN] = {};
+ return ebt_obj_to_user(ubase + ((char *)m - base),
+ m->u.match->name, m->data, sizeof(*m),
+ m->u.match->usersize, m->match_size);
+}
- strlcpy(name, w->u.watcher->name, sizeof(name));
- if (copy_to_user(hlp, name, EBT_FUNCTION_MAXNAMELEN))
- return -EFAULT;
- return 0;
+static inline int ebt_watcher_to_user(const struct ebt_entry_watcher *w,
+ const char *base, char __user *ubase)
+{
+ return ebt_obj_to_user(ubase + ((char *)w - base),
+ w->u.watcher->name, w->data, sizeof(*w),
+ w->u.watcher->usersize, w->watcher_size);
}
-static inline int ebt_make_names(struct ebt_entry *e, const char *base,
- char __user *ubase)
+static inline int ebt_entry_to_user(struct ebt_entry *e, const char *base,
+ char __user *ubase)
{
int ret;
char __user *hlp;
const struct ebt_entry_target *t;
- char name[EBT_FUNCTION_MAXNAMELEN] = {};
- if (e->bitmask == 0)
+ if (e->bitmask == 0) {
+ /* special case !EBT_ENTRY_OR_ENTRIES */
+ if (copy_to_user(ubase + ((char *)e - base), e,
+ sizeof(struct ebt_entries)))
+ return -EFAULT;
return 0;
+ }
+
+ if (copy_to_user(ubase + ((char *)e - base), e, sizeof(*e)))
+ return -EFAULT;
hlp = ubase + (((char *)e + e->target_offset) - base);
t = (struct ebt_entry_target *)(((char *)e) + e->target_offset);
- ret = EBT_MATCH_ITERATE(e, ebt_make_matchname, base, ubase);
+ ret = EBT_MATCH_ITERATE(e, ebt_match_to_user, base, ubase);
if (ret != 0)
return ret;
- ret = EBT_WATCHER_ITERATE(e, ebt_make_watchername, base, ubase);
+ ret = EBT_WATCHER_ITERATE(e, ebt_watcher_to_user, base, ubase);
if (ret != 0)
return ret;
- strlcpy(name, t->u.target->name, sizeof(name));
- if (copy_to_user(hlp, name, EBT_FUNCTION_MAXNAMELEN))
- return -EFAULT;
+ ret = ebt_obj_to_user(hlp, t->u.target->name, t->data, sizeof(*t),
+ t->u.target->usersize, t->target_size);
+ if (ret != 0)
+ return ret;
+
return 0;
}
@@ -1475,13 +1491,9 @@ static int copy_everything_to_user(struct ebt_table *t, void __user *user,
if (ret)
return ret;
- if (copy_to_user(tmp.entries, entries, entries_size)) {
- BUGPRINT("Couldn't copy entries to userspace\n");
- return -EFAULT;
- }
/* set the match/watcher/target names right */
return EBT_ENTRY_ITERATE(entries, entries_size,
- ebt_make_names, entries, tmp.entries);
+ ebt_entry_to_user, entries, tmp.entries);
}
static int do_ebt_set_ctl(struct sock *sk,
@@ -1630,8 +1642,10 @@ static int compat_match_to_user(struct ebt_entry_match *m, void __user **dstptr,
if (match->compat_to_user) {
if (match->compat_to_user(cm->data, m->data))
return -EFAULT;
- } else if (copy_to_user(cm->data, m->data, msize))
+ } else {
+ if (xt_data_to_user(cm->data, m->data, match->usersize, msize))
return -EFAULT;
+ }
*size -= ebt_compat_entry_padsize() + off;
*dstptr = cm->data;
@@ -1657,8 +1671,10 @@ static int compat_target_to_user(struct ebt_entry_target *t,
if (target->compat_to_user) {
if (target->compat_to_user(cm->data, t->data))
return -EFAULT;
- } else if (copy_to_user(cm->data, t->data, tsize))
- return -EFAULT;
+ } else {
+ if (xt_data_to_user(cm->data, t->data, target->usersize, tsize))
+ return -EFAULT;
+ }
*size -= ebt_compat_entry_padsize() + off;
*dstptr = cm->data;
--
2.8.0.rc3.226.g39d4020
^ permalink raw reply related [flat|nested] 9+ messages in thread
* [PATCH nf-next 6/7] xtables: use match, target and data copy_to_user helpers in compat
2017-01-02 22:19 [PATCH nf-next 0/7] xtables: use dedicated copy_to_user helpers Willem de Bruijn
` (4 preceding siblings ...)
2017-01-02 22:19 ` [PATCH nf-next 5/7] ebtables: " Willem de Bruijn
@ 2017-01-02 22:19 ` Willem de Bruijn
2017-01-02 22:19 ` [PATCH nf-next 7/7] xtables: extend matches and targets with .usersize Willem de Bruijn
2017-01-09 16:30 ` [PATCH nf-next 0/7] xtables: use dedicated copy_to_user helpers Pablo Neira Ayuso
7 siblings, 0 replies; 9+ messages in thread
From: Willem de Bruijn @ 2017-01-02 22:19 UTC (permalink / raw)
To: netfilter-devel; +Cc: netdev, davem, fw, dborkman, pablo, Willem de Bruijn
From: Willem de Bruijn <willemb@google.com>
Convert compat to copying entries, matches and targets one by one,
using the xt_match_to_user and xt_target_to_user helper functions.
Signed-off-by: Willem de Bruijn <willemb@google.com>
---
net/netfilter/x_tables.c | 14 ++++----------
1 file changed, 4 insertions(+), 10 deletions(-)
diff --git a/net/netfilter/x_tables.c b/net/netfilter/x_tables.c
index feccf52..016db6b 100644
--- a/net/netfilter/x_tables.c
+++ b/net/netfilter/x_tables.c
@@ -619,17 +619,14 @@ int xt_compat_match_to_user(const struct xt_entry_match *m,
int off = xt_compat_match_offset(match);
u_int16_t msize = m->u.user.match_size - off;
- if (copy_to_user(cm, m, sizeof(*cm)) ||
- put_user(msize, &cm->u.user.match_size) ||
- copy_to_user(cm->u.user.name, m->u.kernel.match->name,
- strlen(m->u.kernel.match->name) + 1))
+ if (XT_OBJ_TO_USER(cm, m, match, msize))
return -EFAULT;
if (match->compat_to_user) {
if (match->compat_to_user((void __user *)cm->data, m->data))
return -EFAULT;
} else {
- if (copy_to_user(cm->data, m->data, msize - sizeof(*cm)))
+ if (XT_DATA_TO_USER(cm, m, match, msize - sizeof(*cm)))
return -EFAULT;
}
@@ -977,17 +974,14 @@ int xt_compat_target_to_user(const struct xt_entry_target *t,
int off = xt_compat_target_offset(target);
u_int16_t tsize = t->u.user.target_size - off;
- if (copy_to_user(ct, t, sizeof(*ct)) ||
- put_user(tsize, &ct->u.user.target_size) ||
- copy_to_user(ct->u.user.name, t->u.kernel.target->name,
- strlen(t->u.kernel.target->name) + 1))
+ if (XT_OBJ_TO_USER(ct, t, target, tsize))
return -EFAULT;
if (target->compat_to_user) {
if (target->compat_to_user((void __user *)ct->data, t->data))
return -EFAULT;
} else {
- if (copy_to_user(ct->data, t->data, tsize - sizeof(*ct)))
+ if (XT_DATA_TO_USER(ct, t, target, tsize - sizeof(*ct)))
return -EFAULT;
}
--
2.8.0.rc3.226.g39d4020
^ permalink raw reply related [flat|nested] 9+ messages in thread
* [PATCH nf-next 7/7] xtables: extend matches and targets with .usersize
2017-01-02 22:19 [PATCH nf-next 0/7] xtables: use dedicated copy_to_user helpers Willem de Bruijn
` (5 preceding siblings ...)
2017-01-02 22:19 ` [PATCH nf-next 6/7] xtables: use match, target and data copy_to_user helpers in compat Willem de Bruijn
@ 2017-01-02 22:19 ` Willem de Bruijn
2017-01-09 16:30 ` [PATCH nf-next 0/7] xtables: use dedicated copy_to_user helpers Pablo Neira Ayuso
7 siblings, 0 replies; 9+ messages in thread
From: Willem de Bruijn @ 2017-01-02 22:19 UTC (permalink / raw)
To: netfilter-devel; +Cc: netdev, davem, fw, dborkman, pablo, Willem de Bruijn
From: Willem de Bruijn <willemb@google.com>
In matches and targets that define a kernel-only tail to their
xt_match and xt_target data structs, add a field .usersize that
specifies up to where data is to be shared with userspace.
Performed a search for comment "Used internally by the kernel" to find
relevant matches and targets. Manually inspected the structs to derive
a valid offsetof.
Signed-off-by: Willem de Bruijn <willemb@google.com>
---
net/bridge/netfilter/ebt_limit.c | 1 +
net/ipv4/netfilter/ipt_CLUSTERIP.c | 1 +
net/ipv6/netfilter/ip6t_NPT.c | 2 ++
net/netfilter/xt_CT.c | 3 +++
net/netfilter/xt_RATEEST.c | 1 +
net/netfilter/xt_TEE.c | 2 ++
net/netfilter/xt_bpf.c | 2 ++
net/netfilter/xt_cgroup.c | 1 +
net/netfilter/xt_connlimit.c | 1 +
net/netfilter/xt_hashlimit.c | 4 ++++
net/netfilter/xt_limit.c | 2 ++
net/netfilter/xt_quota.c | 1 +
net/netfilter/xt_rateest.c | 1 +
net/netfilter/xt_string.c | 1 +
14 files changed, 23 insertions(+)
diff --git a/net/bridge/netfilter/ebt_limit.c b/net/bridge/netfilter/ebt_limit.c
index 517e78b..61a9f1b 100644
--- a/net/bridge/netfilter/ebt_limit.c
+++ b/net/bridge/netfilter/ebt_limit.c
@@ -105,6 +105,7 @@ static struct xt_match ebt_limit_mt_reg __read_mostly = {
.match = ebt_limit_mt,
.checkentry = ebt_limit_mt_check,
.matchsize = sizeof(struct ebt_limit_info),
+ .usersize = offsetof(struct ebt_limit_info, prev),
#ifdef CONFIG_COMPAT
.compatsize = sizeof(struct ebt_compat_limit_info),
#endif
diff --git a/net/ipv4/netfilter/ipt_CLUSTERIP.c b/net/ipv4/netfilter/ipt_CLUSTERIP.c
index 21db00d..8a3d20e 100644
--- a/net/ipv4/netfilter/ipt_CLUSTERIP.c
+++ b/net/ipv4/netfilter/ipt_CLUSTERIP.c
@@ -468,6 +468,7 @@ static struct xt_target clusterip_tg_reg __read_mostly = {
.checkentry = clusterip_tg_check,
.destroy = clusterip_tg_destroy,
.targetsize = sizeof(struct ipt_clusterip_tgt_info),
+ .usersize = offsetof(struct ipt_clusterip_tgt_info, config),
#ifdef CONFIG_COMPAT
.compatsize = sizeof(struct compat_ipt_clusterip_tgt_info),
#endif /* CONFIG_COMPAT */
diff --git a/net/ipv6/netfilter/ip6t_NPT.c b/net/ipv6/netfilter/ip6t_NPT.c
index 590f767..a379d2f 100644
--- a/net/ipv6/netfilter/ip6t_NPT.c
+++ b/net/ipv6/netfilter/ip6t_NPT.c
@@ -112,6 +112,7 @@ static struct xt_target ip6t_npt_target_reg[] __read_mostly = {
.table = "mangle",
.target = ip6t_snpt_tg,
.targetsize = sizeof(struct ip6t_npt_tginfo),
+ .usersize = offsetof(struct ip6t_npt_tginfo, adjustment),
.checkentry = ip6t_npt_checkentry,
.family = NFPROTO_IPV6,
.hooks = (1 << NF_INET_LOCAL_IN) |
@@ -123,6 +124,7 @@ static struct xt_target ip6t_npt_target_reg[] __read_mostly = {
.table = "mangle",
.target = ip6t_dnpt_tg,
.targetsize = sizeof(struct ip6t_npt_tginfo),
+ .usersize = offsetof(struct ip6t_npt_tginfo, adjustment),
.checkentry = ip6t_npt_checkentry,
.family = NFPROTO_IPV6,
.hooks = (1 << NF_INET_PRE_ROUTING) |
diff --git a/net/netfilter/xt_CT.c b/net/netfilter/xt_CT.c
index 95c7503..26b0bccfa 100644
--- a/net/netfilter/xt_CT.c
+++ b/net/netfilter/xt_CT.c
@@ -373,6 +373,7 @@ static struct xt_target xt_ct_tg_reg[] __read_mostly = {
.name = "CT",
.family = NFPROTO_UNSPEC,
.targetsize = sizeof(struct xt_ct_target_info),
+ .usersize = offsetof(struct xt_ct_target_info, ct),
.checkentry = xt_ct_tg_check_v0,
.destroy = xt_ct_tg_destroy_v0,
.target = xt_ct_target_v0,
@@ -384,6 +385,7 @@ static struct xt_target xt_ct_tg_reg[] __read_mostly = {
.family = NFPROTO_UNSPEC,
.revision = 1,
.targetsize = sizeof(struct xt_ct_target_info_v1),
+ .usersize = offsetof(struct xt_ct_target_info, ct),
.checkentry = xt_ct_tg_check_v1,
.destroy = xt_ct_tg_destroy_v1,
.target = xt_ct_target_v1,
@@ -395,6 +397,7 @@ static struct xt_target xt_ct_tg_reg[] __read_mostly = {
.family = NFPROTO_UNSPEC,
.revision = 2,
.targetsize = sizeof(struct xt_ct_target_info_v1),
+ .usersize = offsetof(struct xt_ct_target_info, ct),
.checkentry = xt_ct_tg_check_v2,
.destroy = xt_ct_tg_destroy_v1,
.target = xt_ct_target_v1,
diff --git a/net/netfilter/xt_RATEEST.c b/net/netfilter/xt_RATEEST.c
index 91a373a..498b54f 100644
--- a/net/netfilter/xt_RATEEST.c
+++ b/net/netfilter/xt_RATEEST.c
@@ -162,6 +162,7 @@ static struct xt_target xt_rateest_tg_reg __read_mostly = {
.checkentry = xt_rateest_tg_checkentry,
.destroy = xt_rateest_tg_destroy,
.targetsize = sizeof(struct xt_rateest_target_info),
+ .usersize = offsetof(struct xt_rateest_target_info, est),
.me = THIS_MODULE,
};
diff --git a/net/netfilter/xt_TEE.c b/net/netfilter/xt_TEE.c
index 1c57ace..86b0580 100644
--- a/net/netfilter/xt_TEE.c
+++ b/net/netfilter/xt_TEE.c
@@ -133,6 +133,7 @@ static struct xt_target tee_tg_reg[] __read_mostly = {
.family = NFPROTO_IPV4,
.target = tee_tg4,
.targetsize = sizeof(struct xt_tee_tginfo),
+ .usersize = offsetof(struct xt_tee_tginfo, priv),
.checkentry = tee_tg_check,
.destroy = tee_tg_destroy,
.me = THIS_MODULE,
@@ -144,6 +145,7 @@ static struct xt_target tee_tg_reg[] __read_mostly = {
.family = NFPROTO_IPV6,
.target = tee_tg6,
.targetsize = sizeof(struct xt_tee_tginfo),
+ .usersize = offsetof(struct xt_tee_tginfo, priv),
.checkentry = tee_tg_check,
.destroy = tee_tg_destroy,
.me = THIS_MODULE,
diff --git a/net/netfilter/xt_bpf.c b/net/netfilter/xt_bpf.c
index 2dedaa2..38986a9 100644
--- a/net/netfilter/xt_bpf.c
+++ b/net/netfilter/xt_bpf.c
@@ -110,6 +110,7 @@ static struct xt_match bpf_mt_reg[] __read_mostly = {
.match = bpf_mt,
.destroy = bpf_mt_destroy,
.matchsize = sizeof(struct xt_bpf_info),
+ .usersize = offsetof(struct xt_bpf_info, filter),
.me = THIS_MODULE,
},
{
@@ -120,6 +121,7 @@ static struct xt_match bpf_mt_reg[] __read_mostly = {
.match = bpf_mt_v1,
.destroy = bpf_mt_destroy_v1,
.matchsize = sizeof(struct xt_bpf_info_v1),
+ .usersize = offsetof(struct xt_bpf_info_v1, filter),
.me = THIS_MODULE,
},
};
diff --git a/net/netfilter/xt_cgroup.c b/net/netfilter/xt_cgroup.c
index a086a91..1db1ce5 100644
--- a/net/netfilter/xt_cgroup.c
+++ b/net/netfilter/xt_cgroup.c
@@ -122,6 +122,7 @@ static struct xt_match cgroup_mt_reg[] __read_mostly = {
.checkentry = cgroup_mt_check_v1,
.match = cgroup_mt_v1,
.matchsize = sizeof(struct xt_cgroup_info_v1),
+ .usersize = offsetof(struct xt_cgroup_info_v1, priv),
.destroy = cgroup_mt_destroy_v1,
.me = THIS_MODULE,
.hooks = (1 << NF_INET_LOCAL_OUT) |
diff --git a/net/netfilter/xt_connlimit.c b/net/netfilter/xt_connlimit.c
index 2aff2b7..6d16f46 100644
--- a/net/netfilter/xt_connlimit.c
+++ b/net/netfilter/xt_connlimit.c
@@ -431,6 +431,7 @@ static struct xt_match connlimit_mt_reg __read_mostly = {
.checkentry = connlimit_mt_check,
.match = connlimit_mt,
.matchsize = sizeof(struct xt_connlimit_info),
+ .usersize = offsetof(struct xt_connlimit_info, data),
.destroy = connlimit_mt_destroy,
.me = THIS_MODULE,
};
diff --git a/net/netfilter/xt_hashlimit.c b/net/netfilter/xt_hashlimit.c
index 1006340..26ef70c 100644
--- a/net/netfilter/xt_hashlimit.c
+++ b/net/netfilter/xt_hashlimit.c
@@ -838,6 +838,7 @@ static struct xt_match hashlimit_mt_reg[] __read_mostly = {
.family = NFPROTO_IPV4,
.match = hashlimit_mt_v1,
.matchsize = sizeof(struct xt_hashlimit_mtinfo1),
+ .usersize = offsetof(struct xt_hashlimit_mtinfo1, hinfo),
.checkentry = hashlimit_mt_check_v1,
.destroy = hashlimit_mt_destroy_v1,
.me = THIS_MODULE,
@@ -848,6 +849,7 @@ static struct xt_match hashlimit_mt_reg[] __read_mostly = {
.family = NFPROTO_IPV4,
.match = hashlimit_mt,
.matchsize = sizeof(struct xt_hashlimit_mtinfo2),
+ .usersize = offsetof(struct xt_hashlimit_mtinfo2, hinfo),
.checkentry = hashlimit_mt_check,
.destroy = hashlimit_mt_destroy,
.me = THIS_MODULE,
@@ -859,6 +861,7 @@ static struct xt_match hashlimit_mt_reg[] __read_mostly = {
.family = NFPROTO_IPV6,
.match = hashlimit_mt_v1,
.matchsize = sizeof(struct xt_hashlimit_mtinfo1),
+ .usersize = offsetof(struct xt_hashlimit_mtinfo1, hinfo),
.checkentry = hashlimit_mt_check_v1,
.destroy = hashlimit_mt_destroy_v1,
.me = THIS_MODULE,
@@ -869,6 +872,7 @@ static struct xt_match hashlimit_mt_reg[] __read_mostly = {
.family = NFPROTO_IPV6,
.match = hashlimit_mt,
.matchsize = sizeof(struct xt_hashlimit_mtinfo2),
+ .usersize = offsetof(struct xt_hashlimit_mtinfo2, hinfo),
.checkentry = hashlimit_mt_check,
.destroy = hashlimit_mt_destroy,
.me = THIS_MODULE,
diff --git a/net/netfilter/xt_limit.c b/net/netfilter/xt_limit.c
index bef8505..dab962d 100644
--- a/net/netfilter/xt_limit.c
+++ b/net/netfilter/xt_limit.c
@@ -192,6 +192,8 @@ static struct xt_match limit_mt_reg __read_mostly = {
.compatsize = sizeof(struct compat_xt_rateinfo),
.compat_from_user = limit_mt_compat_from_user,
.compat_to_user = limit_mt_compat_to_user,
+#else
+ .usersize = offsetof(struct xt_rateinfo, prev),
#endif
.me = THIS_MODULE,
};
diff --git a/net/netfilter/xt_quota.c b/net/netfilter/xt_quota.c
index 44c8eb4..10d61a6 100644
--- a/net/netfilter/xt_quota.c
+++ b/net/netfilter/xt_quota.c
@@ -73,6 +73,7 @@ static struct xt_match quota_mt_reg __read_mostly = {
.checkentry = quota_mt_check,
.destroy = quota_mt_destroy,
.matchsize = sizeof(struct xt_quota_info),
+ .usersize = offsetof(struct xt_quota_info, master),
.me = THIS_MODULE,
};
diff --git a/net/netfilter/xt_rateest.c b/net/netfilter/xt_rateest.c
index 1db02f6..755d2f6 100644
--- a/net/netfilter/xt_rateest.c
+++ b/net/netfilter/xt_rateest.c
@@ -133,6 +133,7 @@ static struct xt_match xt_rateest_mt_reg __read_mostly = {
.checkentry = xt_rateest_mt_checkentry,
.destroy = xt_rateest_mt_destroy,
.matchsize = sizeof(struct xt_rateest_match_info),
+ .usersize = offsetof(struct xt_rateest_match_info, est1),
.me = THIS_MODULE,
};
diff --git a/net/netfilter/xt_string.c b/net/netfilter/xt_string.c
index 0bc3460..423293e 100644
--- a/net/netfilter/xt_string.c
+++ b/net/netfilter/xt_string.c
@@ -77,6 +77,7 @@ static struct xt_match xt_string_mt_reg __read_mostly = {
.match = string_mt,
.destroy = string_mt_destroy,
.matchsize = sizeof(struct xt_string_info),
+ .usersize = offsetof(struct xt_string_info, config),
.me = THIS_MODULE,
};
--
2.8.0.rc3.226.g39d4020
^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [PATCH nf-next 0/7] xtables: use dedicated copy_to_user helpers
2017-01-02 22:19 [PATCH nf-next 0/7] xtables: use dedicated copy_to_user helpers Willem de Bruijn
` (6 preceding siblings ...)
2017-01-02 22:19 ` [PATCH nf-next 7/7] xtables: extend matches and targets with .usersize Willem de Bruijn
@ 2017-01-09 16:30 ` Pablo Neira Ayuso
7 siblings, 0 replies; 9+ messages in thread
From: Pablo Neira Ayuso @ 2017-01-09 16:30 UTC (permalink / raw)
To: Willem de Bruijn
Cc: netfilter-devel, netdev, davem, fw, dborkman, Willem de Bruijn
On Mon, Jan 02, 2017 at 05:19:39PM -0500, Willem de Bruijn wrote:
> From: Willem de Bruijn <willemb@google.com>
>
> xtables list and save interfaces share xt_match and xt_target state
> with userspace. The kernel and userspace definitions of these structs
> differ. Currently, the structs are copied wholesale, then patched up.
> The match and target structs contain a kernel pointer. Type-specific
> data may contain additional kernel-only state.
>
> Introduce xt_match_to_user and xt_target_to_user helper functions to
> copy only fields intended to be shared with userspace.
>
> Introduce xt_data_to_user to do the same for type-specific state. Add
> a field .usersize to xt_match and xt_target to define the range of
> bytes in .matchsize that should be shared with userspace. All matches
> and targets that define kernel-only data store this at the tail of
> their struct.
Series applied, thanks a lot Willem!
^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2017-01-09 16:30 UTC | newest]
Thread overview: 9+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2017-01-02 22:19 [PATCH nf-next 0/7] xtables: use dedicated copy_to_user helpers Willem de Bruijn
2017-01-02 22:19 ` [PATCH nf-next 1/7] xtables: add xt_match, xt_target and data copy_to_user functions Willem de Bruijn
2017-01-02 22:19 ` [PATCH nf-next 2/7] iptables: use match, target and data copy_to_user helpers Willem de Bruijn
2017-01-02 22:19 ` [PATCH nf-next 3/7] ip6tables: " Willem de Bruijn
2017-01-02 22:19 ` [PATCH nf-next 4/7] arptables: " Willem de Bruijn
2017-01-02 22:19 ` [PATCH nf-next 5/7] ebtables: " Willem de Bruijn
2017-01-02 22:19 ` [PATCH nf-next 6/7] xtables: use match, target and data copy_to_user helpers in compat Willem de Bruijn
2017-01-02 22:19 ` [PATCH nf-next 7/7] xtables: extend matches and targets with .usersize Willem de Bruijn
2017-01-09 16:30 ` [PATCH nf-next 0/7] xtables: use dedicated copy_to_user helpers Pablo Neira Ayuso
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).