Re: [RFC PATCH net-next] bpf: fix potential percpu map overcopy to user.

2016-10-20 Thread Daniel Borkmann
On 10/20/2016 08:41 PM, William Tu wrote: On Thu, Oct 20, 2016 at 9:58 AM, Alexei Starovoitov wrote: On Thu, Oct 20, 2016 at 06:04:38PM +0200, Daniel Borkmann wrote: diff --git a/tools/testing/selftests/bpf/test_maps.c b/tools/testing/selftests/bpf/test_maps.c

Re: [RFC PATCH net-next] bpf: fix potential percpu map overcopy to user.

2016-10-20 Thread William Tu
> Documentation/cputopology.txt +106 says /sys/devices/system/cpu/possible > outputs cpu_possible_mask. That is the same as in num_possible_cpus(), so > first step would be to fix the buggy example code, imho. > > What perhaps could be done in a second step to reduce overhead is an option > for

Re: [RFC PATCH net-next] bpf: fix potential percpu map overcopy to user.

2016-10-20 Thread William Tu
On Thu, Oct 20, 2016 at 9:58 AM, Alexei Starovoitov wrote: > On Thu, Oct 20, 2016 at 06:04:38PM +0200, Daniel Borkmann wrote: >> >> diff --git a/tools/testing/selftests/bpf/test_maps.c >> b/tools/testing/selftests/bpf/test_maps.c >> index ee384f0..d4832e8 100644 >>

Re: [RFC PATCH net-next] bpf: fix potential percpu map overcopy to user.

2016-10-20 Thread Alexei Starovoitov
On Thu, Oct 20, 2016 at 06:04:38PM +0200, Daniel Borkmann wrote: > > diff --git a/tools/testing/selftests/bpf/test_maps.c > b/tools/testing/selftests/bpf/test_maps.c > index ee384f0..d4832e8 100644 > --- a/tools/testing/selftests/bpf/test_maps.c > +++ b/tools/testing/selftests/bpf/test_maps.c >

Re: [RFC PATCH net-next] bpf: fix potential percpu map overcopy to user.

2016-10-20 Thread Daniel Borkmann
On 10/19/2016 05:15 PM, Daniel Borkmann wrote: On 10/19/2016 07:31 AM, William Tu wrote: ... - if (copy_to_user(uvalue, value, value_size) != 0) + if (copy_to_user(uvalue, value, min_t(u32, usize, value_size)) != 0) goto free_value; I think such approach won't actually

Re: [RFC PATCH net-next] bpf: fix potential percpu map overcopy to user.

2016-10-19 Thread Daniel Borkmann
On 10/19/2016 07:31 AM, William Tu wrote: ... - if (copy_to_user(uvalue, value, value_size) != 0) + if (copy_to_user(uvalue, value, min_t(u32, usize, value_size)) != 0) goto free_value; I think such approach won't actually fix anything. User space may lose some of the

Re: [RFC PATCH net-next] bpf: fix potential percpu map overcopy to user.

2016-10-19 Thread Daniel Borkmann
On 10/19/2016 07:31 AM, William Tu wrote: ... - if (copy_to_user(uvalue, value, value_size) != 0) + if (copy_to_user(uvalue, value, min_t(u32, usize, value_size)) != 0) goto free_value; I think such approach won't actually fix anything. User space may lose some of the

Re: [RFC PATCH net-next] bpf: fix potential percpu map overcopy to user.

2016-10-18 Thread William Tu
> ... >> - if (copy_to_user(uvalue, value, value_size) != 0) >> + if (copy_to_user(uvalue, value, min_t(u32, usize, value_size)) != 0) >> goto free_value; > > I think such approach won't actually fix anything. User space > may lose some of the values and won't have any idea

Re: [RFC PATCH net-next] bpf: fix potential percpu map overcopy to user.

2016-10-18 Thread Alexei Starovoitov
On Sun, Oct 16, 2016 at 09:41:28AM -0700, William Tu wrote: > When running bpf_map_lookup on percpu elements, the bytes copied to > userspace depends on num_possible_cpus() * value_size, which could > potentially be larger than memory allocated from user, which depends > on

[RFC PATCH net-next] bpf: fix potential percpu map overcopy to user.

2016-10-16 Thread William Tu
When running bpf_map_lookup on percpu elements, the bytes copied to userspace depends on num_possible_cpus() * value_size, which could potentially be larger than memory allocated from user, which depends on sysconf(_SC_NPROCESSORS_CONF) to get the current cpu num. As a result, the inconsistency