Thanks for the fix David I sent an alternative fix here: https://patchwork.ozlabs.org/patch/1028908/
Darrell On Mon, Jan 21, 2019 at 2:15 AM David Marchand <[email protected]> wrote: > replace_substring() wants the total size of the string in order to move > the end of the string after the part being replaced. > > When replacing the ipv4 address in repl_ftp_v4_addr(), the remain_size > variable must be updated after replace_substring() has been called, not > before. > Besides, the substraction is off by one, since we need to account for the > ',' that is skipped. > > This goes unnoticed most of the time, unless you choose carefully your > setup and ip addresses. > Example for ftp in passive mode with address 10.1.1.200 DNAT'd to > 10.1.100.1: > > Breakpoint 1, replace_substring (substr=0x1e68497 > "10,1,100,1,241,144).\r\n", substr_size=2 '\002', total_size=20 '\024', > rep_str=0x7fff92ee4e50 "10", rep_str_size=2 '\002') at > lib/conntrack.c:2864 > > Breakpoint 1, replace_substring (substr=0x1e6849a > "1,100,1,241,144).\r\n", substr_size=1 '\001', total_size=19 '\023', > rep_str=0x7fff92ee4e50 "1", rep_str_size=1 '\001') at > lib/conntrack.c:2864 > > Breakpoint 1, replace_substring (substr=0x1e6849c "100,1,241,144).\r\n", > substr_size=3 '\003', total_size=16 '\020', rep_str=0x7fff92ee4e50 "1", > rep_str_size=1 '\001') at lib/conntrack.c:2864 > > Breakpoint 1, replace_substring (substr=0x1e6849e "1,241,144).\r.\r\n", > substr_size=1 '\001', total_size=15 '\017', rep_str=0x7fff92ee4e50 > "200", rep_str_size=3 '\003') at lib/conntrack.c:2864 > > From the "unnated" side, the payload ends up being incorrectly terminated: > > 0x0040: b715 3232 3720 456e 7465 7269 6e67 2050 ..227.Entering.P > 0x0050: 6173 7369 7665 204d 6f64 6520 2831 302c assive.Mode.(10, > 0x0060: 312c 312c 3230 302c 3234 312c 3134 3429 1,1,200,241,144) > 0x0070: 2e0d 2e ... > > Fixes: bd5e81a0e596 ("Userspace Datapath: Add ALG infra and FTP.") > Signed-off-by: David Marchand <[email protected]> > --- > lib/conntrack.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/lib/conntrack.c b/lib/conntrack.c > index f732b9e..8eddc8e 100644 > --- a/lib/conntrack.c > +++ b/lib/conntrack.c > @@ -2817,7 +2817,6 @@ repl_ftp_v4_addr(struct dp_packet *pkt, ovs_be32 > v4_addr_rep, > char *next_delim = memchr(byte_str, ',', 4); > ovs_assert(next_delim); > int substr_size = next_delim - byte_str; > - remain_size -= substr_size; > > /* Compose the new string for this octet, and replace it. */ > char rep_str[4]; > @@ -2825,6 +2824,7 @@ repl_ftp_v4_addr(struct dp_packet *pkt, ovs_be32 > v4_addr_rep, > int replace_size = sprintf(rep_str, "%d", rep_byte); > replace_substring(byte_str, substr_size, remain_size, > rep_str, replace_size); > + remain_size -= substr_size + 1; > overall_delta += replace_size - substr_size; > > /* Advance past the octet and the following comma. */ > -- > 1.8.3.1 > > _______________________________________________ dev mailing list [email protected] https://mail.openvswitch.org/mailman/listinfo/ovs-dev
