Messages in this thread | | | From | Sven Schnelle <> | Subject | Re: [PATCH v2] tracing: Add test for user space strings when filtering on string pointers | Date | Tue, 11 Jan 2022 21:55:53 +0100 |
| |
David Laight <David.Laight@ACULAB.COM> writes:
> From: Steven Rostedt >> Sent: 10 January 2022 17:25 > ... >> > ... >> > > + if (likely((unsigned long)str >= TASK_SIZE)) { >> > > + /* For safety, do not trust the string pointer */ >> > > + if (!strncpy_from_kernel_nofault(kstr, str, USTRING_BUF_SIZE)) >> > > + return NULL; >> > > + } else { >> > > + /* user space address? */ >> > > + ustr = (char __user *)str; >> > > + if (!strncpy_from_user_nofault(kstr, ustr, USTRING_BUF_SIZE)) >> > > + return NULL; >> > >> > Is that check against TASK_SIZE even correct for all architectures? >> > copy_to/from_user() uses access_ok() - which is architecture dependant. >> >> The problem with access_ok() (which I tried first) is that it can't be used >> from interrupt context, and this check can happen in interrupt context. >> Either way, if we pick the wrong one for the arch, the only thing bad that >> can happen is that it returns "fault" and the filter fails, just like if >> the pointer was to bad memory. > > Isn't there also at least one architecture where you can't differentiate > between user and kernel pointers by looking at the address? > (Something like sparc ASI is used for user accesses so both user > and kernel get the full 4G address range. But it isn't sparc (or pdp/11)) > ISTR it causing issues with the code for kernel_setsockopt() and > required a separate flag.
On s390 TASK_SIZE is defined as -PAGE_SIZE, so with the patch above the kernel would always try to fetch it from user space. I think it would be the same for parisc.
> Put that together with something that needs user_access_begin() > to bracket user accesses and you probably fail big-time. > > David > > - > Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, UK > Registration No: 1397386 (Wales)
| |