linux-kernel - Re: [PATCH] virtio-spec: split virtio-net device status filed into ro and rw byte

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: <20120318122252.GE29902@redhat.com>
Date:	Sun, 18 Mar 2012 14:22:54 +0200
From:	"Michael S. Tsirkin" <mst@...hat.com>
To:	Jason Wang <jasowang@...hat.com>
Cc:	qemu-devel@...gnu.org, rusty@...tcorp.com.au,
	virtualization@...ts.linux-foundation.org,
	linux-kernel@...r.kernel.org
Subject: Re: [PATCH] virtio-spec: split virtio-net device status filed into
 ro and rw byte

On Fri, Mar 16, 2012 at 11:20:26PM +0800, Jason Wang wrote:
> This patch splits the device status field of virtio-net into ro and rw
> byte. This would simplify the implementation of both host and guest
> and make the layout more clean. As VIRTIO_NET_S_ANNOUNCE is a rw bit,
> it was moved to bit 8 (0x100).
> 
> btw. looks like there's no implementation that depends on
> VIRTIO_NET_S_ANNOUNCE, so the move is safe.
> 
> Signed-off-by: Jason Wang <jasowang@...hat.com>

Hmm, I know I proposed this myself, and I thought it will
prevent problems if we will add more rw bits,
but I missed the following race:

host writes VIRTIO_NET_S_ANNOUNCE = 1, interrupt

				guest reads VIRTIO_NET_S_ANNOUNCE = 1

host writes VIRTIO_NET_S_SOME_NEW_FIELD = 1, interrupt

				guest writes VIRTIO_NET_S_ANNOUNCE = 0
				VIRTIO_NET_S_SOME_NEW_FIELD is overwritten

				guest reads VIRTIO_NET_S_SOME_NEW_FIELD = 0

How about making the new bit write 1 to clear?
If we do, we can keep it where it is currently ...



> ---
>  virtio-0.9.4.lyx |   20 +++++++++++++++++++-
>  1 files changed, 19 insertions(+), 1 deletions(-)
> 
> diff --git a/virtio-0.9.4.lyx b/virtio-0.9.4.lyx
> index 6c7bab1..ef3951c 100644
> --- a/virtio-0.9.4.lyx
> +++ b/virtio-0.9.4.lyx
> @@ -58,6 +58,7 @@
>  \html_be_strict false
>  \author -608949062 "Rusty Russell,,," 
>  \author 1531152142 "pbonzini" 
> +\author 2090695081 "Jason" 
>  \end_header
>  
>  \begin_body
> @@ -4012,8 +4013,19 @@ configuration
>  layout Two configuration fields are currently defined.
>   The mac address field always exists (though is only valid if VIRTIO_NET_F_MAC
>   is set), and the status field only exists if VIRTIO_NET_F_STATUS is set.
> +
> +\change_inserted 2090695081 1331907586
> + The low byte of status field is read-only, guest write to this byte would
> + be ignored.
> + Currently only one bit is defined for this byte: VIRTIO_NET_S_LINK_UP.
> + The high byte of status field is read-writable.
> + Currently only one bit is defined for this byte: VIRTIO_NET_S_ANNOUNCE.
> +
> +\change_deleted 2090695081 1331907489
>   Two bits are currently defined for the status field: VIRTIO_NET_S_LINK_UP
>   and VIRTIO_NET_S_ANNOUNCE.
> +
> +\change_unchanged
>   
>  \begin_inset listings
>  inline false
> @@ -4026,7 +4038,13 @@ status open
>  
>  \begin_layout Plain Layout
>  
> -#define VIRTIO_NET_S_ANNOUNCE	2
> +#define VIRTIO_NET_S_ANNOUNCE	
> +\change_inserted 2090695081 1331907493
> +0x100
> +\change_deleted 2090695081 1331907491
> +2
> +\change_unchanged
> +
>  \end_layout
>  
>  \begin_layout Plain Layout
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/