On 01/25/13 09:14, Amos Kong wrote:
> FD_SET() and FD_CLR() are used to add and remove one descriptor from a
> set, the 'fd' should be less than FD_SETSIZE. Glibc will give a warning
> and crash the qemu when we set a fd (1024) to a set.
>
> # qemu -device virtio-net-pci,netdev=macvtap_netdev,mac=92:ff:8a:11:fe:57
> -netdev tap,id=macvtap_netdev,fd=1024 1024<>/dev/tap4
>
> *** buffer overflow detected ***: x86_64-softmmu/qemu-system-x86_64
> terminated
> ======= Backtrace: =========
> /lib64/libc.so.6(__fortify_fail+0x37)[0x7f842a2134a7]
> /lib64/libc.so.6(+0x35e9d08620)[0x7f842a211620]
> /lib64/libc.so.6(+0x35e9d0a417)[0x7f842a213417]
> x86_64-softmmu/qemu-system-x86_64(+0x1901fd)[0x7f842f09f1fd]
> x86_64-softmmu/qemu-system-x86_64(+0x198388)[0x7f842f0a7388]
> x86_64-softmmu/qemu-system-x86_64(main+0xfa9)[0x7f842ef897a9]
> /lib64/libc.so.6(__libc_start_main+0xf5)[0x7f842a12aa05]
> x86_64-softmmu/qemu-system-x86_64(+0x7ed49)[0x7f842ef8dd49]
> ======= Memory map: ========
> ....
>
> This patch added limitations when init tap device and set fd handler
> for synchronous IO.
>
> Signed-off-by: Amos Kong <ak...@redhat.com>
> ---
> iohandler.c | 3 +++
> net/tap.c | 3 ++-
> 2 files changed, 5 insertions(+), 1 deletions(-)
>
> diff --git a/iohandler.c b/iohandler.c
> index 2523adc..c22edab 100644
> --- a/iohandler.c
> +++ b/iohandler.c
> @@ -66,6 +66,9 @@ int qemu_set_fd_handler2(int fd,
> }
> }
> } else {
> + if (fd >= FD_SETSIZE) {
> + return 1;
> + }
qemu_set_fd_handler2() -- and consequently, qemu_set_fd_handler() --
could never fail before. I tried to check their call sites, and most of
those don't bother to check for the return value; they assume these
functions always succeed.
Wouldn't it be better to abort() here (or exit with an error message)
instead of returning 1? (Not suggesting, just asking.)
Thanks,
Laszlo
> QLIST_FOREACH(ioh, &io_handlers, next) {
> if (ioh->fd == fd)
> goto found;
> diff --git a/net/tap.c b/net/tap.c
> index eb40c42..be856dd 100644
> --- a/net/tap.c
> +++ b/net/tap.c
> @@ -618,7 +618,8 @@ int net_init_tap(const NetClientOptions *opts, const char
> *name,
> }
>
> fd = monitor_handle_fd_param(cur_mon, tap->fd);
> - if (fd == -1) {
> + if (fd == -1 || fd >= FD_SETSIZE) {
> + error_report("Invalid fd : %d", fd);
> return -1;
> }
>
