Skip to content

Commit

Permalink
Merge branch 'net-handshake-fixes'
Browse files Browse the repository at this point in the history
Chuck Lever says:

====================
Bug fixes for net/handshake

Please consider these for merge via net-next.

Paolo observed that there is a possible leak of sock->file. I
haven't looked into that yet, but it seems to be separate from
the fixes in this series, so no need to hold these up.

Changes since v2:
- Address Paolo comment regarding handshake_dup()

Changes since v1:
- Rework "Fix handshake_dup() ref counting"
- Unpin sock->file when a handshake is cancelled
====================

Signed-off-by: David S. Miller <davem@davemloft.net>
  • Loading branch information
David S. Miller committed May 12, 2023
2 parents 0fae884 + eefca7e commit deb2e48
Show file tree
Hide file tree
Showing 8 changed files with 29 additions and 7 deletions.
4 changes: 4 additions & 0 deletions Documentation/netlink/specs/handshake.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -68,6 +68,9 @@ attribute-sets:
type: nest
nested-attributes: x509
multi-attr: true
-
name: peername
type: string
-
name: done
attributes:
Expand Down Expand Up @@ -105,6 +108,7 @@ operations:
- auth-mode
- peer-identity
- certificate
- peername
-
name: done
doc: Handler reports handshake completion
Expand Down
5 changes: 5 additions & 0 deletions Documentation/networking/tls-handshake.rst
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,7 @@ fills in a structure that contains the parameters of the request:
struct socket *ta_sock;
tls_done_func_t ta_done;
void *ta_data;
const char *ta_peername;
unsigned int ta_timeout_ms;
key_serial_t ta_keyring;
key_serial_t ta_my_cert;
Expand All @@ -71,6 +72,10 @@ instantiated a struct file in sock->file.
has completed. Further explanation of this function is in the "Handshake
Completion" sesction below.

The consumer can provide a NUL-terminated hostname in the @ta_peername
field that is sent as part of ClientHello. If no peername is provided,
the DNS hostname associated with the server's IP address is used instead.

The consumer can fill in the @ta_timeout_ms field to force the servicing
handshake agent to exit after a number of milliseconds. This enables the
socket to be fully closed once both the kernel and the handshake agent
Expand Down
1 change: 1 addition & 0 deletions include/net/handshake.h
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ struct tls_handshake_args {
struct socket *ta_sock;
tls_done_func_t ta_done;
void *ta_data;
const char *ta_peername;
unsigned int ta_timeout_ms;
key_serial_t ta_keyring;
key_serial_t ta_my_cert;
Expand Down
1 change: 1 addition & 0 deletions include/uapi/linux/handshake.h
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,7 @@ enum {
HANDSHAKE_A_ACCEPT_AUTH_MODE,
HANDSHAKE_A_ACCEPT_PEER_IDENTITY,
HANDSHAKE_A_ACCEPT_CERTIFICATE,
HANDSHAKE_A_ACCEPT_PEERNAME,

__HANDSHAKE_A_ACCEPT_MAX,
HANDSHAKE_A_ACCEPT_MAX = (__HANDSHAKE_A_ACCEPT_MAX - 1)
Expand Down
1 change: 1 addition & 0 deletions net/handshake/handshake.h
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@ struct handshake_req {
struct list_head hr_list;
struct rhash_head hr_rhash;
unsigned long hr_flags;
struct file *hr_file;
const struct handshake_proto *hr_proto;
struct sock *hr_sk;
void (*hr_odestruct)(struct sock *sk);
Expand Down
12 changes: 5 additions & 7 deletions net/handshake/netlink.c
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,7 @@ int handshake_genl_notify(struct net *net, const struct handshake_proto *proto,
proto->hp_handler_class))
return -ESRCH;

msg = genlmsg_new(GENLMSG_DEFAULT_SIZE, GFP_KERNEL);
msg = genlmsg_new(GENLMSG_DEFAULT_SIZE, flags);
if (!msg)
return -ENOMEM;

Expand Down Expand Up @@ -99,9 +99,6 @@ static int handshake_dup(struct socket *sock)
struct file *file;
int newfd;

if (!sock->file)
return -EBADF;

file = get_file(sock->file);
newfd = get_unused_fd_flags(O_CLOEXEC);
if (newfd < 0) {
Expand Down Expand Up @@ -142,15 +139,16 @@ int handshake_nl_accept_doit(struct sk_buff *skb, struct genl_info *info)
goto out_complete;
}
err = req->hr_proto->hp_accept(req, info, fd);
if (err)
if (err) {
fput(sock->file);
goto out_complete;
}

trace_handshake_cmd_accept(net, req, req->hr_sk, fd);
return 0;

out_complete:
handshake_complete(req, -EIO, NULL);
fput(sock->file);
out_status:
trace_handshake_cmd_accept_err(net, req, NULL, err);
return err;
Expand All @@ -159,8 +157,8 @@ int handshake_nl_accept_doit(struct sk_buff *skb, struct genl_info *info)
int handshake_nl_done_doit(struct sk_buff *skb, struct genl_info *info)
{
struct net *net = sock_net(skb->sk);
struct handshake_req *req = NULL;
struct socket *sock = NULL;
struct handshake_req *req;
int fd, status, err;

if (GENL_REQ_ATTR_CHECK(info, HANDSHAKE_A_DONE_SOCKFD))
Expand Down
4 changes: 4 additions & 0 deletions net/handshake/request.c
Original file line number Diff line number Diff line change
Expand Up @@ -239,6 +239,7 @@ int handshake_req_submit(struct socket *sock, struct handshake_req *req,
}
req->hr_odestruct = req->hr_sk->sk_destruct;
req->hr_sk->sk_destruct = handshake_sk_destruct;
req->hr_file = sock->file;

ret = -EOPNOTSUPP;
net = sock_net(req->hr_sk);
Expand Down Expand Up @@ -334,6 +335,9 @@ bool handshake_req_cancel(struct sock *sk)
return false;
}

/* Request accepted and waiting for DONE */
fput(req->hr_file);

out_true:
trace_handshake_cancel(net, req, sk);

Expand Down
8 changes: 8 additions & 0 deletions net/handshake/tlshd.c
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@ struct tls_handshake_req {
int th_type;
unsigned int th_timeout_ms;
int th_auth_mode;
const char *th_peername;
key_serial_t th_keyring;
key_serial_t th_certificate;
key_serial_t th_privkey;
Expand All @@ -48,6 +49,7 @@ tls_handshake_req_init(struct handshake_req *req,
treq->th_timeout_ms = args->ta_timeout_ms;
treq->th_consumer_done = args->ta_done;
treq->th_consumer_data = args->ta_data;
treq->th_peername = args->ta_peername;
treq->th_keyring = args->ta_keyring;
treq->th_num_peerids = 0;
treq->th_certificate = TLS_NO_CERT;
Expand Down Expand Up @@ -214,6 +216,12 @@ static int tls_handshake_accept(struct handshake_req *req,
ret = nla_put_u32(msg, HANDSHAKE_A_ACCEPT_MESSAGE_TYPE, treq->th_type);
if (ret < 0)
goto out_cancel;
if (treq->th_peername) {
ret = nla_put_string(msg, HANDSHAKE_A_ACCEPT_PEERNAME,
treq->th_peername);
if (ret < 0)
goto out_cancel;
}
if (treq->th_timeout_ms) {
ret = nla_put_u32(msg, HANDSHAKE_A_ACCEPT_TIMEOUT, treq->th_timeout_ms);
if (ret < 0)
Expand Down

0 comments on commit deb2e48

Please sign in to comment.