On Fri, Oct 12, 2018 at 7:31 PM, Alexei Starovoitov alexei.starovoitov@gmail.com wrote:
On Fri, Oct 12, 2018 at 03:54:27AM -0700, Daniel Colascione wrote:
The map-in-map frequently serves as a mechanism for atomic snapshotting of state that a BPF program might record. The current implementation is dangerous to use in this way, however, since userspace has no way of knowing when all programs that might have retrieved the "old" value of the map may have completed.
This change ensures that map update operations on map-in-map map types always wait for all references to the old map to drop before returning to userspace.
Signed-off-by: Daniel Colascione dancol@google.com
kernel/bpf/syscall.c | 14 ++++++++++++++ 1 file changed, 14 insertions(+)
diff --git a/kernel/bpf/syscall.c b/kernel/bpf/syscall.c index 8339d81cba1d..d7c16ae1e85a 100644 --- a/kernel/bpf/syscall.c +++ b/kernel/bpf/syscall.c @@ -741,6 +741,18 @@ static int map_lookup_elem(union bpf_attr *attr) return err; }
+static void maybe_wait_bpf_programs(struct bpf_map *map) +{
/* Wait for any running BPF programs to complete so that* userspace, when we return to it, knows that all programs* that could be running use the new map value.*/if (map->map_type == BPF_MAP_TYPE_HASH_OF_MAPS ||map->map_type == BPF_MAP_TYPE_ARRAY_OF_MAPS) {synchronize_rcu();}extra {} were not necessary. I removed them while applying to bpf-next. Please run checkpatch.pl next time. Thanks
Thanks Alexei for taking it. Me and Lorenzo were discussing that not having this causes incorrect behavior for apps using map-in-map for this. So I CC'd stable as well.
-Joel
On Tue, Oct 16, 2018 at 10:39:57AM -0700, Joel Fernandes wrote:
On Fri, Oct 12, 2018 at 7:31 PM, Alexei Starovoitov alexei.starovoitov@gmail.com wrote:
On Fri, Oct 12, 2018 at 03:54:27AM -0700, Daniel Colascione wrote:
The map-in-map frequently serves as a mechanism for atomic snapshotting of state that a BPF program might record. The current implementation is dangerous to use in this way, however, since userspace has no way of knowing when all programs that might have retrieved the "old" value of the map may have completed.
This change ensures that map update operations on map-in-map map types always wait for all references to the old map to drop before returning to userspace.
Signed-off-by: Daniel Colascione dancol@google.com
kernel/bpf/syscall.c | 14 ++++++++++++++ 1 file changed, 14 insertions(+)
diff --git a/kernel/bpf/syscall.c b/kernel/bpf/syscall.c index 8339d81cba1d..d7c16ae1e85a 100644 --- a/kernel/bpf/syscall.c +++ b/kernel/bpf/syscall.c @@ -741,6 +741,18 @@ static int map_lookup_elem(union bpf_attr *attr) return err; }
+static void maybe_wait_bpf_programs(struct bpf_map *map) +{
/* Wait for any running BPF programs to complete so that* userspace, when we return to it, knows that all programs* that could be running use the new map value.*/if (map->map_type == BPF_MAP_TYPE_HASH_OF_MAPS ||map->map_type == BPF_MAP_TYPE_ARRAY_OF_MAPS) {synchronize_rcu();}extra {} were not necessary. I removed them while applying to bpf-next. Please run checkpatch.pl next time. Thanks
Thanks Alexei for taking it. Me and Lorenzo were discussing that not having this causes incorrect behavior for apps using map-in-map for this. So I CC'd stable as well.
It is too late in the release cycle. We can submit it to stable releases after the merge window.
On Thu, Oct 18, 2018 at 08:46:59AM -0700, Alexei Starovoitov wrote:
On Tue, Oct 16, 2018 at 10:39:57AM -0700, Joel Fernandes wrote:
On Fri, Oct 12, 2018 at 7:31 PM, Alexei Starovoitov alexei.starovoitov@gmail.com wrote:
On Fri, Oct 12, 2018 at 03:54:27AM -0700, Daniel Colascione wrote:
The map-in-map frequently serves as a mechanism for atomic snapshotting of state that a BPF program might record. The current implementation is dangerous to use in this way, however, since userspace has no way of knowing when all programs that might have retrieved the "old" value of the map may have completed.
This change ensures that map update operations on map-in-map map types always wait for all references to the old map to drop before returning to userspace.
Signed-off-by: Daniel Colascione dancol@google.com
kernel/bpf/syscall.c | 14 ++++++++++++++ 1 file changed, 14 insertions(+)
diff --git a/kernel/bpf/syscall.c b/kernel/bpf/syscall.c index 8339d81cba1d..d7c16ae1e85a 100644 --- a/kernel/bpf/syscall.c +++ b/kernel/bpf/syscall.c @@ -741,6 +741,18 @@ static int map_lookup_elem(union bpf_attr *attr) return err; }
+static void maybe_wait_bpf_programs(struct bpf_map *map) +{
/* Wait for any running BPF programs to complete so that* userspace, when we return to it, knows that all programs* that could be running use the new map value.*/if (map->map_type == BPF_MAP_TYPE_HASH_OF_MAPS ||map->map_type == BPF_MAP_TYPE_ARRAY_OF_MAPS) {synchronize_rcu();}extra {} were not necessary. I removed them while applying to bpf-next. Please run checkpatch.pl next time. Thanks
Thanks Alexei for taking it. Me and Lorenzo were discussing that not having this causes incorrect behavior for apps using map-in-map for this. So I CC'd stable as well.
It is too late in the release cycle. We can submit it to stable releases after the merge window.
Sounds good, thanks.
- Joel
Hi netdev,
Could we queue up this patch to stable 4.14 and stable 4.19? I can provide a backport patch if needed. I checked it is a clean cherry-pick for 4.19 but have some minor conflict for 4.14.
Thanks Chenbo Feng On Thu, Oct 18, 2018 at 4:36 PM Joel Fernandes joel@joelfernandes.org wrote:
On Thu, Oct 18, 2018 at 08:46:59AM -0700, Alexei Starovoitov wrote:
On Tue, Oct 16, 2018 at 10:39:57AM -0700, Joel Fernandes wrote:
On Fri, Oct 12, 2018 at 7:31 PM, Alexei Starovoitov alexei.starovoitov@gmail.com wrote:
On Fri, Oct 12, 2018 at 03:54:27AM -0700, Daniel Colascione wrote:
The map-in-map frequently serves as a mechanism for atomic snapshotting of state that a BPF program might record. The current implementation is dangerous to use in this way, however, since userspace has no way of knowing when all programs that might have retrieved the "old" value of the map may have completed.
This change ensures that map update operations on map-in-map map types always wait for all references to the old map to drop before returning to userspace.
Signed-off-by: Daniel Colascione dancol@google.com
kernel/bpf/syscall.c | 14 ++++++++++++++ 1 file changed, 14 insertions(+)
diff --git a/kernel/bpf/syscall.c b/kernel/bpf/syscall.c index 8339d81cba1d..d7c16ae1e85a 100644 --- a/kernel/bpf/syscall.c +++ b/kernel/bpf/syscall.c @@ -741,6 +741,18 @@ static int map_lookup_elem(union bpf_attr *attr) return err; }
+static void maybe_wait_bpf_programs(struct bpf_map *map) +{
/* Wait for any running BPF programs to complete so that* userspace, when we return to it, knows that all programs* that could be running use the new map value.*/if (map->map_type == BPF_MAP_TYPE_HASH_OF_MAPS ||map->map_type == BPF_MAP_TYPE_ARRAY_OF_MAPS) {synchronize_rcu();}extra {} were not necessary. I removed them while applying to bpf-next. Please run checkpatch.pl next time. Thanks
Thanks Alexei for taking it. Me and Lorenzo were discussing that not having this causes incorrect behavior for apps using map-in-map for this. So I CC'd stable as well.
It is too late in the release cycle. We can submit it to stable releases after the merge window.
Sounds good, thanks.
- Joel
On Fri, Nov 09, 2018 at 06:01:54PM -0800, Chenbo Feng wrote:
Hi netdev,
Could we queue up this patch to stable 4.14 and stable 4.19? I can provide a backport patch if needed. I checked it is a clean cherry-pick for 4.19 but have some minor conflict for 4.14.
What is the git commit id of the patch in Linus's tree?
thanks
greg k-h
The Linus's tree commit SHA is 1ae80cf31938c8f77c37a29bbe29e7f1cd492be8.
I can send the patch to stable directly if needed.
Thanks Chenbo Feng On Sat, Nov 10, 2018 at 7:22 AM Greg KH gregkh@linuxfoundation.org wrote:
On Fri, Nov 09, 2018 at 06:01:54PM -0800, Chenbo Feng wrote:
Hi netdev,
Could we queue up this patch to stable 4.14 and stable 4.19? I can provide a backport patch if needed. I checked it is a clean cherry-pick for 4.19 but have some minor conflict for 4.14.
What is the git commit id of the patch in Linus's tree?
thanks
greg k-h
linux-stable-mirror@lists.linaro.org
-
Alexei Starovoitov -
Chenbo Feng -
Greg KH -
Joel Fernandes