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-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <CABBYNZKc7Y8JJ-J9+yUvnDTeVwYuqmzEZYpvfzvN0ctKGyj-Ow@mail.gmail.com>
Date:   Mon, 24 Oct 2022 13:54:24 -0700
From:   Luiz Augusto von Dentz <luiz.dentz@...il.com>
To:     Igor Skalkin <Igor.Skalkin@...nsynergy.com>
Cc:     virtualization@...ts.linux-foundation.org, mst@...hat.com,
        marcel@...tmann.org, johan.hedberg@...il.com, jasowang@...hat.com,
        linux-bluetooth@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH v4 1/1] virtio_bt: Fix alignment in configuration struct

Hi Igor,

On Mon, Oct 24, 2022 at 6:41 AM Igor Skalkin
<Igor.Skalkin@...nsynergy.com> wrote:
>
> The current version of the configuration structure has unaligned
> 16-bit fields, but according to the specification [1], access to
> the configuration space must be aligned.
>
> Add a second, aligned  version of the configuration structure
> and a new feature bit indicating that this version is being used.
>
> [1] https://docs.oasis-open.org/virtio/virtio/v1.1/virtio-v1.1.pdf
>
> Signed-off-by: Igor Skalkin <Igor.Skalkin@...nsynergy.com>
> ---
>  drivers/bluetooth/virtio_bt.c  | 16 +++++++++++++---
>  include/uapi/linux/virtio_bt.h |  8 ++++++++
>  2 files changed, 21 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/bluetooth/virtio_bt.c b/drivers/bluetooth/virtio_bt.c
> index 67c21263f9e0..35f8041722c8 100644
> --- a/drivers/bluetooth/virtio_bt.c
> +++ b/drivers/bluetooth/virtio_bt.c
> @@ -306,7 +306,12 @@ static int virtbt_probe(struct virtio_device *vdev)
>         if (virtio_has_feature(vdev, VIRTIO_BT_F_VND_HCI)) {
>                 __u16 vendor;
>
> -               virtio_cread(vdev, struct virtio_bt_config, vendor, &vendor);
> +               if (virtio_has_feature(vdev, VIRTIO_BT_F_CONFIG_V2))
> +                       virtio_cread(vdev, struct virtio_bt_config_v2,
> +                                    vendor, &vendor);
> +               else
> +                       virtio_cread(vdev, struct virtio_bt_config,
> +                                    vendor, &vendor);
>
>                 switch (vendor) {
>                 case VIRTIO_BT_CONFIG_VENDOR_ZEPHYR:
> @@ -339,8 +344,12 @@ static int virtbt_probe(struct virtio_device *vdev)
>         if (virtio_has_feature(vdev, VIRTIO_BT_F_MSFT_EXT)) {
>                 __u16 msft_opcode;
>
> -               virtio_cread(vdev, struct virtio_bt_config,
> -                            msft_opcode, &msft_opcode);
> +               if (virtio_has_feature(vdev, VIRTIO_BT_F_CONFIG_V2))
> +                       virtio_cread(vdev, struct virtio_bt_config_v2,
> +                                    msft_opcode, &msft_opcode);
> +               else
> +                       virtio_cread(vdev, struct virtio_bt_config,
> +                                    msft_opcode, &msft_opcode);
>
>                 hci_set_msft_opcode(hdev, msft_opcode);
>         }
> @@ -387,6 +396,7 @@ static const unsigned int virtbt_features[] = {
>         VIRTIO_BT_F_VND_HCI,
>         VIRTIO_BT_F_MSFT_EXT,
>         VIRTIO_BT_F_AOSP_EXT,
> +       VIRTIO_BT_F_CONFIG_V2,
>  };

So this introduces a new flag which must be checked when attempting to
config, right? But is this backward compatible? What happens if for
some reason the userspace doesn't use the new struct are we able to
detect that?

>  static struct virtio_driver virtbt_driver = {
> diff --git a/include/uapi/linux/virtio_bt.h b/include/uapi/linux/virtio_bt.h
> index a7bd48daa9a9..af798f4c9680 100644
> --- a/include/uapi/linux/virtio_bt.h
> +++ b/include/uapi/linux/virtio_bt.h
> @@ -9,6 +9,7 @@
>  #define VIRTIO_BT_F_VND_HCI    0       /* Indicates vendor command support */
>  #define VIRTIO_BT_F_MSFT_EXT   1       /* Indicates MSFT vendor support */
>  #define VIRTIO_BT_F_AOSP_EXT   2       /* Indicates AOSP vendor support */
> +#define VIRTIO_BT_F_CONFIG_V2  3       /* Use second version configuration */
>
>  enum virtio_bt_config_type {
>         VIRTIO_BT_CONFIG_TYPE_PRIMARY   = 0,
> @@ -28,4 +29,11 @@ struct virtio_bt_config {
>         __u16 msft_opcode;
>  } __attribute__((packed));
>
> +struct virtio_bt_config_v2 {
> +       __u8  type;
> +       __u8  alignment;
> +       __u16 vendor;
> +       __u16 msft_opcode;
> +};
> +
>  #endif /* _UAPI_LINUX_VIRTIO_BT_H */
> --
> 2.37.2
>


-- 
Luiz Augusto von Dentz

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ