[Qemu-devel] [PATCH] net: tap: fix NULL dereference when passing both fd and vhostfds to tap
This is because vhostfdname were passed as NULL to net_init_tap_one() when
vhostfd were not specified, but net_init_tap_one() will still pass it to
monitor_handle_fd_param() when tap-has_vhostfds is true. Since file descriptor
(fd, vhostfd) and file descriptor set (fds, vhostfds) were not compatible, so
this patch forbids passing them to tap in the same time.
This solve the segfault when passing the command line like:
./qemu-system-x86_64 -netdev tap,fd=2,vhost=on,vhostfds=baz,id=xyz
Cc: Paolo Bonzini pbonz...@redhat.com
Cc: Stefan Hajnoczi shajn...@redhat.com
Cc: Laszlo Ersek ler...@redhat.com
Cc: qemu-sta...@nongnu.org
Signed-off-by: Jason Wang jasow...@redhat.com
---
net/tap.c | 10 ++
1 files changed, 6 insertions(+), 4 deletions(-)
diff --git a/net/tap.c b/net/tap.c
index e0b7a2a..477505f 100644
--- a/net/tap.c
+++ b/net/tap.c
@@ -698,9 +698,10 @@ int net_init_tap(const NetClientOptions *opts, const char
*name,
if (tap-has_fd) {
if (tap-has_ifname || tap-has_script || tap-has_downscript ||
tap-has_vnet_hdr || tap-has_helper || tap-has_queues ||
-tap-has_fds) {
+tap-has_fds || tap-has_vhostfds) {
error_report(ifname=, script=, downscript=, vnet_hdr=,
- helper=, queues=, and fds= are invalid with fd=);
+ helper=, queues=, fds=, and vhostfds=
+ are invalid with fd=);
return -1;
}
@@ -725,9 +726,10 @@ int net_init_tap(const NetClientOptions *opts, const char
*name,
if (tap-has_ifname || tap-has_script || tap-has_downscript ||
tap-has_vnet_hdr || tap-has_helper || tap-has_queues ||
-tap-has_fd) {
+tap-has_fd || tap-has_vhostfd) {
error_report(ifname=, script=, downscript=, vnet_hdr=,
- helper=, queues=, and fd= are invalid with fds=);
+ helper=, queues=, fd=, and vhostfd=
+ are invalid with fds=);
return -1;
}
--
1.7.1
Re: [Qemu-devel] [PATCH] net: tap: fix NULL dereference when passing both fd and vhostfds to tap
comments below
On 06/03/13 11:04, Jason Wang wrote:
This is because vhostfdname were passed as NULL to net_init_tap_one() when
vhostfd were not specified, but net_init_tap_one() will still pass it to
monitor_handle_fd_param() when tap-has_vhostfds is true. Since file
descriptor
(fd, vhostfd) and file descriptor set (fds, vhostfds) were not compatible, so
this patch forbids passing them to tap in the same time.
This solve the segfault when passing the command line like:
./qemu-system-x86_64 -netdev tap,fd=2,vhost=on,vhostfds=baz,id=xyz
Cc: Paolo Bonzini pbonz...@redhat.com
Cc: Stefan Hajnoczi shajn...@redhat.com
Cc: Laszlo Ersek ler...@redhat.com
Cc: qemu-sta...@nongnu.org
Signed-off-by: Jason Wang jasow...@redhat.com
---
net/tap.c | 10 ++
1 files changed, 6 insertions(+), 4 deletions(-)
diff --git a/net/tap.c b/net/tap.c
index e0b7a2a..477505f 100644
--- a/net/tap.c
+++ b/net/tap.c
@@ -698,9 +698,10 @@ int net_init_tap(const NetClientOptions *opts, const
char *name,
if (tap-has_fd) {
if (tap-has_ifname || tap-has_script || tap-has_downscript ||
tap-has_vnet_hdr || tap-has_helper || tap-has_queues ||
-tap-has_fds) {
+tap-has_fds || tap-has_vhostfds) {
error_report(ifname=, script=, downscript=, vnet_hdr=,
- helper=, queues=, and fds= are invalid with fd=);
+ helper=, queues=, fds=, and vhostfds=
+ are invalid with fd=);
return -1;
}
This seems OK, since
has_fd precludeshas_fds
- optional: has_vhostfd - optional: has_vhostfds
@@ -725,9 +726,10 @@ int net_init_tap(const NetClientOptions *opts, const
char *name,
if (tap-has_ifname || tap-has_script || tap-has_downscript ||
tap-has_vnet_hdr || tap-has_helper || tap-has_queues ||
-tap-has_fd) {
+tap-has_fd || tap-has_vhostfd) {
error_report(ifname=, script=, downscript=, vnet_hdr=,
- helper=, queues=, and fd= are invalid with fds=);
+ helper=, queues=, fd=, and vhostfd=
+ are invalid with fds=);
return -1;
}
I think this us OK too (implementing the above exclusion from the other
side), but in the second hunk tap-has_fd can never be true actually.
I think relying on that in both the condition and the error message here
would simplify them.
Finally, I think the has_helper, and the outermost else branch
should both be updated as well, for consistency with hunk #1 here. These
branches use vhostfdname too. In these branches,
- both has_fd and has_fds are false (should simplify check errmsg),
- has_vhostfds should be rejected.
Thanks
Laszlo
Re: [Qemu-devel] [PATCH] net: tap: fix NULL dereference when passing both fd and vhostfds to tap
On 06/03/2013 06:48 PM, Laszlo Ersek wrote:
comments below
On 06/03/13 11:04, Jason Wang wrote:
This is because vhostfdname were passed as NULL to net_init_tap_one() when
vhostfd were not specified, but net_init_tap_one() will still pass it to
monitor_handle_fd_param() when tap-has_vhostfds is true. Since file
descriptor
(fd, vhostfd) and file descriptor set (fds, vhostfds) were not compatible, so
this patch forbids passing them to tap in the same time.
This solve the segfault when passing the command line like:
./qemu-system-x86_64 -netdev tap,fd=2,vhost=on,vhostfds=baz,id=xyz
Cc: Paolo Bonzini pbonz...@redhat.com
Cc: Stefan Hajnoczi shajn...@redhat.com
Cc: Laszlo Ersek ler...@redhat.com
Cc: qemu-sta...@nongnu.org
Signed-off-by: Jason Wang jasow...@redhat.com
---
net/tap.c | 10 ++
1 files changed, 6 insertions(+), 4 deletions(-)
diff --git a/net/tap.c b/net/tap.c
index e0b7a2a..477505f 100644
--- a/net/tap.c
+++ b/net/tap.c
@@ -698,9 +698,10 @@ int net_init_tap(const NetClientOptions *opts, const
char *name,
if (tap-has_fd) {
if (tap-has_ifname || tap-has_script || tap-has_downscript ||
tap-has_vnet_hdr || tap-has_helper || tap-has_queues ||
-tap-has_fds) {
+tap-has_fds || tap-has_vhostfds) {
error_report(ifname=, script=, downscript=, vnet_hdr=,
- helper=, queues=, and fds= are invalid with fd=);
+ helper=, queues=, fds=, and vhostfds=
+ are invalid with fd=);
return -1;
}
This seems OK, since
has_fd precludeshas_fds
- optional: has_vhostfd - optional: has_vhostfds
@@ -725,9 +726,10 @@ int net_init_tap(const NetClientOptions *opts, const
char *name,
if (tap-has_ifname || tap-has_script || tap-has_downscript ||
tap-has_vnet_hdr || tap-has_helper || tap-has_queues ||
-tap-has_fd) {
+tap-has_fd || tap-has_vhostfd) {
error_report(ifname=, script=, downscript=, vnet_hdr=,
- helper=, queues=, and fd= are invalid with fds=);
+ helper=, queues=, fd=, and vhostfd=
+ are invalid with fds=);
return -1;
}
I think this us OK too (implementing the above exclusion from the other
side), but in the second hunk tap-has_fd can never be true actually.
I think relying on that in both the condition and the error message here
would simplify them.
Ture
Finally, I think the has_helper, and the outermost else branch
should both be updated as well, for consistency with hunk #1 here. These
branches use vhostfdname too. In these branches,
- both has_fd and has_fds are false (should simplify check errmsg),
- has_vhostfds should be rejected.
Sure will send v2.
Thanks
Thanks
Laszlo
