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 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 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 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(