Commit d73aa2867f33582314f098277421ded65f5745a9

Authored by Yann Droneaud
Committed by David S. Miller
1 parent de2aa4760b

net: handle error more gracefully in socketpair()

This patch makes socketpair() use error paths which do not
rely on heavy-weight call to sys_close(): it's better to try
to push the file descriptor to userspace before installing
the socket file to the file descriptor, so that errors are
catched earlier and being easier to handle.

Using sys_close() seems to be the exception, while writing the
file descriptor before installing it look like it's more or less
the norm: eg. except for code used in init/, error handling
involve fput() and put_unused_fd(), but not sys_close().

This make socketpair() usage of sys_close() quite unusual.
So it deserves to be replaced by the common pattern relying on
fput() and put_unused_fd() just like, for example, the one used
in pipe(2) or recvmsg(2).

Three distinct error paths are still needed since calling
fput() on file structure returned by sock_alloc_file() will
implicitly call sock_release() on the associated socket
structure.

Cc: David S. Miller <davem@davemloft.net>
Cc: Al Viro <viro@zeniv.linux.org.uk>
Signed-off-by: Yann Droneaud <ydroneaud@opteya.com>
Link: http://marc.info/?i=1385979146-13825-1-git-send-email-ydroneaud@opteya.com
Signed-off-by: David S. Miller <davem@davemloft.net>

Showing 1 changed file with 31 additions and 18 deletions Side-by-side Diff

... ... @@ -1445,48 +1445,61 @@
1445 1445 err = fd1;
1446 1446 goto out_release_both;
1447 1447 }
  1448 +
1448 1449 fd2 = get_unused_fd_flags(flags);
1449 1450 if (unlikely(fd2 < 0)) {
1450 1451 err = fd2;
1451   - put_unused_fd(fd1);
1452   - goto out_release_both;
  1452 + goto out_put_unused_1;
1453 1453 }
1454 1454  
1455 1455 newfile1 = sock_alloc_file(sock1, flags, NULL);
1456 1456 if (unlikely(IS_ERR(newfile1))) {
1457 1457 err = PTR_ERR(newfile1);
1458   - put_unused_fd(fd1);
1459   - put_unused_fd(fd2);
1460   - goto out_release_both;
  1458 + goto out_put_unused_both;
1461 1459 }
1462 1460  
1463 1461 newfile2 = sock_alloc_file(sock2, flags, NULL);
1464 1462 if (IS_ERR(newfile2)) {
1465 1463 err = PTR_ERR(newfile2);
1466   - fput(newfile1);
1467   - put_unused_fd(fd1);
1468   - put_unused_fd(fd2);
1469   - sock_release(sock2);
1470   - goto out;
  1464 + goto out_fput_1;
1471 1465 }
1472 1466  
  1467 + err = put_user(fd1, &usockvec[0]);
  1468 + if (err)
  1469 + goto out_fput_both;
  1470 +
  1471 + err = put_user(fd2, &usockvec[1]);
  1472 + if (err)
  1473 + goto out_fput_both;
  1474 +
1473 1475 audit_fd_pair(fd1, fd2);
  1476 +
1474 1477 fd_install(fd1, newfile1);
1475 1478 fd_install(fd2, newfile2);
1476 1479 /* fd1 and fd2 may be already another descriptors.
1477 1480 * Not kernel problem.
1478 1481 */
1479 1482  
1480   - err = put_user(fd1, &usockvec[0]);
1481   - if (!err)
1482   - err = put_user(fd2, &usockvec[1]);
1483   - if (!err)
1484   - return 0;
  1483 + return 0;
1485 1484  
1486   - sys_close(fd2);
1487   - sys_close(fd1);
1488   - return err;
  1485 +out_fput_both:
  1486 + fput(newfile2);
  1487 + fput(newfile1);
  1488 + put_unused_fd(fd2);
  1489 + put_unused_fd(fd1);
  1490 + goto out;
1489 1491  
  1492 +out_fput_1:
  1493 + fput(newfile1);
  1494 + put_unused_fd(fd2);
  1495 + put_unused_fd(fd1);
  1496 + sock_release(sock2);
  1497 + goto out;
  1498 +
  1499 +out_put_unused_both:
  1500 + put_unused_fd(fd2);
  1501 +out_put_unused_1:
  1502 + put_unused_fd(fd1);
1490 1503 out_release_both:
1491 1504 sock_release(sock2);
1492 1505 out_release_1: