On 2024-09-02, Arnd Bergmann <arnd@xxxxxxxx> wrote: > On Mon, Sep 2, 2024, at 07:06, Aleksa Sarai wrote: > > While we do currently return -EFAULT in this case, it seems prudent to > > follow the behaviour of other syscalls like clone3. It seems quite > > unlikely that anyone depends on this error code being EFAULT, but we can > > always revert this if it turns out to be an issue. > > Right, it's probably a good idea to have a limit there rather than > having a busy loop with a user-provided length when the only bound is > the available virtual memory. > > > if (unlikely(usize < OPEN_HOW_SIZE_VER0)) > > return -EINVAL; > > + if (unlikely(usize > PAGE_SIZE)) > > + return -E2BIG; > > > > Is PAGE_SIZE significant here? If there is a need to enforce a limit, > I would expect this to be the same regardless of kernel configuration, > since the structure layout is also independent of the configuration. PAGE_SIZE is what clone3, perf_event_open, sched_setattr, bpf, etc all use. The idea was that PAGE_SIZE is the absolute limit of any reasonable extensible structure size because we are never going to have argument structures that are larger than a page (I think this was discussed in the original copy_struct_from_user() patchset thread in late 2019, but I can't find the reference at the moment.) I simply forgot to add this when I first submitted openat2, the original intention was to just match the other syscalls. > Where is the current -EFAULT for users passing more than a page? > I only see it for reads beyond the VMA, but not e.g. when checking > terabytes of zero pages from an anonymous mapping. I meant that we in practice return -EFAULT if you pass a really large size (because you end up running off the end of mapped memory). There is no explicit -EFAULT for large sizes, which is exactly the problem. :P > > Arnd -- Aleksa Sarai Senior Software Engineer (Containers) SUSE Linux GmbH <https://www.cyphar.com/>
Attachment:
signature.asc
Description: PGP signature
