Hi,
ke, 2023-10-18 kello 17:34 +0300, Iulia Tanasescu kirjoitti:
> This makes it possible to bind a PA sync socket to a number of BISes
> before issuing the BIG Create Sync command.
>
> Signed-off-by: Iulia Tanasescu <iulia.tanasescu@xxxxxxx>
> ---
> net/bluetooth/iso.c | 40 ++++++++++++++++++++++++++++++++++++++++
> 1 file changed, 40 insertions(+)
>
> diff --git a/net/bluetooth/iso.c b/net/bluetooth/iso.c
> index 07b80e97aead..f20238c4702f 100644
> --- a/net/bluetooth/iso.c
> +++ b/net/bluetooth/iso.c
> @@ -813,6 +813,37 @@ static int iso_sock_bind_bc(struct socket *sock, struct sockaddr *addr,
> return 0;
> }
>
> +static int iso_sock_bind_pa_sk(struct sock *sk, struct sockaddr_iso *sa,
> + int addr_len)
> +{
> + int err = 0;
> +
> + if (sk->sk_type != SOCK_SEQPACKET) {
> + err = -EINVAL;
> + goto done;
> + }
> +
> + if (addr_len <= sizeof(*sa)) {
> + err = -EINVAL;
> + goto done;
> + }
> +
This does not seem to check addr_len is big enough, sizeof(*sa) won't
count the sa->iso_bc flexible array member.
That sa->iso_bc->bc_num_bis <= ISO_MAX_NUM_BIS is not checked, so
memcpy may write out of bounds.
The values in sa come from user, so may be invalid.
iso_sock_bind_bc seems to have similar issue.
Sorry for second round comments.
> + iso_pi(sk)->bc_num_bis = sa->iso_bc->bc_num_bis;
> +
> + for (int i = 0; i < iso_pi(sk)->bc_num_bis; i++)
> + if (sa->iso_bc->bc_bis[i] < 0x01 ||
> + sa->iso_bc->bc_bis[i] > 0x1f) {
> + err = -EINVAL;
> + goto done;
> + }
> +
> + memcpy(iso_pi(sk)->bc_bis, sa->iso_bc->bc_bis,
> + iso_pi(sk)->bc_num_bis);
> +
> +done:
> + return err;
> +}
> +
> static int iso_sock_bind(struct socket *sock, struct sockaddr *addr,
> int addr_len)
> {
> @@ -828,6 +859,15 @@ static int iso_sock_bind(struct socket *sock, struct sockaddr *addr,
>
> lock_sock(sk);
>
> + /* Allow the user to bind a PA sync socket to a number
> + * of BISes to sync to.
> + */
> + if (sk->sk_state == BT_CONNECT2 &&
> + test_bit(BT_SK_PA_SYNC, &iso_pi(sk)->flags)) {
> + err = iso_sock_bind_pa_sk(sk, sa, addr_len);
> + goto done;
> + }
> +
> if (sk->sk_state != BT_OPEN) {
> err = -EBADFD;
> goto done;
