netdev - [RFC][PATCH] af_key: return error when meet errors on sendmsg() syscall

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]

Date:	Fri, 9 May 2014 13:47:35 +0800
From:	Xufeng Zhang <xufeng.zhang@...driver.com>
To:	<steffen.klassert@...unet.com>, <herbert@...dor.apana.org.au>,
	<davem@...emloft.net>
CC:	<netdev@...r.kernel.org>, <linux-kernel@...r.kernel.org>
Subject: [RFC][PATCH] af_key: return error when meet errors on sendmsg() syscall

Current implementation for pfkey_sendmsg() always return success
no matter whether or not error happens during this syscall,
this is incompatible with the general send()/sendmsg() API:
  man send
    RETURN VALUE
      On success, these calls return the number of characters sent.
      On error, -1 is returned, and errno is set appropriately.

One side effect this problem introduces is that we can't determine
when to resend the message when the previous send() fails because
it was interrupted by signals.
We detect such a problem when racoon is sending SADBADD message to
add SAD entry in the kernel, but sometimes kernel is responding with
"Interrupted system call"(-EINTR) error.

Check the send implementation of strongswan, it has below logic:
  pfkey_send_socket()
  {
  	...
  	while (TRUE)
  	{
        	len = send(socket, in, in_len, 0);

  		if (len != in_len)
  		{
  			case EINTR:
  				/* interrupted, try again */
  				continue;
  			...
  		}
  	}
  	...
}
So it makes sense to return errors for send() syscall.      

Signed-off-by: Xufeng Zhang <xufeng.zhang@...driver.com>
---
 net/key/af_key.c |    6 +++---
 1 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/net/key/af_key.c b/net/key/af_key.c
index f3c8307..9e4bc8c 100644
--- a/net/key/af_key.c
+++ b/net/key/af_key.c
@@ -335,7 +335,7 @@ static int pfkey_error(const struct sadb_msg *orig, int err, struct sock *sk)
 
 	pfkey_broadcast(skb, GFP_KERNEL, BROADCAST_ONE, sk, sock_net(sk));
 
-	return 0;
+	return -err;
 }
 
 static const u8 sadb_ext_min_len[] = {
@@ -3644,8 +3644,8 @@ static int pfkey_sendmsg(struct kiocb *kiocb,
 	mutex_unlock(&net->xfrm.xfrm_cfg_mutex);
 
 out:
-	if (err && hdr && pfkey_error(hdr, err, sk) == 0)
-		err = 0;
+	if (err && hdr)
+		err = pfkey_error(hdr, err, sk);
 	kfree_skb(skb);
 
 	return err ? : len;
-- 
1.7.0.2

--
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