linux-kernel - Re: [PATCH] virtio_bt: Fix alignment in configuration struct

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>] [day] [month] [year] [list]

Date:   Sun, 7 Aug 2022 19:00:54 -0400
From:   "Michael S. Tsirkin" <mst@...hat.com>
To:     Igor Skalkin <Igor.Skalkin@...nsynergy.com>
Cc:     Jason Wang <jasowang@...hat.com>,
        virtualization@...ts.linux-foundation.org,
        linux-kernel@...r.kernel.org,
        Marcel Holtmann <marcel@...tmann.org>,
        Johan Hedberg <johan.hedberg@...il.com>,
        Luiz Augusto von Dentz <luiz.dentz@...il.com>,
        linux-bluetooth@...r.kernel.org
Subject: Re: [PATCH] virtio_bt: Fix alignment in configuration struct

On Mon, Aug 08, 2022 at 12:11:52AM +0200, Igor Skalkin wrote:
> According to specification [1], "For the device-specific configuration
> space, the driver MUST use 8 bit wide accesses for 8 bit wide fields,
> 16 bit wide and aligned accesses for 16 bit wide fields and 32 bit wide
> and aligned accesses for 32 and 64 bit wide fields.".
> 
> Current version of the configuration structure:
> 
>     struct virtio_bt_config {
>         __u8  type;
>         __u16 vendor;
>         __u16 msft_opcode;
>     } __attribute__((packed));
> 
> has both 16bit fields non-aligned.
> 
> This commit fixes it.
> 
> [1] https://docs.oasis-open.org/virtio/virtio/v1.1/virtio-v1.1.pdf
> 
> Signed-off-by: Igor Skalkin <Igor.Skalkin@...nsynergy.com>

This is all true enough, but the problem is
1. changing uapi like this can't be done, will break userspace
2. the driver has more issues and no one seems to want to
   maintain it. 
I posted a patch "Bluetooth: virtio_bt: mark broken" and intend
to merge it for this release.


> ---
>  include/uapi/linux/virtio_bt.h | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/include/uapi/linux/virtio_bt.h b/include/uapi/linux/virtio_bt.h
> index a7bd48daa9a9..adc03709cc4f 100644
> --- a/include/uapi/linux/virtio_bt.h
> +++ b/include/uapi/linux/virtio_bt.h
> @@ -23,9 +23,9 @@ enum virtio_bt_config_vendor {
>  };
>  
>  struct virtio_bt_config {
> -	__u8  type;
>  	__u16 vendor;
>  	__u16 msft_opcode;
> +	__u8  type;
>  } __attribute__((packed));
>  
>  #endif /* _UAPI_LINUX_VIRTIO_BT_H */
> -- 
> 2.34.1