On Fri, Nov 25, 2022 at 02:47:51AM +0100, Stefano Brivio wrote:On Thu, 24 Nov 2022 12:16:44 +1100 David Gibson <david(a)gibson.dropbear.id.au> wrote:You mean in the target namespace? Which could be init or otherwise depending on direction. That did occur to me, however, I believe the only way it would be not free is if we have a fixed port mapping occupying that port - in which case the problem goes away again in 8/16.pasta handles "spliced" port forwarding by resending datagrams received on a bound socket in the init namespace to a connected socket in the guest namespace. This means there are actually three ports associated with each "connection". First there's the source and destination ports of the originating datagram. That's also the destination port of the forwarded datagram, but the source port of the forwarded datagram is the kernel allocated bound address of the connected socket. However, by bind()ing as well as connect()ing the forwarding socket we can choose the source port of the forwarded datagrams. By choosing it to match the original source port we remove that surprising third port number and no longer need to store port numbers in struct udp_splice_port.If you wondered, I think the whole connect() with getsockname() thing without a bind() came from the fundamental misconception I had that you couldn't connect() a bound socket -- and I didn't quite think of dropping connect() as you do in 3/16 anyway. There's one minor problem this introduces: the source port of the originating datagram now needs to be free in the init namespace. It's still better than the alternative problem you fix in 16/16, though.I'm wondering if we could, _once you're done with all this_ (it already looks complicated enough), revisit the 'goto fail' in udp_splice_connect() (now udp_splice_new()) when bind() fails, and just proceed with an ephemeral port then.Oof... I'd rather not, because then we'd have to have somewhere to keep track of the original source port just for that unlikely edge case.Also, I haven't tried, but I'm not sure if this introduces some kind of DoS possibility: even if pasta forwards a single port, it should be possible for a remote host to make pasta bind to a large amount of non-ephemeral ports.Hm, yes, that's a point.Maybe it would make sense to think of a limit on how many ports a single peer could cause pasta to bind.Maybe, yes.I'm not sure yet how we could track peers without a separate address storage (even though keeping an LRU array should be feasible) -- the simpler alternative, limiting bound ports by destination port, would offer an even more convenient way to a DoS. On the other hand, this is exceedingly minor I guess. We're binding ports in the namespace after all, and we can reuse bound sockets.Right. I think this is something to address when and if it proves to be a problem in practice. -- David Gibson | I'll have my music baroque, and my code david AT gibson.dropbear.id.au | minimalist, thank you. NOT _the_ _other_ | _way_ _around_! http://www.ozlabs.org/~dgibson