On Thu, 2023-09-21 at 10:01 -0700, Jordan Rife wrote:

On Thu, Sep 21, 2023 at 8:26 AM Paolo Abeni [off-list ref] wrote:

quoted

On Thu, 2023-09-21 at 09:30 -0400, Willem de Bruijn wrote:

quoted

On Thu, Sep 21, 2023 at 4:35 AM Paolo Abeni [off-list ref] wrote:

quoted

On Wed, 2023-09-20 at 09:30 -0400, Willem de Bruijn wrote:

quoted

Jordan Rife wrote:

quoted

Similar to the change in commit 0bdf399342c5("net: Avoid address
overwrite in kernel_connect"), BPF hooks run on bind may rewrite the
address passed to kernel_bind(). This change

1) Makes a copy of the bind address in kernel_bind() to insulate
   callers.
2) Replaces direct calls to sock->ops->bind() with kernel_bind()

Link: https://lore.kernel.org/netdev/20230912013332.2048422-1-jrife@google.com/ (local)
Fixes: 4fbac77d2d09 ("bpf: Hooks for sys_bind")
Cc: stable@vger.kernel.org
Signed-off-by: Jordan Rife <redacted>

Reviewed-by: Willem de Bruijn <willemb@google.com>

I fear this is going to cause a few conflicts with other trees. We can
still take it, but at very least we will need some acks from the
relevant maintainers.

I *think* it would be easier split this and patch 1/3 in individual
patches targeting the different trees, hopefully not many additional
patches will be required. What do you think?

Roughly how many patches would result from this one patch. From the
stat line I count { block/drbd, char/agp, infiniband, isdn, fs/dlm,
fs/ocfs2, fs/smb, netfilter, rds }. That's worst case nine callers
plus the core patch to net/socket.c?

I think there should not be problems taking directly changes for rds
and nf/ipvs.

Additionally, I think the non network changes could consolidate the
bind and connect changes in a single patch.

It should be 7 not-network patches overall.

quoted

If logistically simpler and you prefer the approach, we can also
revisit Jordan's original approach, which embedded the memcpy inside
the BPF branches.

That has the slight benefit to in-kernel callers that it limits the
cost of the memcpy to cgroup_bpf_enabled. But adds a superfluous
second copy to the more common userspace callers, again at least only
if cgroup_bpf_enabled.

If so, it should at least move the whole logic around those BPF hooks
into helper functions.

IMHO the approach implemented here is preferable, I suggest going
forward with it.

Thanks,

Paolo

quoted

Additionally, I think the non network changes could consolidate the
bind and connect changes in a single patch.

It should be 7 not-network patches overall.

I'm fine with this. If there are no objections, I can drop the non-net
changes in this patch series and send out several
kernel_connect/kernel_bind patches to the appropriate trees as a
follow up. Shall we wait to hear back from the maintainers or just go
ahead with this plan?

I'm guessing you can go ahead with that: it should better fit anybody.

Thanks

Paolo

p.s. also using this reply to check if finally vger accepts my message
again...