Linux kernel mirror (for testing) git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git
kernel os linux

sctp: fix possibly using a bad saddr with a given dst

Under certain circumstances, depending on the order of addresses on the
interfaces, it could be that sctp_v[46]_get_dst() would return a dst
with a mismatched struct flowi.

For example, if when walking through the bind addresses and the first
one is not a match, it saves the dst as a fallback (added in
410f03831c07), but not the flowi. Then if the next one is also not a
match, the previous dst will be returned but with the flowi information
for the 2nd address, which is wrong.

The fix is to use a locally stored flowi that can be used for such
attempts, and copy it to the parameter only in case it is a possible
match, together with the corresponding dst entry.

The patch updates IPv6 code mostly just to be in sync. Even though the issue
is also present there, it fallback is not expected to work with IPv6.

Fixes: 410f03831c07 ("sctp: add routing output fallback")
Reported-by: Jin Meng <meng.a.jin@nokia-sbell.com>
Signed-off-by: Marcelo Ricardo Leitner <marcelo.leitner@gmail.com>
Tested-by: Xin Long <lucien.xin@gmail.com>
Signed-off-by: David S. Miller <davem@davemloft.net>

authored by

Marcelo Ricardo Leitner and committed by
David S. Miller
582eea23 5c3e82fe

+33 -15
+14 -6
net/sctp/ipv6.c
··· 228 228 { 229 229 struct sctp_association *asoc = t->asoc; 230 230 struct dst_entry *dst = NULL; 231 - struct flowi6 *fl6 = &fl->u.ip6; 231 + struct flowi _fl; 232 + struct flowi6 *fl6 = &_fl.u.ip6; 232 233 struct sctp_bind_addr *bp; 233 234 struct ipv6_pinfo *np = inet6_sk(sk); 234 235 struct sctp_sockaddr_entry *laddr; ··· 239 238 enum sctp_scope scope; 240 239 __u8 matchlen = 0; 241 240 242 - memset(fl6, 0, sizeof(struct flowi6)); 241 + memset(&_fl, 0, sizeof(_fl)); 243 242 fl6->daddr = daddr->v6.sin6_addr; 244 243 fl6->fl6_dport = daddr->v6.sin6_port; 245 244 fl6->flowi6_proto = IPPROTO_SCTP; ··· 277 276 rcu_read_unlock(); 278 277 279 278 dst = ip6_dst_lookup_flow(sock_net(sk), sk, fl6, final_p); 280 - if (!asoc || saddr) 279 + if (!asoc || saddr) { 280 + t->dst = dst; 281 + memcpy(fl, &_fl, sizeof(_fl)); 281 282 goto out; 283 + } 282 284 283 285 bp = &asoc->base.bind_addr; 284 286 scope = sctp_scope(daddr); ··· 304 300 if ((laddr->a.sa.sa_family == AF_INET6) && 305 301 (sctp_v6_cmp_addr(&dst_saddr, &laddr->a))) { 306 302 rcu_read_unlock(); 303 + t->dst = dst; 304 + memcpy(fl, &_fl, sizeof(_fl)); 307 305 goto out; 308 306 } 309 307 } ··· 344 338 if (!IS_ERR_OR_NULL(dst)) 345 339 dst_release(dst); 346 340 dst = bdst; 341 + t->dst = dst; 342 + memcpy(fl, &_fl, sizeof(_fl)); 347 343 break; 348 344 } 349 345 ··· 359 351 dst_release(dst); 360 352 dst = bdst; 361 353 matchlen = bmatchlen; 354 + t->dst = dst; 355 + memcpy(fl, &_fl, sizeof(_fl)); 362 356 } 363 357 rcu_read_unlock(); 364 358 ··· 369 359 struct rt6_info *rt; 370 360 371 361 rt = (struct rt6_info *)dst; 372 - t->dst = dst; 373 362 t->dst_cookie = rt6_get_cookie(rt); 374 363 pr_debug("rt6_dst:%pI6/%d rt6_src:%pI6\n", 375 364 &rt->rt6i_dst.addr, rt->rt6i_dst.plen, 376 - &fl6->saddr); 365 + &fl->u.ip6.saddr); 377 366 } else { 378 367 t->dst = NULL; 379 - 380 368 pr_debug("no route\n"); 381 369 } 382 370 }
+19 -9
net/sctp/protocol.c
··· 409 409 { 410 410 struct sctp_association *asoc = t->asoc; 411 411 struct rtable *rt; 412 - struct flowi4 *fl4 = &fl->u.ip4; 412 + struct flowi _fl; 413 + struct flowi4 *fl4 = &_fl.u.ip4; 413 414 struct sctp_bind_addr *bp; 414 415 struct sctp_sockaddr_entry *laddr; 415 416 struct dst_entry *dst = NULL; ··· 420 419 421 420 if (t->dscp & SCTP_DSCP_SET_MASK) 422 421 tos = t->dscp & SCTP_DSCP_VAL_MASK; 423 - memset(fl4, 0x0, sizeof(struct flowi4)); 422 + memset(&_fl, 0x0, sizeof(_fl)); 424 423 fl4->daddr = daddr->v4.sin_addr.s_addr; 425 424 fl4->fl4_dport = daddr->v4.sin_port; 426 425 fl4->flowi4_proto = IPPROTO_SCTP; ··· 439 438 &fl4->saddr); 440 439 441 440 rt = ip_route_output_key(sock_net(sk), fl4); 442 - if (!IS_ERR(rt)) 441 + if (!IS_ERR(rt)) { 443 442 dst = &rt->dst; 443 + t->dst = dst; 444 + memcpy(fl, &_fl, sizeof(_fl)); 445 + } 444 446 445 447 /* If there is no association or if a source address is passed, no 446 448 * more validation is required. ··· 506 502 odev = __ip_dev_find(sock_net(sk), laddr->a.v4.sin_addr.s_addr, 507 503 false); 508 504 if (!odev || odev->ifindex != fl4->flowi4_oif) { 509 - if (!dst) 505 + if (!dst) { 510 506 dst = &rt->dst; 511 - else 507 + t->dst = dst; 508 + memcpy(fl, &_fl, sizeof(_fl)); 509 + } else { 512 510 dst_release(&rt->dst); 511 + } 513 512 continue; 514 513 } 515 514 516 515 dst_release(dst); 517 516 dst = &rt->dst; 517 + t->dst = dst; 518 + memcpy(fl, &_fl, sizeof(_fl)); 518 519 break; 519 520 } 520 521 521 522 out_unlock: 522 523 rcu_read_unlock(); 523 524 out: 524 - t->dst = dst; 525 - if (dst) 525 + if (dst) { 526 526 pr_debug("rt_dst:%pI4, rt_src:%pI4\n", 527 - &fl4->daddr, &fl4->saddr); 528 - else 527 + &fl->u.ip4.daddr, &fl->u.ip4.saddr); 528 + } else { 529 + t->dst = NULL; 529 530 pr_debug("no route\n"); 531 + } 530 532 } 531 533 532 534 /* For v4, the source address is cached in the route entry(dst). So no need