lists.openwall.net   lists  /  announce  owl-users  owl-dev  john-users  john-dev  passwdqc-users  yescrypt  popa3d-users  /  oss-security  kernel-hardening  musl  sabotage  tlsify  passwords  /  crypt-dev  xvendor  /  Bugtraq  Full-Disclosure  linux-kernel  linux-netdev  linux-ext4  linux-hardening  PHC 
Open Source and information security mailing list archives
 
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Date:   Sat, 16 Nov 2019 21:57:45 -0800
From:   Andrii Nakryiko <andrii.nakryiko@...il.com>
To:     Alexei Starovoitov <ast@...com>
Cc:     Daniel Borkmann <daniel@...earbox.net>,
        Andrii Nakryiko <andriin@...com>,
        "bpf@...r.kernel.org" <bpf@...r.kernel.org>,
        "netdev@...r.kernel.org" <netdev@...r.kernel.org>,
        Kernel Team <Kernel-team@...com>,
        Johannes Weiner <hannes@...xchg.org>,
        Rik van Riel <riel@...riel.com>
Subject: Re: [PATCH v4 bpf-next 2/4] bpf: add mmap() support for BPF_MAP_TYPE_ARRAY

On Fri, Nov 15, 2019 at 5:18 PM Alexei Starovoitov <ast@...com> wrote:
>
> On 11/15/19 4:13 PM, Daniel Borkmann wrote:
> >>> Yeah, only for fd array currently. Question is, if we ever reuse that
> >>> map_release_uref
> >>> callback in future for something else, will we remember that we earlier
> >>> missed to add
> >>> it here? :/
> >>
> >> What do you mean 'missed to add' ?
> >
> > Was saying missed to add the inc/put for the uref counter.
> >
> >> This is mmap path. Anything that needs releasing (like FDs for
> >> prog_array or progs for sockmap) cannot be mmap-able.
> >
> > Right, I meant if in future we ever have another use case outside of it
> > for some reason (unrelated to those maps you mention above). Can we
> > guarantee this is never going to happen? Seemed less fragile at least to
> > maintain proper count here.

I don't think we'll ever going to allow mmaping anything that contains
not just pure data. E.g., we disallow mmaping array that contains spin
lock for that reason. So I think it's safe to assume that this is not
going to happen even for future maps. At least not without some
serious considerations before that. So I'm going to keep it as just
plain bpf_map_inc for now.

I'm going to convert bpf_prog_add/bpf_prog_inc, though, and will do it
as a separate patch, on top of bpf_map_inc refactor. It touches quite
a lot drivers, so would benefit from having being separate.

>
> I'm struggling to understand the concern.
> map-in-map, xskmap, socket local storage are doing bpf_map_inc(, false)
> when they need to hold the map. Why this case is any different?

Powered by blists - more mailing lists