[PATCH] tap: don't let overheard traffic move addr_seen when serving a fixed address

[PATCH] tap: don't let overheard traffic move addr_seen when serving a fixed address

[EJ Campbell]

The latest source address seen on the tap interface (ip4.addr_seen) is
taken to be the guest's and is used as the target for inbound port
forwarding. The update accepts any source address.

On a point-to-point tap that is always correct. But when the tap sits on
a shared L2 segment -- for example when it is bridged together with other
hosts -- pasta also sees frames from those other hosts. With a static
configuration (-a / address set, DHCP disabled), an overheard frame from
another host silently moves addr_seen to that host, and every subsequent
inbound forwarded connection is sent there instead of to the guest, so
connections are reset or hang.

I hit this running pasta with its tap enslaved to a Linux bridge that also
carries other traffic: inbound port forwarding to the guest broke as soon
as another address was seen on the segment.

Restrict the update so that, when DHCP is disabled (no_dhcp), only frames
sourced from the configured guest address may move addr_seen. With DHCP
enabled -- the default -- behaviour is unchanged, since the guest's
address is whatever pasta leases and tracking it is still correct.

The same reasoning applies to ip6.addr_seen on a shared segment; I left
IPv6 alone for now since I only exercise IPv4 in this setup, and would be
glad to extend it the same way if you prefer.

Signed-off-by: EJ Campbell <ej.campbell@gmail.com>
---
 tap.c | 14 +++++++++++++-
 1 file changed, 13 insertions(+), 1 deletion(-)

diff --git a/tap.c b/tap.c
index 4cba4c7..514379e 100644
--- a/tap.c
+++ b/tap.c
@@ -770,7 +770,19 @@ resume:
                      continue;
              }

-             if (iph->saddr && c->ip4.addr_seen.s_addr != iph->saddr)
+             /* The latest source address seen on the tap is taken to be the
+              * guest's, and becomes the target for inbound forwarding. On a
+              * point-to-point tap that's always correct. But if the tap sits
+              * on a shared L2 segment (for example bridged together with
+              * other hosts) and we serve a fixed address (DHCP disabled), a
+              * frame overheard from another host on that segment must not
+              * move addr_seen and silently retarget forwarded connections
+              * away from the guest. When DHCP is disabled, only the
+              * configured guest address may update addr_seen; with DHCP
+              * enabled (the default) behaviour is unchanged.
+              */
+             if (iph->saddr && c->ip4.addr_seen.s_addr != iph->saddr &&
+                 (!c->no_dhcp || iph->saddr == c->ip4.addr.s_addr))
                      c->ip4.addr_seen.s_addr = iph->saddr;

              if (!iov_drop_header(&data, hlen))

base-commit: 4b2823784aab04a70dfc295b16fd6f0592955790
--
2.53.0

Re: [PATCH] tap: don't let overheard traffic move addr_seen when serving a fixed address

[Stefano Brivio]

EJ, thanks for the detailed patch and commit message, and apologies for
the delayed response.

Some remarks:

On Tue, 2 Jun 2026 18:15:24 -0700
EJ Campbell <ej.campbell@gmail.com> wrote:

> The latest source address seen on the tap interface (ip4.addr_seen) is
> taken to be the guest's and is used as the target for inbound port
> forwarding. The update accepts any source address.
> 
> On a point-to-point tap that is always correct. But when the tap sits on
> a shared L2 segment -- for example when it is bridged together with other
> hosts -- pasta also sees frames from those other hosts.

That's a interesting use case, it's the first time I see pasta used
like this. Thanks for sharing.

> With a static
> configuration (-a / address set, DHCP disabled), an overheard frame from
> another host silently moves addr_seen to that host, and every subsequent
> inbound forwarded connection is sent there instead of to the guest, so
> connections are reset or hang.
> 
> I hit this running pasta with its tap enslaved to a Linux bridge that also
> carries other traffic: inbound port forwarding to the guest broke as soon
> as another address was seen on the segment.

Oops. We should definitely fix this, and maybe your patch is good enough
for the moment (or better than the alternative), but I have a concern
here:

> Restrict the update so that, when DHCP is disabled (no_dhcp), only frames
> sourced from the configured guest address may move addr_seen. With DHCP
> enabled -- the default -- behaviour is unchanged, since the guest's
> address is whatever pasta leases and tracking it is still correct.

...inferring possibly but not necessarily related information from
whether DHCP is disabled or enabled looks a bit risky to me:

- somebody might have DHCP enabled (just because it's the default
  option), but one single connected container / guest not using DHCP at
  all, and picking an address that we didn't expect. Your patch would
  break this (perhaps unlikely?) usage

- somebody might have DHCP enabled but just the intended container /
  guest using it, and your same problem. Your patch wouldn't fix that
  (definitely less likely) usage

So I've been thinking: shouldn't we rather use as a condition the fact
that -a / --address was explicitly given? We don't track that from
conf() at the moment but it would be a small change.

If -a / --address is given, I'd say the user is very much expecting the
container / guest to be using that address, and in that case I think
it's safer to ignore other addresses.

It would be even better if we could track our designated container /
guest among a number of them, only if there are more than one.

For context, Jon is working on a series adding support for multiple
addresses (assigned, used, or observed):

  https://archives.passt.top/passt-dev/20260413005319.3295910-1-jmaloy@redhat.com/

and patch 8/13:

  https://archives.passt.top/passt-dev/20260413005319.3295910-9-jmaloy@redhat.com/

refactors the same exact part your patch is touching, here:

  https://archives.passt.top/passt-dev/7f04e12/s/?b=tap.c#n764

with no functional change.

By the way, if you want to try things out on top of that series, you
can apply it with:

  b4 shazam
  https://archives.passt.top/passt-dev/20260413005319.3295910-1-jmaloy@redhat.com/

...you might want to check out a slightly older revision to apply
cleanly without conflicts.

The main purpose of this work is actually to implement a netlink
monitor to track changes in host addresses:

  https://bugs.passt.top/show_bug.cgi?id=141
  https://pad.passt.top/p/netlinkMonitor

...but it might also be convenient for this kind of tracking. For
guests, we could establish some preference rules, for example:

- if any guest made a DHCP / DHCPv6 request, select its address (I
  guess we should eventually implement guest-side MAC address tracking
  for a full solution but just preferring addresses we assigned might
  be quite good). I'm not sure how we would handle SLAAC

- if we haven't seen any DHCP / DHCPv6 request, but we have seen a
  single address, default to that one as "guest" address

and so on.

For containers, we actually know which namespace we're associated to,
so we could, on any new observed address, check the source MAC address
of the frame and see if it corresponds to our namespace (querying via
netlink).

Thoughts? Jon?

If this is all too complicated for the moment, would a condition based
on explicit -a / --address option work for you?

> The same reasoning applies to ip6.addr_seen on a shared segment; I left
> IPv6 alone for now since I only exercise IPv4 in this setup, and would be
> glad to extend it the same way if you prefer.

I think we should definitely take care of IPv6 as well, I'm just not
sure how.

A couple of comments about the patch itself:

> Signed-off-by: EJ Campbell <ej.campbell@gmail.com>
> ---
>  tap.c | 14 +++++++++++++-
>  1 file changed, 13 insertions(+), 1 deletion(-)
> 
> diff --git a/tap.c b/tap.c
> index 4cba4c7..514379e 100644
> --- a/tap.c
> +++ b/tap.c
> @@ -770,7 +770,19 @@ resume:
>                       continue;

It looks like the patch itself was copied and pasted from a terminal,
because all tabs became spaces, and the patch doesn't apply.

It's not a problem for small patches like this one as I tend to fix
things up manually, but it would be good to have it directly from git
send-email instead.

>               }
> 
> -             if (iph->saddr && c->ip4.addr_seen.s_addr != iph->saddr)
> +             /* The latest source address seen on the tap is taken to be the
> +              * guest's, and becomes the target for inbound forwarding. On a
> +              * point-to-point tap that's always correct. But if the tap sits
> +              * on a shared L2 segment (for example bridged together with
> +              * other hosts) and we serve a fixed address (DHCP disabled), a
> +              * frame overheard from another host on that segment must not
> +              * move addr_seen and silently retarget forwarded connections
> +              * away from the guest. When DHCP is disabled, only the
> +              * configured guest address may update addr_seen; with DHCP
> +              * enabled (the default) behaviour is unchanged.

The fact that the behaviour is _unchanged_ (by your patch) belongs in
the commit message rather than the code, because if one reads the code
they aren't explicitly reading your change and it's not clear to what
previous behaviour you would be referring to.

Other than that it's pretty well described, thanks. Let's see if we can
find a better solution though.

> +              */
> +             if (iph->saddr && c->ip4.addr_seen.s_addr != iph->saddr &&
> +                 (!c->no_dhcp || iph->saddr == c->ip4.addr.s_addr))
>                       c->ip4.addr_seen.s_addr = iph->saddr;
> 
>               if (!iov_drop_header(&data, hlen))
> 
> base-commit: 4b2823784aab04a70dfc295b16fd6f0592955790
> --
> 2.53.0
> 

-- 
Stefano

Re: [PATCH] tap: don't let overheard traffic move addr_seen when serving a fixed address

[David Gibson]

[-- Attachment #1: Type: text/plain, Size: 9660 bytes --]

On Fri, Jun 05, 2026 at 01:57:41PM +0200, Stefano Brivio wrote:
> EJ, thanks for the detailed patch and commit message, and apologies for
> the delayed response.
> 
> Some remarks:
> 
> On Tue, 2 Jun 2026 18:15:24 -0700
> EJ Campbell <ej.campbell@gmail.com> wrote:
> 
> > The latest source address seen on the tap interface (ip4.addr_seen) is
> > taken to be the guest's and is used as the target for inbound port
> > forwarding. The update accepts any source address.
> > 
> > On a point-to-point tap that is always correct. But when the tap sits on
> > a shared L2 segment -- for example when it is bridged together with other
> > hosts -- pasta also sees frames from those other hosts.
> 
> That's a interesting use case, it's the first time I see pasta used
> like this. Thanks for sharing.

Likewise, it's the first I've heard of someone using it specifically
like this, but it is a use model I've had in mind for a while.  So
this is a known limitation I've been thinking about for a while.  It
hasn't reached the top of my priority list because while your very
specific case almost works already, the more general case of a bridge
on the guest side requires a bunch more infrastructure.  That's in
progress or at least planned, but it's going to be a while.

Fwiw, I really dislike the addr_seen mechanism even in cases where it
"works": it allows poorly configured guests to work at the expense of
substantial complexity which impacts all cases.  However, it's
established behaviour, so Stefano is understandably against simply
removing it.

> > With a static
> > configuration (-a / address set, DHCP disabled), an overheard frame from
> > another host silently moves addr_seen to that host, and every subsequent
> > inbound forwarded connection is sent there instead of to the guest, so
> > connections are reset or hang.
> > 
> > I hit this running pasta with its tap enslaved to a Linux bridge that also
> > carries other traffic: inbound port forwarding to the guest broke as soon
> > as another address was seen on the segment.
> 
> Oops. We should definitely fix this, and maybe your patch is good enough
> for the moment (or better than the alternative), but I have a concern
> here:
> 
> > Restrict the update so that, when DHCP is disabled (no_dhcp), only frames
> > sourced from the configured guest address may move addr_seen. With DHCP
> > enabled -- the default -- behaviour is unchanged, since the guest's
> > address is whatever pasta leases and tracking it is still correct.
> 
> ...inferring possibly but not necessarily related information from
> whether DHCP is disabled or enabled looks a bit risky to me:
> 
> - somebody might have DHCP enabled (just because it's the default
>   option), but one single connected container / guest not using DHCP at
>   all, and picking an address that we didn't expect. Your patch would
>   break this (perhaps unlikely?) usage
> 
> - somebody might have DHCP enabled but just the intended container /
>   guest using it, and your same problem. Your patch wouldn't fix that
>   (definitely less likely) usage

Right.  I agree with Stefano that basing this of --no-dhcp is not a
good idea.  It happens to work for your case, but it doesn't
generalise well.  In addition to the cases Stefano mentions, for the
more general case of a guest-side bridge, we'd eventually like to have
our DHCP implementation serve multiple address, in which case we
wouldn't want addr_seen, even though DHCP is enabled.

> So I've been thinking: shouldn't we rather use as a condition the fact
> that -a / --address was explicitly given? We don't track that from
> conf() at the moment but it would be a small change.
> 
> If -a / --address is given, I'd say the user is very much expecting the
> container / guest to be using that address, and in that case I think
> it's safer to ignore other addresses.

I think having an explcit -a is a sufficient condition to disable
addr_seen, but maybe not a necessary one.  I think requiring the guest
to use the assigned address would be a reasonable request, even if
that address is taken from the host.

Stefano and others talked me out of an explicit --no-address-snooping
option, on the grounds of option proliferation at our last call.
However, thinking about it again, I'm still wondering if it might be
useful: we can think of addr_seen as a (not very good) address
assignment mechanism, along with DHCP, NDP, DHCPv6 and --config-net.
All of those (better) mechanisms can be explicitly disabled, so why
should we not be able to disable the worst one.

> It would be even better if we could track our designated container /
> guest among a number of them, only if there are more than one.
> 
> For context, Jon is working on a series adding support for multiple
> addresses (assigned, used, or observed):
> 
>   https://archives.passt.top/passt-dev/20260413005319.3295910-1-jmaloy@redhat.com/
> 
> and patch 8/13:
> 
>   https://archives.passt.top/passt-dev/20260413005319.3295910-9-jmaloy@redhat.com/
> 
> refactors the same exact part your patch is touching, here:
> 
>   https://archives.passt.top/passt-dev/7f04e12/s/?b=tap.c#n764
> 
> with no functional change.
> 
> By the way, if you want to try things out on top of that series, you
> can apply it with:
> 
>   b4 shazam
>   https://archives.passt.top/passt-dev/20260413005319.3295910-1-jmaloy@redhat.com/
> 
> ...you might want to check out a slightly older revision to apply
> cleanly without conflicts.
> 
> The main purpose of this work is actually to implement a netlink
> monitor to track changes in host addresses:
> 
>   https://bugs.passt.top/show_bug.cgi?id=141
>   https://pad.passt.top/p/netlinkMonitor
> 
> ...but it might also be convenient for this kind of tracking. For
> guests, we could establish some preference rules, for example:
> 
> - if any guest made a DHCP / DHCPv6 request, select its address (I
>   guess we should eventually implement guest-side MAC address tracking
>   for a full solution but just preferring addresses we assigned might
>   be quite good). I'm not sure how we would handle SLAAC
> 
> - if we haven't seen any DHCP / DHCPv6 request, but we have seen a
>   single address, default to that one as "guest" address
> 
> and so on.
> 
> For containers, we actually know which namespace we're associated to,
> so we could, on any new observed address, check the source MAC address
> of the frame and see if it corresponds to our namespace (querying via
> netlink).
> 
> Thoughts? Jon?
> 
> If this is all too complicated for the moment, would a condition based
> on explicit -a / --address option work for you?
> 
> > The same reasoning applies to ip6.addr_seen on a shared segment; I left
> > IPv6 alone for now since I only exercise IPv4 in this setup, and would be
> > glad to extend it the same way if you prefer.
> 
> I think we should definitely take care of IPv6 as well, I'm just not
> sure how.
> 
> A couple of comments about the patch itself:
> 
> > Signed-off-by: EJ Campbell <ej.campbell@gmail.com>
> > ---
> >  tap.c | 14 +++++++++++++-
> >  1 file changed, 13 insertions(+), 1 deletion(-)
> > 
> > diff --git a/tap.c b/tap.c
> > index 4cba4c7..514379e 100644
> > --- a/tap.c
> > +++ b/tap.c
> > @@ -770,7 +770,19 @@ resume:
> >                       continue;
> 
> It looks like the patch itself was copied and pasted from a terminal,
> because all tabs became spaces, and the patch doesn't apply.
> 
> It's not a problem for small patches like this one as I tend to fix
> things up manually, but it would be good to have it directly from git
> send-email instead.
> 
> >               }
> > 
> > -             if (iph->saddr && c->ip4.addr_seen.s_addr != iph->saddr)
> > +             /* The latest source address seen on the tap is taken to be the
> > +              * guest's, and becomes the target for inbound forwarding. On a
> > +              * point-to-point tap that's always correct. But if the tap sits
> > +              * on a shared L2 segment (for example bridged together with
> > +              * other hosts) and we serve a fixed address (DHCP disabled), a
> > +              * frame overheard from another host on that segment must not
> > +              * move addr_seen and silently retarget forwarded connections
> > +              * away from the guest. When DHCP is disabled, only the
> > +              * configured guest address may update addr_seen; with DHCP
> > +              * enabled (the default) behaviour is unchanged.
> 
> The fact that the behaviour is _unchanged_ (by your patch) belongs in
> the commit message rather than the code, because if one reads the code
> they aren't explicitly reading your change and it's not clear to what
> previous behaviour you would be referring to.
> 
> Other than that it's pretty well described, thanks. Let's see if we can
> find a better solution though.
> 
> > +              */
> > +             if (iph->saddr && c->ip4.addr_seen.s_addr != iph->saddr &&
> > +                 (!c->no_dhcp || iph->saddr == c->ip4.addr.s_addr))
> >                       c->ip4.addr_seen.s_addr = iph->saddr;
> > 
> >               if (!iov_drop_header(&data, hlen))
> > 
> > base-commit: 4b2823784aab04a70dfc295b16fd6f0592955790
> > --
> > 2.53.0
> > 
> 
> -- 
> Stefano
> 
> 

-- 
David Gibson (he or they)	| 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

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]