Re: [PATCH 11/18] maccess: remove strncpy_from_unsafe

2020-05-14 Thread Daniel Borkmann
On 5/14/20 11:44 AM, Masami Hiramatsu wrote: On Wed, 13 May 2020 19:43:24 -0700 Linus Torvalds wrote: On Wed, May 13, 2020 at 6:00 PM Masami Hiramatsu wrote: But we should likely at least disallow it entirely on platforms where we really can't - or pick one hardcoded choice. On sparc, you r

Re: [PATCH 11/18] maccess: remove strncpy_from_unsafe

2020-05-14 Thread Daniel Borkmann
On 5/14/20 12:01 PM, David Laight wrote: [...] If it's not a stupid question why is a BPF program allowed to get into a situation where it might have an invalid kernel address. It all stinks of a hole that allows all of kernel memory to be read and copied to userspace. Now you might want to som

RE: [PATCH 11/18] maccess: remove strncpy_from_unsafe

2020-05-14 Thread David Laight
From: Daniel Borkmann > Sent: 14 May 2020 00:59 > On 5/14/20 1:28 AM, Al Viro wrote: > > On Thu, May 14, 2020 at 12:36:28AM +0200, Daniel Borkmann wrote: > > > >>> So on say s390 TASK_SIZE_USUALLy is (-PAGE_SIZE), which means we'd alway > >>> try the user copy first, which seems odd. > >>> > >>> I'

Re: [PATCH 11/18] maccess: remove strncpy_from_unsafe

2020-05-14 Thread Masami Hiramatsu
On Wed, 13 May 2020 19:43:24 -0700 Linus Torvalds wrote: > On Wed, May 13, 2020 at 6:00 PM Masami Hiramatsu wrote: > > > > > But we should likely at least disallow it entirely on platforms where > > > we really can't - or pick one hardcoded choice. On sparc, you really > > > _have_ to specify on

Re: [PATCH 11/18] maccess: remove strncpy_from_unsafe

2020-05-13 Thread Linus Torvalds
On Wed, May 13, 2020 at 6:00 PM Masami Hiramatsu wrote: > > > But we should likely at least disallow it entirely on platforms where > > we really can't - or pick one hardcoded choice. On sparc, you really > > _have_ to specify one or the other. > > OK. BTW, is there any way to detect the kernel/us

Re: [PATCH 11/18] maccess: remove strncpy_from_unsafe

2020-05-13 Thread Masami Hiramatsu
On Wed, 13 May 2020 16:59:40 -0700 Linus Torvalds wrote: > On Wed, May 13, 2020 at 4:21 PM Masami Hiramatsu wrote: > > > > > > For trace_kprobe.c current order (kernel -> user fallback) is preferred > > because it has another function dedicated for user memory. > > Well, then it should just use

Re: [PATCH 11/18] maccess: remove strncpy_from_unsafe

2020-05-13 Thread Linus Torvalds
On Wed, May 13, 2020 at 4:21 PM Masami Hiramatsu wrote: > > > For trace_kprobe.c current order (kernel -> user fallback) is preferred > because it has another function dedicated for user memory. Well, then it should just use the "strict" kernel-only one for the non-user memory. But yes, if there

Re: [PATCH 11/18] maccess: remove strncpy_from_unsafe

2020-05-13 Thread Daniel Borkmann
On 5/14/20 1:28 AM, Al Viro wrote: On Thu, May 14, 2020 at 12:36:28AM +0200, Daniel Borkmann wrote: So on say s390 TASK_SIZE_USUALLy is (-PAGE_SIZE), which means we'd alway try the user copy first, which seems odd. I'd really like to here from the bpf folks what the expected use case is here,

Re: [PATCH 11/18] maccess: remove strncpy_from_unsafe

2020-05-13 Thread Al Viro
On Thu, May 14, 2020 at 12:36:28AM +0200, Daniel Borkmann wrote: > > So on say s390 TASK_SIZE_USUALLy is (-PAGE_SIZE), which means we'd alway > > try the user copy first, which seems odd. > > > > I'd really like to here from the bpf folks what the expected use case > > is here, and if the typical

Re: [PATCH 11/18] maccess: remove strncpy_from_unsafe

2020-05-13 Thread Daniel Borkmann
On 5/14/20 1:03 AM, Linus Torvalds wrote: On Wed, May 13, 2020 at 3:36 PM Daniel Borkmann wrote: It's used for both. Daniel, BPF real;ly needs to make up its mind about that. You *cannot* use ti for both. Yes, it happens to work on x86 and some other architectures. But on other architectu

Re: [PATCH 11/18] maccess: remove strncpy_from_unsafe

2020-05-13 Thread Masami Hiramatsu
On Thu, 14 May 2020 00:36:28 +0200 Daniel Borkmann wrote: > On 5/13/20 9:28 PM, Christoph Hellwig wrote: > > On Wed, May 13, 2020 at 12:11:27PM -0700, Linus Torvalds wrote: > >> On Wed, May 13, 2020 at 9:01 AM Christoph Hellwig wrote: > >>> > >>> +static void bpf_strncpy(char *buf, long unsafe_a

Re: [PATCH 11/18] maccess: remove strncpy_from_unsafe

2020-05-13 Thread Linus Torvalds
On Wed, May 13, 2020 at 3:36 PM Daniel Borkmann wrote: > > It's used for both. Daniel, BPF real;ly needs to make up its mind about that. You *cannot* use ti for both. Yes, it happens to work on x86 and some other architectures. But on other architectures, the exact same pointer value can be a

Re: [PATCH 11/18] maccess: remove strncpy_from_unsafe

2020-05-13 Thread Daniel Borkmann
On 5/13/20 9:28 PM, Christoph Hellwig wrote: On Wed, May 13, 2020 at 12:11:27PM -0700, Linus Torvalds wrote: On Wed, May 13, 2020 at 9:01 AM Christoph Hellwig wrote: +static void bpf_strncpy(char *buf, long unsafe_addr) +{ + buf[0] = 0; + if (strncpy_from_kernel_nofault(buf, (void

Re: [PATCH 11/18] maccess: remove strncpy_from_unsafe

2020-05-13 Thread Christoph Hellwig
On Wed, May 13, 2020 at 12:11:27PM -0700, Linus Torvalds wrote: > On Wed, May 13, 2020 at 9:01 AM Christoph Hellwig wrote: > > > > +static void bpf_strncpy(char *buf, long unsafe_addr) > > +{ > > + buf[0] = 0; > > + if (strncpy_from_kernel_nofault(buf, (void *)unsafe_addr, > > +

Re: [PATCH 11/18] maccess: remove strncpy_from_unsafe

2020-05-13 Thread Linus Torvalds
On Wed, May 13, 2020 at 9:01 AM Christoph Hellwig wrote: > > +static void bpf_strncpy(char *buf, long unsafe_addr) > +{ > + buf[0] = 0; > + if (strncpy_from_kernel_nofault(buf, (void *)unsafe_addr, > + BPF_STRNCPY_LEN)) > + strncpy_from_user_nofault(

[PATCH 11/18] maccess: remove strncpy_from_unsafe

2020-05-13 Thread Christoph Hellwig
All three callers really should try the explicit kernel and user copies instead. One has already deprecated the somewhat dangerous either kernel or user address concept, the other two still need to follow up eventually. Signed-off-by: Christoph Hellwig Reviewed-by: Masami Hiramatsu --- include