netdev - [PATCH net-next] sock: consistent errqueue errors and signals

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [thread-next>] [day] [month] [year] [list]

Message-Id: <1409534896-372-1-git-send-email-willemb@google.com>
Date:	Sun, 31 Aug 2014 21:28:16 -0400
From:	Willem de Bruijn <willemb@...gle.com>
To:	netdev@...r.kernel.org
Cc:	davem@...emloft.net, Willem de Bruijn <willemb@...gle.com>
Subject: [PATCH net-next] sock: consistent errqueue errors and signals

When a socket error is pending, send()/recv() must abort their normal
operation and return the error. An error means having non-zero
sk->sk_err or having non-empty sk->sk_error_queue.

Currently, the behavior for the second is inconsistent depending on
whether an error has previously been dequeued. In all cases,
recv()/send() test sk->sk_err. This is not modified on enqueue onto
the error queue, so may be 0. It is modified on dequeue, however, to
match the queued skb's errno. I observed the following when two errors
were queued:

  ret = poll(pollfd, 1, -1);
  assert(ret == 1);
  assert(pollfd.revents == POLLERR);

  ret = recv(fd, buf, size, MSG_NONBLOCK);
  assert(ret == -1 && errno == EAGAIN);		/* <-- A */

  ret = recv(fd, buf, size, MSG_ERRQUEUE);
  assert(ret > 0);

  ret = recv(fd, buf, size, MSG_NONBLOCK);
  assert(ret == -1 && errno == ENOMSG);		/* <-- B */

  ret = recv(fd, buf, size, MSG_ERRQUEUE);
  assert(ret > 0);

The recv call in B returns the error code embedded in
SKB_EXT_ERR(skb), in this case ENOMSG, because I am working with
timestamps. The recv call in A should have returned the
same.

Implement this behavior. This may surprise existing applications.

Also make the wake-up signal when data is ready on the error queue
consistent between enqueue and dequeue: use sk_error_report in both
cases.

Signed-off-by: Willem de Bruijn <willemb@...gle.com>

---

This approach leaves one issue:
The states of sk->sk_err and sk->sk_error_queue are related, but only
loosely. Error queue enqueue, dequeue and other code may overwrite
sk->sk_err unconditionally. For one, sock_error will reset
sk->sk_err to 0 even if sk->sk_error_queue is not empty. If socket
calls should abort on all errors, then should be change to test
sk_error_queue.qlen. But, doing so requires taking a lock in a busy
data path.
---
 net/core/skbuff.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/net/core/skbuff.c b/net/core/skbuff.c
index 163b673..f7a280b 100644
--- a/net/core/skbuff.c
+++ b/net/core/skbuff.c
@@ -3485,8 +3485,11 @@ int sock_queue_err_skb(struct sock *sk, struct sk_buff *skb)
 	skb_dst_force(skb);

 	skb_queue_tail(&sk->sk_error_queue, skb);
+	sk->sk_err = SKB_EXT_ERR(skb)->ee.ee_errno;
+
 	if (!sock_flag(sk, SOCK_DEAD))
-		sk->sk_data_ready(sk);
+		sk->sk_error_report(sk);
+
 	return 0;
 }
 EXPORT_SYMBOL(sock_queue_err_skb);
-- 
2.1.0.rc2.206.gedb03e5

--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html