aboutsummaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorDavid S. Miller2023-02-06 10:16:55 +0000
committerDavid S. Miller2023-02-06 10:16:55 +0000
commitc21adf256f8dcfbc07436d45be4ba2edf7a6f463 (patch)
treedb2d69568ee3cbce31d03565f18468eb6f63f628
parentb601135e8d704f3218efaf07cdb2ebf310aedc2a (diff)
parent66b2c338adce580dfce2199591e65e2bab889cff (diff)
Merge branch 'tuntap-socket-uid'
Pietro Borrello says: ==================== tuntap: correctly initialize socket uid sock_init_data() assumes that the `struct socket` passed in input is contained in a `struct socket_alloc` allocated with sock_alloc(). However, tap_open() and tun_chr_open() pass a `struct socket` embedded in a `struct tap_queue` and `struct tun_file` respectively, both allocated with sk_alloc(). This causes a type confusion when issuing a container_of() with SOCK_INODE() in sock_init_data() which results in assigning a wrong sk_uid to the `struct sock` in input. Due to the type confusion, both sockets happen to have their uid set to 0, i.e. root. While it will be often correct, as tuntap devices require CAP_NET_ADMIN, it may not always be the case. Not sure how widespread is the impact of this, it seems the socket uid may be used for network filtering and routing, thus tuntap sockets may be incorrectly managed. Additionally, it seems a socket with an incorrect uid may be returned to the vhost driver when issuing a get_socket() on a tuntap device in vhost_net_set_backend(). Fix the bugs by adding and using sock_init_data_uid(), which explicitly takes a uid as argument. Signed-off-by: Pietro Borrello <borrello@diag.uniroma1.it> --- Changes in v3: - Fix the bug by defining and using sock_init_data_uid() - Link to v2: https://lore.kernel.org/r/20230131-tuntap-sk-uid-v2-0-29ec15592813@diag.uniroma1.it Changes in v2: - Shorten and format comments - Link to v1: https://lore.kernel.org/r/20230131-tuntap-sk-uid-v1-0-af4f9f40979d@diag.uniroma1.it ==================== Signed-off-by: David S. Miller <davem@davemloft.net>
-rw-r--r--drivers/net/tap.c2
-rw-r--r--drivers/net/tun.c2
-rw-r--r--include/net/sock.h7
-rw-r--r--net/core/sock.c15
4 files changed, 20 insertions, 6 deletions
diff --git a/drivers/net/tap.c b/drivers/net/tap.c
index a2be1994b389..8941aa199ea3 100644
--- a/drivers/net/tap.c
+++ b/drivers/net/tap.c
@@ -533,7 +533,7 @@ static int tap_open(struct inode *inode, struct file *file)
q->sock.state = SS_CONNECTED;
q->sock.file = file;
q->sock.ops = &tap_socket_ops;
- sock_init_data(&q->sock, &q->sk);
+ sock_init_data_uid(&q->sock, &q->sk, inode->i_uid);
q->sk.sk_write_space = tap_sock_write_space;
q->sk.sk_destruct = tap_sock_destruct;
q->flags = IFF_VNET_HDR | IFF_NO_PI | IFF_TAP;
diff --git a/drivers/net/tun.c b/drivers/net/tun.c
index a7d17c680f4a..745131b2d6db 100644
--- a/drivers/net/tun.c
+++ b/drivers/net/tun.c
@@ -3448,7 +3448,7 @@ static int tun_chr_open(struct inode *inode, struct file * file)
tfile->socket.file = file;
tfile->socket.ops = &tun_socket_ops;
- sock_init_data(&tfile->socket, &tfile->sk);
+ sock_init_data_uid(&tfile->socket, &tfile->sk, inode->i_uid);
tfile->sk.sk_write_space = tun_sock_write_space;
tfile->sk.sk_sndbuf = INT_MAX;
diff --git a/include/net/sock.h b/include/net/sock.h
index dcd72e6285b2..937e842dc930 100644
--- a/include/net/sock.h
+++ b/include/net/sock.h
@@ -1956,7 +1956,12 @@ void sk_common_release(struct sock *sk);
* Default socket callbacks and setup code
*/
-/* Initialise core socket variables */
+/* Initialise core socket variables using an explicit uid. */
+void sock_init_data_uid(struct socket *sock, struct sock *sk, kuid_t uid);
+
+/* Initialise core socket variables.
+ * Assumes struct socket *sock is embedded in a struct socket_alloc.
+ */
void sock_init_data(struct socket *sock, struct sock *sk);
/*
diff --git a/net/core/sock.c b/net/core/sock.c
index f08b76acde9b..208634b01df5 100644
--- a/net/core/sock.c
+++ b/net/core/sock.c
@@ -3383,7 +3383,7 @@ void sk_stop_timer_sync(struct sock *sk, struct timer_list *timer)
}
EXPORT_SYMBOL(sk_stop_timer_sync);
-void sock_init_data(struct socket *sock, struct sock *sk)
+void sock_init_data_uid(struct socket *sock, struct sock *sk, kuid_t uid)
{
sk_init_common(sk);
sk->sk_send_head = NULL;
@@ -3403,11 +3403,10 @@ void sock_init_data(struct socket *sock, struct sock *sk)
sk->sk_type = sock->type;
RCU_INIT_POINTER(sk->sk_wq, &sock->wq);
sock->sk = sk;
- sk->sk_uid = SOCK_INODE(sock)->i_uid;
} else {
RCU_INIT_POINTER(sk->sk_wq, NULL);
- sk->sk_uid = make_kuid(sock_net(sk)->user_ns, 0);
}
+ sk->sk_uid = uid;
rwlock_init(&sk->sk_callback_lock);
if (sk->sk_kern_sock)
@@ -3466,6 +3465,16 @@ void sock_init_data(struct socket *sock, struct sock *sk)
refcount_set(&sk->sk_refcnt, 1);
atomic_set(&sk->sk_drops, 0);
}
+EXPORT_SYMBOL(sock_init_data_uid);
+
+void sock_init_data(struct socket *sock, struct sock *sk)
+{
+ kuid_t uid = sock ?
+ SOCK_INODE(sock)->i_uid :
+ make_kuid(sock_net(sk)->user_ns, 0);
+
+ sock_init_data_uid(sock, sk, uid);
+}
EXPORT_SYMBOL(sock_init_data);
void lock_sock_nested(struct sock *sk, int subclass)