* [PATCH net-next 1/4] net: Use sockptr_is_kernel() instead of testing is_kernel
2023-12-25 9:46 [PATCH net-next 0/4] sockptr: Change sockptr_t to be a struct David Laight
@ 2023-12-25 9:51 ` David Laight
2023-12-25 9:55 ` [PATCH net-next 2/4] bpf: Use bpfptr_is_kernel() instead of checking the is_kernel member David Laight
` (3 subsequent siblings)
4 siblings, 0 replies; 6+ messages in thread
From: David Laight @ 2023-12-25 9:51 UTC (permalink / raw)
To: 'netdev@vger.kernel.org', 'David S . Miller',
'kuba@kernel.org'
Cc: 'eric.dumazet@gmail.com', 'martin.lau@linux.dev',
'Alexei Starovoitov', 'Stephen Hemminger',
'Jens Axboe', 'Daniel Borkmann',
'Andrii Nakryiko'
Some changes to option handling directly accesses optval.is_kernel.
Use the sockptr_is_kernel() helper instead.
No functional change.
Signed-off-by: David Laight <david.laight@aculab.com>
---
net/ipv4/ip_sockglue.c | 2 +-
net/ipv6/ipv6_sockglue.c | 2 +-
net/socket.c | 2 +-
3 files changed, 3 insertions(+), 3 deletions(-)
diff --git a/net/ipv4/ip_sockglue.c b/net/ipv4/ip_sockglue.c
index 2efc53526a38..94b2f8c095f5 100644
--- a/net/ipv4/ip_sockglue.c
+++ b/net/ipv4/ip_sockglue.c
@@ -1647,7 +1647,7 @@ int do_ip_getsockopt(struct sock *sk, int level, int optname,
if (sk->sk_type != SOCK_STREAM)
return -ENOPROTOOPT;
- if (optval.is_kernel) {
+ if (sockptr_is_kernel(optval)) {
msg.msg_control_is_user = false;
msg.msg_control = optval.kernel;
} else {
diff --git a/net/ipv6/ipv6_sockglue.c b/net/ipv6/ipv6_sockglue.c
index 7d661735cb9d..64fc52d928c1 100644
--- a/net/ipv6/ipv6_sockglue.c
+++ b/net/ipv6/ipv6_sockglue.c
@@ -1144,7 +1144,7 @@ int do_ipv6_getsockopt(struct sock *sk, int level, int optname,
if (sk->sk_type != SOCK_STREAM)
return -ENOPROTOOPT;
- if (optval.is_kernel) {
+ if (sockptr_is_kernel(optval)) {
msg.msg_control_is_user = false;
msg.msg_control = optval.kernel;
} else {
diff --git a/net/socket.c b/net/socket.c
index 3379c64217a4..8821f083ab0a 100644
--- a/net/socket.c
+++ b/net/socket.c
@@ -2366,7 +2366,7 @@ int do_sock_getsockopt(struct socket *sock, bool compat, int level,
} else if (unlikely(!ops->getsockopt)) {
err = -EOPNOTSUPP;
} else {
- if (WARN_ONCE(optval.is_kernel || optlen.is_kernel,
+ if (WARN_ONCE(sockptr_is_kernel(optval) || sockptr_is_kernel(optlen),
"Invalid argument type"))
return -EOPNOTSUPP;
--
2.17.1
-
Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, UK
Registration No: 1397386 (Wales)
^ permalink raw reply related [flat|nested] 6+ messages in thread
* [PATCH net-next 2/4] bpf: Use bpfptr_is_kernel() instead of checking the is_kernel member.
2023-12-25 9:46 [PATCH net-next 0/4] sockptr: Change sockptr_t to be a struct David Laight
2023-12-25 9:51 ` [PATCH net-next 1/4] net: Use sockptr_is_kernel() instead of testing is_kernel David Laight
@ 2023-12-25 9:55 ` David Laight
2023-12-25 9:57 ` [PATCH net-next 3/4] bpf: Use the sockptr_t helpers David Laight
` (2 subsequent siblings)
4 siblings, 0 replies; 6+ messages in thread
From: David Laight @ 2023-12-25 9:55 UTC (permalink / raw)
To: 'netdev@vger.kernel.org', 'David S . Miller',
'kuba@kernel.org'
Cc: 'eric.dumazet@gmail.com', 'martin.lau@linux.dev',
'Alexei Starovoitov', 'Stephen Hemminger',
'Jens Axboe', 'Daniel Borkmann',
'Andrii Nakryiko'
In some places the bpf code directly access the is_kernel member of bpfptr_t.
Change to use the bpfptr_is_kernel() helper.
No functional change.
Signed-off-by: David Laight <david.laight@aculab.com>
---
I'm not at all sure that the pattern:
urecord = make_bpfptr(attr->func_info, bpfptr_is_kernel(uattr));
isn't bending the rules somewhat - but that is a different issue.
kernel/bpf/bpf_iter.c | 2 +-
kernel/bpf/btf.c | 2 +-
kernel/bpf/syscall.c | 12 ++++++------
kernel/bpf/verifier.c | 10 +++++-----
4 files changed, 13 insertions(+), 13 deletions(-)
diff --git a/kernel/bpf/bpf_iter.c b/kernel/bpf/bpf_iter.c
index 0fae79164187..eb2c858dbf81 100644
--- a/kernel/bpf/bpf_iter.c
+++ b/kernel/bpf/bpf_iter.c
@@ -520,7 +520,7 @@ int bpf_iter_link_attach(const union bpf_attr *attr, bpfptr_t uattr,
memset(&linfo, 0, sizeof(union bpf_iter_link_info));
- ulinfo = make_bpfptr(attr->link_create.iter_info, uattr.is_kernel);
+ ulinfo = make_bpfptr(attr->link_create.iter_info, bpfptr_is_kernel(uattr));
linfo_len = attr->link_create.iter_info_len;
if (bpfptr_is_null(ulinfo) ^ !linfo_len)
return -EINVAL;
diff --git a/kernel/bpf/btf.c b/kernel/bpf/btf.c
index 15d71d2986d3..34720a1f586e 100644
--- a/kernel/bpf/btf.c
+++ b/kernel/bpf/btf.c
@@ -5483,7 +5483,7 @@ static int finalize_log(struct bpf_verifier_log *log, bpfptr_t uattr, u32 uattr_
static struct btf *btf_parse(const union bpf_attr *attr, bpfptr_t uattr, u32 uattr_size)
{
- bpfptr_t btf_data = make_bpfptr(attr->btf, uattr.is_kernel);
+ bpfptr_t btf_data = make_bpfptr(attr->btf, bpfptr_is_kernel(uattr));
char __user *log_ubuf = u64_to_user_ptr(attr->btf_log_buf);
struct btf_struct_metas *struct_meta_tab;
struct btf_verifier_env *env = NULL;
diff --git a/kernel/bpf/syscall.c b/kernel/bpf/syscall.c
index 0ed286b8a0f0..ba59fa8d02db 100644
--- a/kernel/bpf/syscall.c
+++ b/kernel/bpf/syscall.c
@@ -93,7 +93,7 @@ int bpf_check_uarg_tail_zero(bpfptr_t uaddr,
if (actual_size <= expected_size)
return 0;
- if (uaddr.is_kernel)
+ if (bpfptr_is_kernel(uaddr))
res = memchr_inv(uaddr.kernel + expected_size, 0,
actual_size - expected_size) == NULL;
else
@@ -1482,8 +1482,8 @@ static int map_lookup_elem(union bpf_attr *attr)
static int map_update_elem(union bpf_attr *attr, bpfptr_t uattr)
{
- bpfptr_t ukey = make_bpfptr(attr->key, uattr.is_kernel);
- bpfptr_t uvalue = make_bpfptr(attr->value, uattr.is_kernel);
+ bpfptr_t ukey = make_bpfptr(attr->key, bpfptr_is_kernel(uattr));
+ bpfptr_t uvalue = make_bpfptr(attr->value, bpfptr_is_kernel(uattr));
int ufd = attr->map_fd;
struct bpf_map *map;
void *key, *value;
@@ -1538,7 +1538,7 @@ static int map_update_elem(union bpf_attr *attr, bpfptr_t uattr)
static int map_delete_elem(union bpf_attr *attr, bpfptr_t uattr)
{
- bpfptr_t ukey = make_bpfptr(attr->key, uattr.is_kernel);
+ bpfptr_t ukey = make_bpfptr(attr->key, bpfptr_is_kernel(uattr));
int ufd = attr->map_fd;
struct bpf_map *map;
struct fd f;
@@ -2670,12 +2670,12 @@ static int bpf_prog_load(union bpf_attr *attr, bpfptr_t uattr, u32 uattr_size)
err = -EFAULT;
if (copy_from_bpfptr(prog->insns,
- make_bpfptr(attr->insns, uattr.is_kernel),
+ make_bpfptr(attr->insns, bpfptr_is_kernel(uattr)),
bpf_prog_insn_size(prog)) != 0)
goto free_prog_sec;
/* copy eBPF program license from user space */
if (strncpy_from_bpfptr(license,
- make_bpfptr(attr->license, uattr.is_kernel),
+ make_bpfptr(attr->license, bpfptr_is_kernel(uattr)),
sizeof(license) - 1) < 0)
goto free_prog_sec;
license[sizeof(license) - 1] = 0;
diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
index af2819d5c8ee..42fea4966175 100644
--- a/kernel/bpf/verifier.c
+++ b/kernel/bpf/verifier.c
@@ -15838,7 +15838,7 @@ static int check_btf_func_early(struct bpf_verifier_env *env,
prog = env->prog;
btf = prog->aux->btf;
- urecord = make_bpfptr(attr->func_info, uattr.is_kernel);
+ urecord = make_bpfptr(attr->func_info, bpfptr_is_kernel(uattr));
min_size = min_t(u32, krec_size, urec_size);
krecord = kvcalloc(nfuncs, krec_size, GFP_KERNEL | __GFP_NOWARN);
@@ -15938,7 +15938,7 @@ static int check_btf_func(struct bpf_verifier_env *env,
prog = env->prog;
btf = prog->aux->btf;
- urecord = make_bpfptr(attr->func_info, uattr.is_kernel);
+ urecord = make_bpfptr(attr->func_info, bpfptr_is_kernel(uattr));
krecord = prog->aux->func_info;
info_aux = kcalloc(nfuncs, sizeof(*info_aux), GFP_KERNEL | __GFP_NOWARN);
@@ -16036,7 +16036,7 @@ static int check_btf_line(struct bpf_verifier_env *env,
s = 0;
sub = env->subprog_info;
- ulinfo = make_bpfptr(attr->line_info, uattr.is_kernel);
+ ulinfo = make_bpfptr(attr->line_info, bpfptr_is_kernel(uattr));
expected_size = sizeof(struct bpf_line_info);
ncopy = min_t(u32, expected_size, rec_size);
for (i = 0; i < nr_linfo; i++) {
@@ -16154,7 +16154,7 @@ static int check_core_relo(struct bpf_verifier_env *env,
rec_size % sizeof(u32))
return -EINVAL;
- u_core_relo = make_bpfptr(attr->core_relos, uattr.is_kernel);
+ u_core_relo = make_bpfptr(attr->core_relos, bpfptr_is_kernel(uattr));
expected_size = sizeof(struct bpf_core_relo);
ncopy = min_t(u32, expected_size, rec_size);
@@ -20790,7 +20790,7 @@ int bpf_check(struct bpf_prog **prog, union bpf_attr *attr, bpfptr_t uattr, __u3
env->insn_aux_data[i].orig_idx = i;
env->prog = *prog;
env->ops = bpf_verifier_ops[env->prog->type];
- env->fd_array = make_bpfptr(attr->fd_array, uattr.is_kernel);
+ env->fd_array = make_bpfptr(attr->fd_array, bpfptr_is_kernel(uattr));
is_priv = bpf_capable();
bpf_get_btf_vmlinux();
--
2.17.1
-
Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, UK
Registration No: 1397386 (Wales)
^ permalink raw reply related [flat|nested] 6+ messages in thread
* [PATCH net-next 3/4] bpf: Use the sockptr_t helpers
2023-12-25 9:46 [PATCH net-next 0/4] sockptr: Change sockptr_t to be a struct David Laight
2023-12-25 9:51 ` [PATCH net-next 1/4] net: Use sockptr_is_kernel() instead of testing is_kernel David Laight
2023-12-25 9:55 ` [PATCH net-next 2/4] bpf: Use bpfptr_is_kernel() instead of checking the is_kernel member David Laight
@ 2023-12-25 9:57 ` David Laight
2023-12-25 9:58 ` [PATCH net-next 4/4] sockptr: Change sockptr_t to be a struct of a kernel and user pointer David Laight
2024-01-02 22:32 ` [PATCH net-next 0/4] sockptr: Change sockptr_t to be a struct Jakub Kicinski
4 siblings, 0 replies; 6+ messages in thread
From: David Laight @ 2023-12-25 9:57 UTC (permalink / raw)
To: 'netdev@vger.kernel.org', 'David S . Miller',
'kuba@kernel.org'
Cc: 'eric.dumazet@gmail.com', 'martin.lau@linux.dev',
'Alexei Starovoitov', 'Stephen Hemminger',
'Jens Axboe', 'Daniel Borkmann',
'Andrii Nakryiko'
bpfptr_t is defined as sockptr_t but the bpfptr_is_kernel(),
bpfptr_is_null(), KERNEL_BPFPTR() and USER_BPFPTR() helpers are
copies of the sockptr ones.
Instead implement in terms of the sockptr helpers.
Signed-off-by: David Laight <david.laight@aculab.com>
---
include/linux/bpfptr.h | 10 ++++------
1 file changed, 4 insertions(+), 6 deletions(-)
diff --git a/include/linux/bpfptr.h b/include/linux/bpfptr.h
index 79b2f78eec1a..862e87350477 100644
--- a/include/linux/bpfptr.h
+++ b/include/linux/bpfptr.h
@@ -10,17 +10,17 @@ typedef sockptr_t bpfptr_t;
static inline bool bpfptr_is_kernel(bpfptr_t bpfptr)
{
- return bpfptr.is_kernel;
+ return sockptr_is_kernel(bpfptr);
}
static inline bpfptr_t KERNEL_BPFPTR(void *p)
{
- return (bpfptr_t) { .kernel = p, .is_kernel = true };
+ return KERNEL_SOCKPTR(p);
}
static inline bpfptr_t USER_BPFPTR(void __user *p)
{
- return (bpfptr_t) { .user = p };
+ return USER_SOCKPTR(p);
}
static inline bpfptr_t make_bpfptr(u64 addr, bool is_kernel)
@@ -33,9 +33,7 @@ static inline bpfptr_t make_bpfptr(u64 addr, bool is_kernel)
static inline bool bpfptr_is_null(bpfptr_t bpfptr)
{
- if (bpfptr_is_kernel(bpfptr))
- return !bpfptr.kernel;
- return !bpfptr.user;
+ return sockptr_is_null(bpfptr);
}
static inline void bpfptr_add(bpfptr_t *bpfptr, size_t val)
--
2.17.1
-
Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, UK
Registration No: 1397386 (Wales)
^ permalink raw reply related [flat|nested] 6+ messages in thread
* [PATCH net-next 4/4] sockptr: Change sockptr_t to be a struct of a kernel and user pointer.
2023-12-25 9:46 [PATCH net-next 0/4] sockptr: Change sockptr_t to be a struct David Laight
` (2 preceding siblings ...)
2023-12-25 9:57 ` [PATCH net-next 3/4] bpf: Use the sockptr_t helpers David Laight
@ 2023-12-25 9:58 ` David Laight
2024-01-02 22:32 ` [PATCH net-next 0/4] sockptr: Change sockptr_t to be a struct Jakub Kicinski
4 siblings, 0 replies; 6+ messages in thread
From: David Laight @ 2023-12-25 9:58 UTC (permalink / raw)
To: 'netdev@vger.kernel.org', 'David S . Miller',
'kuba@kernel.org'
Cc: 'eric.dumazet@gmail.com', 'martin.lau@linux.dev',
'Alexei Starovoitov', 'Stephen Hemminger',
'Jens Axboe', 'Daniel Borkmann',
'Andrii Nakryiko'
The original commit for sockptr_t tried to use the pointer value
to determine whether a pointer was user or kernel.
This can't work on some architecures and was buffy on x86.
So the is_kernel descriminator was added after the union of pointers.
However this is still open to misuse and accidents.
Replace the union with a struct and remove the is_kernel member.
The user and kernel values are now in different places.
The size doesn't change - it was always padded out to 'two pointers'.
The only functional difference is that NULL pointers are always 'user'.
So dereferncing will (usually) fault in copy_from_user() rather than
panic if supplied as a kernel address.
Simple driver code that uses kernel sockets still works.
I've not tested bpf - but that should work unless it is breaking
the rules.
Signed-off-by: David Laight <david.laight@aculab.com>
---
include/linux/sockptr.h | 15 +++++----------
1 file changed, 5 insertions(+), 10 deletions(-)
diff --git a/include/linux/sockptr.h b/include/linux/sockptr.h
index 307961b41541..7516c2ada6a8 100644
--- a/include/linux/sockptr.h
+++ b/include/linux/sockptr.h
@@ -12,21 +12,18 @@
#include <linux/uaccess.h>
typedef struct {
- union {
- void *kernel;
- void __user *user;
- };
- bool is_kernel : 1;
+ void *kernel;
+ void __user *user;
} sockptr_t;
static inline bool sockptr_is_kernel(sockptr_t sockptr)
{
- return sockptr.is_kernel;
+ return !!sockptr.kernel;
}
static inline sockptr_t KERNEL_SOCKPTR(void *p)
{
- return (sockptr_t) { .kernel = p, .is_kernel = true };
+ return (sockptr_t) { .kernel = p };
}
static inline sockptr_t USER_SOCKPTR(void __user *p)
@@ -36,9 +33,7 @@ static inline sockptr_t USER_SOCKPTR(void __user *p)
static inline bool sockptr_is_null(sockptr_t sockptr)
{
- if (sockptr_is_kernel(sockptr))
- return !sockptr.kernel;
- return !sockptr.user;
+ return !sockptr.user && !sockptr.kernel;
}
static inline int copy_from_sockptr_offset(void *dst, sockptr_t src,
--
2.17.1
-
Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, UK
Registration No: 1397386 (Wales)
^ permalink raw reply related [flat|nested] 6+ messages in thread