On Thu, Oct 29, 2020 at 8:14 PM Michael Kerrisk (man-pages) <mtk.manpages@xxxxxxxxx> wrote: > On 10/29/20 2:42 AM, Jann Horn wrote: > > As discussed at > > <https://lore.kernel.org/r/CAG48ez0m4Y24ZBZCh+Tf4ORMm9_q4n7VOzpGjwGF7_Fe8EQH=Q@xxxxxxxxxxxxxx>, > > we need to re-check checkNotificationIdIsValid() after reading remote > > memory but before using the read value in any way. Otherwise, the > > syscall could in the meantime get interrupted by a signal handler, the > > signal handler could return, and then the function that performed the > > syscall could free() allocations or return (thereby freeing buffers on > > the stack). > > > > In essence, this pread() is (unavoidably) a potential use-after-free > > read; and to make that not have any security impact, we need to check > > whether UAF read occurred before using the read value. This should > > probably be called out elsewhere in the manpage, too... > > > > Now, of course, **reading** is the easy case. The difficult case is if > > we have to **write** to the remote process... because then we can't > > play games like that. If we write data to a freed pointer, we're > > screwed, that's it. (And for somewhat unrelated bonus fun, consider > > that /proc/$pid/mem is originally intended for process debugging, > > including installing breakpoints, and will therefore happily write > > over "readonly" private mappings, such as typical mappings of > > executable code.) > > > > So, uuuuh... I guess if anyone wants to actually write memory back to > > the target process, we'd better come up with some dedicated API for > > that, using an ioctl on the seccomp fd that magically freezes the > > target process inside the syscall while writing to its memory, or > > something like that? And until then, the manpage should have a big fat > > warning that writing to the target's memory is simply not possible > > (safely). > > Thank you for your very clear explanation! It turned out to be > trivially easy to demonstrate this issue with a slightly modified > version of my program. > > As well as the change to the code example that I already mentioned > my reply of a few hours ago, I've added the following text to the > page: > > Caveats regarding the use of /proc/[tid]/mem > The discussion above noted the need to use the > SECCOMP_IOCTL_NOTIF_ID_VALID ioctl(2) when opening the > /proc/[tid]/mem file of the target to avoid the possibility of > accessing the memory of the wrong process in the event that the > target terminates and its ID is recycled by another (unrelated) > thread. However, the use of this ioctl(2) operation is also > necessary in other situations, as explained in the following > pargraphs. (nit: paragraphs) > Consider the following scenario, where the supervisor tries to > read the pathname argument of a target's blocked mount(2) system > call: [...] > Seem okay? Yeah, sounds good. > By the way, is there any analogous kind of issue concerning > pidfd_getfd()? I'm thinking not, but I wonder if I've missed > something. When it is used by a seccomp supervisor, you mean? I think basically the same thing applies - when resource identifiers (such as memory addresses or file descriptors) are passed to a syscall, it generally has to be assumed that those identifiers may become invalid and be reused as soon as the syscall has returned.
